use review period permissions in UI #2444
Conversation
| selectHasAnniversaryReportPermission, | ||
| selectHasBirthdayReportPermission, | ||
| selectHasCheckinsReportPermission, | ||
| selectHasPulseReportPermission, |
There was a problem hiding this comment.
This was renamed to selectHasViewPulseReportPermission for consistency with other permissions.
| import PropTypes from 'prop-types'; | ||
| import queryString from 'query-string'; | ||
| import React, { | ||
| useEffect, |
There was a problem hiding this comment.
Alphabetized imported items.
There was a problem hiding this comment.
My OCD thanks you for this!
| label={reviewer.name} | ||
| variant="outlined" | ||
| onDelete={() => deleteReviewer(member, reviewer)} | ||
| onDelete={canUpdate ? () => deleteReviewer(member, reviewer) : null} |
There was a problem hiding this comment.
Lots of UI elements are not either removed or disabled based on the values of canUpdate and canLaunch.
| export const selectReviewPeriods = state => state.reviewPeriods; | ||
| export const selectPermissions = state => state.permissions; | ||
|
|
||
| const hasPermission = permissionName => createSelector( |
There was a problem hiding this comment.
I removed a lot of code duplication by defining this function.
There was a problem hiding this comment.
So much better to read!
| p?.permission?.includes('CAN_UPDATE_REVIEW_ASSIGNMENTS') | ||
| ) | ||
| ); | ||
| export const selectHasCreateReviewAssignmentsPermission = |
There was a problem hiding this comment.
I added selectors for all the new permissions related to review assignments and review periods.
Maybe Tim is removing some of these.
There was a problem hiding this comment.
I think all these are staying 👍
| const hasPermission = selectHasPermissionAssignmentPermission(state); | ||
| const [selectedRole, setSelectedRole] = useState(roles[0]); | ||
| const [categoriesList, setCategoriesList] = useState([]); | ||
| //TODO: This is not yet getting the "Pulse" category of permissions, |
| selectCsrfToken, | ||
| selectCurrentUser, | ||
| selectHasPulseReportPermission, | ||
| selectHasViewPulseReportPermission, |
There was a problem hiding this comment.
Just renamed this selector.
| //TODO: Skipping the permission check during testing | ||
| // because this permission has not been implemented yet. | ||
| // if (selectHasPulseReportPermission(state)) loadPulses(); | ||
| // TODO: Uncomment this check after PR #2429 is merged. |
There was a problem hiding this comment.
As far as I can tell, it's working 🎉
Do we need a PulseReportPage.test.jsx though 🤔
There was a problem hiding this comment.
Pushed a fix for the test here #2479 the date was changing in the snapshot when under test
No description provided.