Repository navigation
fix(preview): Ensure promoted-by field shows correct user - #3414
Conversation
greg-in-a-box
left a comment
There was a problem hiding this comment.
I think we need some tests added since this is a bug we are trying to fix and ensure it always correct.
| uploader_display_name, | ||
| }: $Shape<BoxItemVersion>): User => { | ||
| const { name, id, ...rest } = restored_by || trashed_by || modified_by || PLACEHOLDER_USER; | ||
| const { name, id, ...rest } = restored_by || trashed_by || promoted_by || modified_by || PLACEHOLDER_USER; |
There was a problem hiding this comment.
I think we should update the test for this selector to include promoted_by. Do we need any other test to ensure that promoted_by is being used on the sidebar?
There was a problem hiding this comment.
Update the test for this code to include the promoted_by. I don't see anywhere else that needs updated
There was a problem hiding this comment.
I was thinking having a test at this level too
box-ui-elements/src/elements/content-sidebar/versions/VersionsItem.js
Lines 141 to 146 in f950772
There was a problem hiding this comment.
assuming this is the component that is shown in your image in your description
There was a problem hiding this comment.
Included this file as well
| FIELD_MODIFIED_BY, | ||
| FIELD_RESTORED_FROM, | ||
| FIELD_SIZE, | ||
| FIELD_PROMOTED_BY, |
There was a problem hiding this comment.
To be fair, it's not truly alphabetical 😣
| FIELD_NAME, | ||
| FIELD_TYPE, | ||
| FIELD_SIZE, | ||
| FIELD_PROMOTED_BY, |
There was a problem hiding this comment.
could you move to this the [...]_BY section below to keep the user fields together
FIELD_CREATED_BY,
FIELD_MODIFIED_BY,
FIELD_OWNED_BY,
FIELD_PROMOTED_BY,
FIELD_RESTORED_BY,
FIELD_TRASHED_BY,
| FIELD_PARENT, | ||
| FIELD_EXTENSION, | ||
| FIELD_PERMISSIONS, | ||
| FIELD_PROMOTED_BY, |
There was a problem hiding this comment.
same comment about grouping with the other [...]_BY fields
There was a problem hiding this comment.
This list actually is sorted alphabetically 😅
| export const FIELD_NAME: 'name' = 'name'; | ||
| export const FIELD_TYPE = 'type'; | ||
| export const FIELD_SIZE: 'size' = 'size'; | ||
| export const FIELD_PROMOTED_BY = 'promoted_by'; |
There was a problem hiding this comment.
also near the other [...]_BY
There was a problem hiding this comment.
I did place this near the other "BY"s but I'm not sure I'm a fan of this "sorting" choice. Could easily be mistaken for being random order. Would prefer pure alphabetical
The promoted-by field displays the owner when it should display the promoter.