From d3fc66fbcf3c1d646bb956e9be3f3e7ea28339e4 Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Mon, 17 Feb 2025 09:30:03 -0600 Subject: [PATCH 1/5] Added new selector method for getting active or inactive profiles (used for kudos display). --- web-ui/src/components/kudos/PublicKudosCard.jsx | 7 +++++-- web-ui/src/components/kudos_card/KudosCard.jsx | 7 +++++-- web-ui/src/context/selectors.js | 10 ++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/web-ui/src/components/kudos/PublicKudosCard.jsx b/web-ui/src/components/kudos/PublicKudosCard.jsx index 08e677170..b3ab124dd 100644 --- a/web-ui/src/components/kudos/PublicKudosCard.jsx +++ b/web-ui/src/components/kudos/PublicKudosCard.jsx @@ -18,7 +18,10 @@ import { DialogActions, TextField, } from "@mui/material"; -import { selectCsrfToken, selectProfile } from "../../context/selectors"; +import { + selectCsrfToken, + selectActiveOrInactiveProfile, +} from "../../context/selectors"; import { AppContext } from "../../context/AppContext"; import { getAvatarURL } from "../../api/api"; import DateFnsUtils from "@date-io/date-fns"; @@ -49,7 +52,7 @@ const KudosCard = ({ kudos }) => { const { state, dispatch } = useContext(AppContext); const csrf = selectCsrfToken(state); - const sender = selectProfile(state, kudos.senderId); + const sender = selectActiveOrInactiveProfile(state, kudos.senderId); const multiTooltip = (num, list) => { let tooltip = ""; diff --git a/web-ui/src/components/kudos_card/KudosCard.jsx b/web-ui/src/components/kudos_card/KudosCard.jsx index 798f0d946..b856dc976 100644 --- a/web-ui/src/components/kudos_card/KudosCard.jsx +++ b/web-ui/src/components/kudos_card/KudosCard.jsx @@ -20,7 +20,10 @@ import { FormControlLabel, Checkbox, } from "@mui/material"; -import { selectCsrfToken, selectProfile } from "../../context/selectors"; +import { + selectCsrfToken, + selectActiveOrInactiveProfile, +} from "../../context/selectors"; import MemberSelector from '../member_selector/MemberSelector'; import { AppContext } from "../../context/AppContext"; import { getAvatarURL } from "../../api/api"; @@ -63,7 +66,7 @@ const KudosCard = ({ kudos, includeActions, includeEdit, onKudosAction }) => { const [memberSelectorOpen, setMemberSelectorOpen] = useState(false); const [kudosRecipientMembers, setKudosRecipientMembers] = useState(kudos.recipientMembers); - const sender = selectProfile(state, kudos.senderId); + const sender = selectActiveOrInactiveProfile(state, kudos.senderId); const getRecipientComponent = useCallback(() => { if (kudos.recipientTeam) { diff --git a/web-ui/src/context/selectors.js b/web-ui/src/context/selectors.js index 984f7d43a..e7ad32f64 100644 --- a/web-ui/src/context/selectors.js +++ b/web-ui/src/context/selectors.js @@ -327,6 +327,16 @@ export const selectProfile = createSelector( (profileMap, profileId) => profileMap[profileId] ); +export const selectActiveOrInactiveProfile = (state, profileId) => { + // See if the profile is active, return it if so. + const sender = selectProfile(state, profileId); + if (sender) return sender; + + // The profile is inactive or does not exist. Check terminated members. + const terminatedMap = selectProfileMapForTerminatedMembers(state); + return terminatedMap[profileId]; +}; + export const selectSkill = createSelector( selectSkills, (state, skillId) => skillId, From 3b73a48f28ee43346157de9494b8ae5fb62ec0ba Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Mon, 17 Feb 2025 10:24:13 -0600 Subject: [PATCH 2/5] Added a test for KudosCard. --- .../components/kudos_card/KudosCard.test.jsx | 129 +++++++ .../__snapshots__/KudosCard.test.jsx.snap | 314 ++++++++++++++++++ 2 files changed, 443 insertions(+) create mode 100644 web-ui/src/components/kudos_card/KudosCard.test.jsx create mode 100644 web-ui/src/components/kudos_card/__snapshots__/KudosCard.test.jsx.snap diff --git a/web-ui/src/components/kudos_card/KudosCard.test.jsx b/web-ui/src/components/kudos_card/KudosCard.test.jsx new file mode 100644 index 000000000..44589a747 --- /dev/null +++ b/web-ui/src/components/kudos_card/KudosCard.test.jsx @@ -0,0 +1,129 @@ +import React from 'react'; +import KudosCard from './KudosCard'; +import { AppContextProvider } from '../../context/AppContext'; + +const initialState = { + state: { + csrf: 'O_3eLX2-e05qpS_yOeg1ZVAs9nDhspEi', + teams: [], + userProfile: { + id: "1", + firstName: 'Jimmy', + lastName: 'Johnson', + role: ['MEMBER'], + }, + terminatedMembers: [ + { + id: "5", + firstName: 'Jerry', + lastName: 'Garcia', + name: 'Jerry Garcia', + role: ['MEMBER'], + }, + ], + memberProfiles: [ + { + id: "1", + firstName: 'Jimmy', + lastName: 'Johnson', + name: 'Jimmy Johnson', + role: ['MEMBER'], + }, + { + id: "2", + firstName: 'Jimmy', + lastName: 'Olsen', + name: 'Jimmy Olsen', + role: ['MEMBER'], + }, + { + id: "3", + firstName: 'Clark', + lastName: 'Kent', + name: 'Clark Kent', + role: ['MEMBER'], + }, + { + id: "4", + firstName: 'Kent', + lastName: 'Brockman', + name: 'Kent Brockman', + role: ['MEMBER'], + }, + { + id: "6", + firstName: 'Brock', + lastName: 'Smith', + name: 'Brock Smith', + role: ['MEMBER'], + }, + { + id: "7", + firstName: 'Jimmy', + middleName: 'T.', + lastName: 'Olsen', + name: 'Jimmy T. Olsen', + role: ['MEMBER'], + }, + ], + } +}; + +const kudos = { + id: 'test-kudos', + message: "Brock and Brockman did a great job helping Clark, Jimmy Olsen, Jimmy T. Olsen, and Johnson", + senderId: "5", + dateCreated: [ 2025, 2, 14 ], + recipientMembers: [ + { + id: "1", + firstName: 'Jimmy', + lastName: 'Johnson', + role: ['MEMBER'], + }, + { + id: "2", + firstName: 'Jimmy', + lastName: 'Olsen', + role: ['MEMBER'], + }, + { + id: "3", + firstName: 'Clark', + lastName: 'Kent', + role: ['MEMBER'], + }, + { + id: "6", + firstName: 'Brock', + lastName: 'Smith', + role: ['MEMBER'], + }, + { + id: "4", + firstName: 'Kent', + lastName: 'Brockman', + role: ['MEMBER'], + }, + { + id: "7", + firstName: 'Jimmy', + middleName: 'T.', + lastName: 'Olsen', + role: ['MEMBER'], + }, + ], +}; + +it('renders correctly', () => { + snapshot( + + {}} + /> + + ); +}); diff --git a/web-ui/src/components/kudos_card/__snapshots__/KudosCard.test.jsx.snap b/web-ui/src/components/kudos_card/__snapshots__/KudosCard.test.jsx.snap new file mode 100644 index 000000000..1ae29a9bf --- /dev/null +++ b/web-ui/src/components/kudos_card/__snapshots__/KudosCard.test.jsx.snap @@ -0,0 +1,314 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[`renders correctly 1`] = ` +
+
+
+
+
+
+ +
+
+ +
+
+ +
+
+ +
+
+ +
+
+ +
+
+

+ received kudos from +

+
+
+ +
+ + Jerry Garcia + +
+
+
+
+ + + +
+
+
+
+
+
+
+
+

+ Brock and Brockman did a great job helping Clark, Jimmy Olsen, Jimmy T. Olsen, and Johnson +

+
+

+ Members: +

+
+
+ +
+ + Jimmy Johnson + +
+
+
+ +
+ + Jimmy Olsen + +
+
+
+ +
+ + Clark Kent + +
+
+
+ +
+ + Brock Smith + +
+
+
+ +
+ + Kent Brockman + +
+
+
+ +
+ + Jimmy Olsen + +
+
+
+
+
+
+
+
+`; From f7a6851fe759d0c8bb404b239fdf1c9d87f4d7db Mon Sep 17 00:00:00 2001 From: Chad Elliott Date: Mon, 17 Feb 2025 10:32:51 -0600 Subject: [PATCH 3/5] Updated the KudosCard test to include an active and an inactive sender. --- .../components/kudos_card/KudosCard.test.jsx | 34 +++- .../__snapshots__/KudosCard.test.jsx.snap | 157 +++++++++++++++++- 2 files changed, 187 insertions(+), 4 deletions(-) diff --git a/web-ui/src/components/kudos_card/KudosCard.test.jsx b/web-ui/src/components/kudos_card/KudosCard.test.jsx index 44589a747..f411e0441 100644 --- a/web-ui/src/components/kudos_card/KudosCard.test.jsx +++ b/web-ui/src/components/kudos_card/KudosCard.test.jsx @@ -69,8 +69,8 @@ const initialState = { } }; -const kudos = { - id: 'test-kudos', +const terminated = { + id: 'test-terminated-kudos', message: "Brock and Brockman did a great job helping Clark, Jimmy Olsen, Jimmy T. Olsen, and Johnson", senderId: "5", dateCreated: [ 2025, 2, 14 ], @@ -115,7 +115,35 @@ const kudos = { ], }; -it('renders correctly', () => { +const kudos = { + id: 'test-kudos', + message: "Jimmy is awesome!", + senderId: "1", + dateCreated: [ 2025, 2, 17 ], + recipientMembers: [ + { + id: "2", + firstName: 'Jimmy', + lastName: 'Olsen', + role: ['MEMBER'], + }, + ], +}; + +it('inactive renders correctly', () => { + snapshot( + + {}} + /> + + ); +}); + +it('active renders correctly', () => { snapshot( +
+
+
+
+
+ +
+ + Jimmy Olsen + +
+

+ received kudos from +

+
+
+ +
+ + Jimmy Johnson + +
+
+
+
+ + + +
+
+
+
+
+
+
+
+

+ Jimmy is awesome! +

+
+
+
+
+
+ +`; + +exports[`inactive renders correctly 1`] = `
Date: Mon, 17 Feb 2025 15:57:16 -0600 Subject: [PATCH 4/5] Converted selectActiveOrInactiveProfile to a standard selector --- web-ui/src/context/selectors.js | 17 +++---- web-ui/src/context/selectors.test.js | 75 +++++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 11 deletions(-) diff --git a/web-ui/src/context/selectors.js b/web-ui/src/context/selectors.js index e7ad32f64..4c046af60 100644 --- a/web-ui/src/context/selectors.js +++ b/web-ui/src/context/selectors.js @@ -1,7 +1,7 @@ import { createSelector } from 'reselect'; export const selectMemberProfiles = state => state.memberProfiles || []; -export const selectTerminatedMembers = state => state.terminatedMembers; +export const selectTerminatedMembers = state => state.terminatedMembers || []; export const selectMemberSkills = state => state.memberSkills || []; export const selectSkills = state => state.skills || []; export const selectTeamMembers = state => state.teamMembers; @@ -327,15 +327,12 @@ export const selectProfile = createSelector( (profileMap, profileId) => profileMap[profileId] ); -export const selectActiveOrInactiveProfile = (state, profileId) => { - // See if the profile is active, return it if so. - const sender = selectProfile(state, profileId); - if (sender) return sender; - - // The profile is inactive or does not exist. Check terminated members. - const terminatedMap = selectProfileMapForTerminatedMembers(state); - return terminatedMap[profileId]; -}; +export const selectActiveOrInactiveProfile = createSelector( + selectProfileMap, + selectProfileMapForTerminatedMembers, + (state, profileId) => profileId, + (profileMap, termedProfileMap, profileId) => profileMap[profileId] || termedProfileMap[profileId] +); export const selectSkill = createSelector( selectSkills, diff --git a/web-ui/src/context/selectors.test.js b/web-ui/src/context/selectors.test.js index 0ba8e98cd..3b8b0d25f 100644 --- a/web-ui/src/context/selectors.test.js +++ b/web-ui/src/context/selectors.test.js @@ -20,7 +20,8 @@ import { selectSupervisorHierarchyIds, selectSubordinates, selectIsSubordinateOfCurrentUser, - selectHasReportPermission + selectHasReportPermission, + selectActiveOrInactiveProfile } from './selectors'; describe('Selectors', () => { @@ -1525,4 +1526,76 @@ describe('Selectors', () => { expect(selectHasReportPermission(testState)).toBe(false); }); + + it('selectActiveOrInactiveProfile should a profile if active or inactive', () => { + const activeTestMember = { + id: 1, + bioText: 'foo', + employeeId: 11, + name: 'A Person', + firstName: 'A', + lastName: 'PersonA', + location: 'St Louis', + title: 'engineer', + workEmail: 'employee@sample.com', + pdlId: 9, + startDate: [2012, 9, 29], + }; + const inactiveTestMember = { + id: 2, + bioText: 'foo', + employeeId: 12, + name: 'B Person', + firstName: 'B', + lastName: 'PersonB', + location: 'St Louis', + title: 'engineer', + workEmail: 'employee@sample.com', + pdlId: 9, + startDate: [2012, 9, 29], + terminationDate: [2013, 9, 29], + }; + /** @type MemberProfile[] */ + const testActiveMemberProfiles = [ + activeTestMember, + { + id: 3, + bioText: 'foo', + employeeId: 13, + name: 'C Person', + firstName: 'C', + lastName: 'PersonC', + location: 'St Louis', + title: 'engineer', + workEmail: 'employee@sample.com', + pdlId: 9, + startDate: [2012, 9, 29], + } + ]; + /** @type MemberProfile[] */ + const testInactiveMemberProfiles = [ + inactiveTestMember, + { + id: 4, + bioText: 'foo', + employeeId: 13, + name: 'D Person', + firstName: 'D', + lastName: 'PersonD', + location: 'St Louis', + title: 'engineer', + workEmail: 'employee@sample.com', + pdlId: 9, + startDate: [2012, 9, 29], + terminationDate: [2013, 9, 29], + } + ]; + const testState = { + memberProfiles: testActiveMemberProfiles, + terminatedMembers: testInactiveMemberProfiles, + }; + + expect(selectActiveOrInactiveProfile(testState, activeTestMember.id)).toEqual(activeTestMember); + expect(selectActiveOrInactiveProfile(testState, inactiveTestMember.id)).toEqual(inactiveTestMember); + }); }); From 652deda02334558418ae0017ace101a0873780ba Mon Sep 17 00:00:00 2001 From: Michael Kimberlin Date: Mon, 17 Feb 2025 16:44:10 -0600 Subject: [PATCH 5/5] Added permission for view terminated profile and adjusted app context to respect it --- .../services/permissions/Permission.java | 1 + web-ui/src/context/AppContext.jsx | 4 +- web-ui/src/context/selectors.js | 8 ++ web-ui/src/context/selectors.test.js | 83 ++++++++++++++++++- 4 files changed, 93 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java b/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java index ba203ca9a..00ddde2fb 100644 --- a/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java +++ b/server/src/main/java/com/objectcomputing/checkins/services/permissions/Permission.java @@ -17,6 +17,7 @@ public enum Permission { CAN_ADMINISTER_FEEDBACK_ANSWER("Administer feedback answers", "Feedback"), CAN_ADMINISTER_FEEDBACK_TEMPLATES("Administer feedback templates", "Feedback"), CAN_SEND_EMAIL("Send email", "Feedback"), + CAN_VIEW_TERMINATED_MEMBERS("Can view the profiles of terminated members", "User Management"), CAN_EDIT_ALL_ORGANIZATION_MEMBERS("Edit all member profiles", "User Management"), CAN_DELETE_ORGANIZATION_MEMBERS("Delete organization members", "User Management"), CAN_CREATE_ORGANIZATION_MEMBERS("Create organization members", "User Management"), diff --git a/web-ui/src/context/AppContext.jsx b/web-ui/src/context/AppContext.jsx index 5dd175260..78481c04e 100644 --- a/web-ui/src/context/AppContext.jsx +++ b/web-ui/src/context/AppContext.jsx @@ -23,7 +23,7 @@ import { } from '../api/member'; import { selectCanViewCheckinsPermission, - selectCanEditAllOrganizationMembers, + selectCanViewTerminatedMembers, } from './selectors'; import { getAllRoles, getAllUserRoles } from '../api/roles'; import { getMemberSkills } from '../api/memberskill'; @@ -191,7 +191,7 @@ const AppContextProvider = props => { if (csrf && userProfile && !memberProfiles) { dispatch({ type: UPDATE_PEOPLE_LOADING, payload: true }); getMemberProfiles(); - if (selectCanEditAllOrganizationMembers(state)) { + if (selectCanViewTerminatedMembers(state)) { getTerminatedMembers(); } } diff --git a/web-ui/src/context/selectors.js b/web-ui/src/context/selectors.js index 4c046af60..27d53c1c1 100644 --- a/web-ui/src/context/selectors.js +++ b/web-ui/src/context/selectors.js @@ -242,6 +242,14 @@ export const selectCanEditAllOrganizationMembers = hasPermission( 'CAN_EDIT_ALL_ORGANIZATION_MEMBERS', ); +export const selectCanViewTerminatedMembers = createSelector( + selectCanEditAllOrganizationMembers, + hasPermission( + 'CAN_VIEW_TERMINATED_MEMBERS' + ), + (canEdit, canView) => canEdit || canView +); + export const selectIsPDL = createSelector( selectUserProfile, userProfile => diff --git a/web-ui/src/context/selectors.test.js b/web-ui/src/context/selectors.test.js index 3b8b0d25f..8f6007702 100644 --- a/web-ui/src/context/selectors.test.js +++ b/web-ui/src/context/selectors.test.js @@ -21,7 +21,9 @@ import { selectSubordinates, selectIsSubordinateOfCurrentUser, selectHasReportPermission, - selectActiveOrInactiveProfile + selectActiveOrInactiveProfile, + selectCanEditAllOrganizationMembers, + selectCanViewTerminatedMembers, } from './selectors'; describe('Selectors', () => { @@ -1527,6 +1529,85 @@ describe('Selectors', () => { expect(selectHasReportPermission(testState)).toBe(false); }); + it("selectCanEditAllOrganizationMembers should return false when user does not have 'CAN_EDIT_ALL_ORGANIZATION_MEMBERS' permission", () => { + const testState = { + userProfile: { + firstName: 'Huey', + lastName: 'Emmerich', + role: 'MEMBER', + permissions: [ + { permission: 'CAN_VIEW_FEEDBACK_REQUEST' }, + { permission: 'CAN_VIEW_FEEDBACK_ANSWER' }, + ] + } + }; + + expect(selectCanEditAllOrganizationMembers(testState)).toBe(false); + }); + + it("selectCanEditAllOrganizationMembers should return true when user has 'CAN_EDIT_ALL_ORGANIZATION_MEMBERS' permission", () => { + const testState = { + userProfile: { + firstName: 'Huey', + lastName: 'Emmerich', + role: 'MEMBER', + permissions: [ + { permission: 'CAN_VIEW_FEEDBACK_REQUEST' }, + { permission: 'CAN_EDIT_ALL_ORGANIZATION_MEMBERS' }, + { permission: 'CAN_VIEW_FEEDBACK_ANSWER' }, + ] + } + }; + + expect(selectCanEditAllOrganizationMembers(testState)).toBe(true); + }); + + it("selectCanViewTerminatedMembers should return false when user does not have 'CAN_EDIT_ALL_ORGANIZATION_MEMBERS' or 'CAN_VIEW_TERMINATED_MEMBERS' permission", () => { + const testState = { + userProfile: { + firstName: 'Huey', + lastName: 'Emmerich', + role: 'MEMBER', + permissions: [ + { permission: 'CAN_VIEW_FEEDBACK_REQUEST' }, + { permission: 'CAN_VIEW_FEEDBACK_ANSWER' }, + ] + } + }; + + expect(selectCanViewTerminatedMembers(testState)).toBe(false); + }); + + it("selectCanViewTerminatedMembers should return true when user has 'CAN_EDIT_ALL_ORGANIZATION_MEMBERS' or 'CAN_VIEW_TERMINATED_MEMBERS' permissions", () => { + const testState = { + userProfile: { + firstName: 'Huey', + lastName: 'Emmerich', + role: 'MEMBER', + permissions: [ + { permission: 'CAN_VIEW_FEEDBACK_REQUEST' }, + { permission: 'CAN_EDIT_ALL_ORGANIZATION_MEMBERS' }, + { permission: 'CAN_VIEW_FEEDBACK_ANSWER' }, + ] + } + }; + const otherTestState = { + userProfile: { + firstName: 'Huey', + lastName: 'Emmerich', + role: 'MEMBER', + permissions: [ + { permission: 'CAN_VIEW_FEEDBACK_REQUEST' }, + { permission: 'CAN_VIEW_TERMINATED_MEMBERS' }, + { permission: 'CAN_VIEW_FEEDBACK_ANSWER' }, + ] + } + }; + + expect(selectCanViewTerminatedMembers(testState)).toBe(true); + expect(selectCanViewTerminatedMembers(otherTestState)).toBe(true); + }); + it('selectActiveOrInactiveProfile should a profile if active or inactive', () => { const activeTestMember = { id: 1,