fix(uaa-parity): Fix consecutive version events issue #3828
Conversation
JChan106
force-pushed
the
uaa-consecutive
branch
from
33dc195 to
47fd7a8
Compare
| let actionMessage = ''; | ||
| if (action === ACTION_TYPE_CREATED) { | ||
| actionMessage = ACTION_MESSAGE_UPLOAD; | ||
| } else if (action === ACTION_TYPE_RESTORED) { | ||
| actionMessage = ACTION_MESSAGE_RESTORE; | ||
| } else if (action === ACTION_TYPE_TRASHED) { | ||
| actionMessage = ACTION_MESSAGE_TRASH; | ||
| } | ||
|
|
There was a problem hiding this comment.
Should this use a switch statement for extendability and readability?
| action !== 'upload' && | ||
| action !== ACTION_TYPE_RESTORED && | ||
| action !== ACTION_TYPE_TRASHED && | ||
| action !== ACTION_TYPE_CREATED |
There was a problem hiding this comment.
I feel like a switch here with a default to null would read better as well or invert this gate and default to returning null.
| const collaborator = action_by ? action_by[0] : collaborators[collaboratorIDs[0]]; | ||
|
|
There was a problem hiding this comment.
is action_by always going to have an element?
There was a problem hiding this comment.
It should, I added some optional chaining to the new code regardless. I also passed in the FF to make it easier to understand the logic.
| test.each` | ||
| actionType | actionText | ||
| ${ACTION_TYPE_RESTORED} | ${'restored v'} | ||
| ${ACTION_TYPE_TRASHED} | ${'deleted v'} | ||
| ${ACTION_TYPE_CREATED} | ${'uploaded v'} | ||
| `('should correctly render when shouldUseUAA is true with actionType $actionType', ({ actionType, actionText }) => { | ||
| renderComponent({ | ||
| shouldUseUAA: true, | ||
| action_type: actionType, | ||
| action_by: [{ name: 'John Doe', id: 3 }], | ||
| version_start: 2, | ||
| version_end: 4, | ||
| }); | ||
|
|
||
| expect(screen.getByText('John Doe')).toBeInTheDocument(); | ||
| expect(screen.getByText(actionText)).toBeInTheDocument(); | ||
| expect(screen.getByText('2 - 4')).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
Nice, thanks for converting this!
JChan106
force-pushed
the
uaa-consecutive
branch
from
47fd7a8 to
19ad5ec
Compare
| jest.mock('react-intl', () => ({ | ||
| ...jest.requireActual('react-intl'), | ||
| })); |
There was a problem hiding this comment.
if you import render and screen from test-utils/testing-library, you won't need to mock this or import IntlProvider separately in this file. there should be some examples in this repo
There was a problem hiding this comment.
Tests not passing when I do that for some reason . Given this change is a little time-sensitive, I will leave as is.
Handle UAA consecutive version events in a range. Also replaces enzyme with RTL in corresponding test.