Skip to content

MM-59373: Respecting name display preference in boards #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Nov 8, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,9 @@ exports[`components/cardDetail/comment return comment 1`] = `
</div>
</div>
<div
class="comment-text"
class="mocked-message-html"
>
<p>
Test comment
</p>
Test Comment
</div>
</div>
</div>
Expand Down Expand Up @@ -246,11 +244,9 @@ exports[`components/cardDetail/comment return comment and delete comment 1`] = `
</div>
</div>
<div
class="comment-text"
class="mocked-message-html"
>
<p>
Test comment
</p>
Test Comment
</div>
</div>
</div>
Expand Down Expand Up @@ -285,11 +281,9 @@ exports[`components/cardDetail/comment return comment readonly 1`] = `
</div>
</div>
<div
class="comment-text"
class="mocked-message-html"
>
<p>
Test comment
</p>
Test Comment
</div>
</div>
</div>
Expand Down Expand Up @@ -422,11 +416,9 @@ exports[`components/cardDetail/comment return guest comment 1`] = `
</div>
</div>
<div
class="comment-text"
class="mocked-message-html"
>
<p>
Test comment
</p>
Test Comment
</div>
</div>
</div>
Expand Down Expand Up @@ -559,11 +551,9 @@ exports[`components/cardDetail/comment return guest comment and delete comment 1
</div>
</div>
<div
class="comment-text"
class="mocked-message-html"
>
<p>
Test comment
</p>
Test Comment
</div>
</div>
</div>
Expand Down Expand Up @@ -607,11 +597,9 @@ exports[`components/cardDetail/comment return guest comment readonly 1`] = `
</div>
</div>
<div
class="comment-text"
class="mocked-message-html"
>
<p>
Test comment
</p>
Test Comment
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,9 @@ exports[`components/cardDetail/CommentsList comments show up 1`] = `
</div>
</div>
<div
class="comment-text"
class="mocked-message-html"
>
<p>
Comment 2
</p>
Demo Comments
</div>
</div>
<div
Expand Down Expand Up @@ -111,11 +109,9 @@ exports[`components/cardDetail/CommentsList comments show up 1`] = `
</div>
</div>
<div
class="comment-text"
class="mocked-message-html"
>
<p>
Comment 1
</p>
Demo Comments
</div>
</div>
<hr
Expand Down Expand Up @@ -155,11 +151,9 @@ exports[`components/cardDetail/CommentsList comments show up in readonly mode 1`
</div>
</div>
<div
class="comment-text"
class="mocked-message-html"
>
<p>
Comment 2
</p>
Demo Comments
</div>
</div>
<div
Expand Down Expand Up @@ -187,11 +181,9 @@ exports[`components/cardDetail/CommentsList comments show up in readonly mode 1`
</div>
</div>
<div
class="comment-text"
class="mocked-message-html"
>
<p>
Comment 1
</p>
Demo Comments
</div>
</div>
<hr
Expand Down
23 changes: 15 additions & 8 deletions webapp/src/components/cardDetail/cardDetail.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,16 @@ import octoClient from '../../octoClient'

import {createTextBlock} from '../../blocks/textBlock'

import {mockMMStore} from '../../../tests/mock_window'

import CardDetail from './cardDetail'

global.fetch = FetchMock.fn
jest.mock('../../octoClient')
jest.mock('../../webapp_globals', () => ({
...jest.requireActual('../../webapp_globals'),
messageHtmlToComponent: jest.fn(() => <div className="mocked-message-html">Test Comment</div>),
}))

const mockedOctoClient = mocked(octoClient, true)

Expand All @@ -42,6 +48,7 @@ beforeAll(() => {
})

describe('components/cardDetail/CardDetail', () => {
(window as any).store = mockMMStore
const board = TestBlockFactory.createBoard()

const view = TestBlockFactory.createBoardView(board)
Expand Down Expand Up @@ -73,7 +80,7 @@ describe('components/cardDetail/CardDetail', () => {
},
},
teams: {
current: {id: 'team-id'},
current: {id: 'team_id'},
},
boards: {
boards: {
Expand Down Expand Up @@ -126,7 +133,7 @@ describe('components/cardDetail/CardDetail', () => {
expect(container).toBeDefined()

// Comments show up
const comments = container!.querySelectorAll('.comment-text')
const comments = container!.querySelectorAll('.mocked-message-html')
expect(comments.length).toBe(2)

// Add comment option visible when readonly mode is off
Expand All @@ -138,7 +145,7 @@ describe('components/cardDetail/CardDetail', () => {
const mockStore = configureStore([])
const store = mockStore({
teams: {
current: {id: 'team-id'},
current: {id: 'team_id'},
},
boards: {
boards: {
Expand Down Expand Up @@ -190,7 +197,7 @@ describe('components/cardDetail/CardDetail', () => {
expect(container).toBeDefined()

// comments show up
const comments = container!.querySelectorAll('.comment-text')
const comments = container!.querySelectorAll('.mocked-message-html')
expect(comments.length).toBe(2)

// Add comment option is not shown in readonly mode
Expand Down Expand Up @@ -223,7 +230,7 @@ describe('components/cardDetail/CardDetail', () => {
},
},
teams: {
current: {id: 'team-id'},
current: {id: 'team_id'},
},
boards: {
boards: {
Expand Down Expand Up @@ -330,7 +337,7 @@ describe('components/cardDetail/CardDetail', () => {
},
},
teams: {
current: {id: 'team-id'},
current: {id: 'team_id'},
},
boards: {
boards: {
Expand Down Expand Up @@ -435,7 +442,7 @@ describe('components/cardDetail/CardDetail', () => {
},
},
teams: {
current: {id: 'team-id'},
current: {id: 'team_id'},
},
boards: {
boards: {
Expand Down Expand Up @@ -533,7 +540,7 @@ describe('components/cardDetail/CardDetail', () => {
],
},
teams: {
current: {id: 'team-id'},
current: {id: 'team_id'},
},
boards: {
boards: {
Expand Down
52 changes: 49 additions & 3 deletions webapp/src/components/cardDetail/comment.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,16 @@ import {TestBlockFactory} from '../../test/testBlockFactory'

import mutator from '../../mutator'

import {mockMMStore} from '../../../tests/mock_window'

import Comment from './comment'

jest.mock('../../mutator')
const mockedMutator = mocked(mutator, true)
jest.mock('../../webapp_globals', () => ({
...jest.requireActual('../../webapp_globals'),
messageHtmlToComponent: jest.fn(() => <div className="mocked-message-html">Test Comment</div>),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't part of this PR, but when I saw this, I realized that whenever we get to shipping a plugin API package from the web app, it should include mock versions of all the APIs to make writing tests that use them easier.

In the mean time, you could add this to the Jest setup script to avoid having to redefine it in multiple test files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hmhealey Looking at the repo now, I realize that jest.config.js isn’t configured in the board/focalboard repo. I’m planning to set up the proper Jest configuration in a follow-up PR, which will include the changes mentioned above.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually configured as part of the package.json currently, but moving it out to it's own file and setting it up a bit better makes sense to me. While it's nice to use fewer files sometimes, I prefer having it spread out since the package.json already does so much stuff

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, got it. Done.

}))

const board = TestBlockFactory.createBoard()
const card = TestBlockFactory.createCard(board)
Expand All @@ -30,10 +36,14 @@ comment.title = 'Test comment'
const userImageUrl = 'data:image/svg+xml'

describe('components/cardDetail/comment', () => {
(window as any).store = mockMMStore
const state = {
users: {
boardUsers: {[comment.modifiedBy]: {username: 'username_1'}},
},
teams: {
current: {id: 'team_id'},
},
}
const store = mockStateStore([], state)

Expand Down Expand Up @@ -101,7 +111,19 @@ describe('components/cardDetail/comment', () => {
})

test('return guest comment', () => {
const localStore = mockStateStore([], {users: {boardUsers: {[comment.modifiedBy]: {username: 'username_1', is_guest: true}}}})
const localStore = mockStateStore([], {
users: {
boardUsers: {
[comment.modifiedBy]: {
username: 'username_1',
is_guest: true
}
}
},
teams: {
current: {id: 'team_id'},
}
})
const {container} = render(wrapIntl(
<ReduxProvider store={localStore}>
<Comment
Expand All @@ -118,7 +140,19 @@ describe('components/cardDetail/comment', () => {
})

test('return guest comment readonly', () => {
const localStore = mockStateStore([], {users: {boardUsers: {[comment.modifiedBy]: {username: 'username_1', is_guest: true}}}})
const localStore = mockStateStore([], {
users: {
boardUsers: {
[comment.modifiedBy]: {
username: 'username_1',
is_guest: true
}
}
},
teams: {
current: {id: 'team_id'},
},
})
const {container} = render(wrapIntl(
<ReduxProvider store={localStore}>
<Comment
Expand All @@ -133,7 +167,19 @@ describe('components/cardDetail/comment', () => {
})

test('return guest comment and delete comment', () => {
const localStore = mockStateStore([], {users: {boardUsers: {[comment.modifiedBy]: {username: 'username_1', is_guest: true}}}})
const localStore = mockStateStore([], {
users: {
boardUsers: {
[comment.modifiedBy]: {
username: 'username_1',
is_guest: true
}
}
},
teams: {
current: {id: 'team_id'},
},
})
const {container} = render(wrapIntl(
<ReduxProvider store={localStore}>
<Comment
Expand Down
29 changes: 24 additions & 5 deletions webapp/src/components/cardDetail/comment.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
import React, {FC} from 'react'
import {useIntl} from 'react-intl'

import {getChannelsNameMapInTeam} from 'mattermost-redux/selectors/entities/channels'

import {Provider} from 'react-redux'

import {Block} from '../../blocks/block'
import mutator from '../../mutator'
import {Utils} from '../../utils'
Expand All @@ -17,6 +21,9 @@ import Tooltip from '../../widgets/tooltip'
import GuestBadge from '../../widgets/guestBadge'

import './comment.scss'
import {formatText, messageHtmlToComponent} from '../../webapp_globals'
import {getCurrentTeam} from '../../store/teams'


type Props = {
comment: Block
Expand All @@ -28,10 +35,25 @@ type Props = {
const Comment: FC<Props> = (props: Props) => {
const {comment, userId, userImageUrl} = props
const intl = useIntl()
const html = Utils.htmlFromMarkdown(comment.title)
const user = useAppSelector(getUser(userId))
const date = new Date(comment.createAt)

const selectedTeam = useAppSelector(getCurrentTeam)
const channelNamesMap = getChannelsNameMapInTeam((window as any).store.getState(), selectedTeam!.id)

const formattedText =
<Provider store={(window as any).store}>
{messageHtmlToComponent(formatText(comment.title, {
singleline: false,
atMentions: true,
mentionHighlight: false,
team: selectedTeam,
channelNamesMap,
}), {
fetchMissingUsers: true,
})}
</Provider>

return (
<div
key={comment.id}
Expand Down Expand Up @@ -65,10 +87,7 @@ const Comment: FC<Props> = (props: Props) => {
</MenuWrapper>
)}
</div>
<div
className='comment-text'
dangerouslySetInnerHTML={{__html: html}}
/>
{formattedText}
</div>
)
}
Expand Down
Loading
Loading