From fea1fe68b7b8e4b218461e0068c98b9e0aa1bba1 Mon Sep 17 00:00:00 2001 From: Artjoms Porss Date: Thu, 15 Aug 2024 00:53:47 +0100 Subject: [PATCH] ATHENS-7451 - fix red role review icon conditions (#2684) Signed-off-by: aporss --- .../__tests__/components/role/RoleRow.test.js | 118 +++++++++++++++++- .../role/__snapshots__/RoleRow.test.js.snap | 3 - .../role/__snapshots__/RoleTable.test.js.snap | 6 - ui/src/components/constants/constants.js | 2 + ui/src/components/role/RoleRow.js | 48 ++++++- 5 files changed, 159 insertions(+), 18 deletions(-) diff --git a/ui/src/__tests__/components/role/RoleRow.test.js b/ui/src/__tests__/components/role/RoleRow.test.js index 66b746379c5..dc8b5476912 100644 --- a/ui/src/__tests__/components/role/RoleRow.test.js +++ b/ui/src/__tests__/components/role/RoleRow.test.js @@ -14,15 +14,19 @@ * limitations under the License. */ import React from 'react'; -import RoleRow from '../../../components/role/RoleRow'; +import RoleRow, { getSmallestExpiryOrReview, isReviewRequired } from '../../../components/role/RoleRow'; +import { _ } from 'lodash'; import { colors } from '../../../components/denali/styles'; import { renderWithRedux } from '../../../tests_utils/ComponentsTestUtils'; -import { fireEvent, screen, waitFor } from '@testing-library/react'; -import { configure } from '@testing-library/dom'; -import { act } from 'react-dom/test-utils'; -import { USER_DOMAIN } from '../../../components/constants/constants'; +import { fireEvent, screen } from '@testing-library/react'; +import moment from 'moment'; + +describe('RoleRow', (object, method) => { + + afterAll(() => { + jest.clearAllMocks(); + }); -describe('RoleRow', () => { it('should render', () => { const details = { name: 'athens:role.ztssia_cert_rotate', @@ -79,4 +83,106 @@ describe('RoleRow', () => { fireEvent.mouseEnter(descriptionIcon); await screen.findByText('test description'); }); + + it('getSmallestExpiryOrReview check memberExpiryDays value is picked up', async () => { + const role = {memberExpiryDays: 5, lastReviewedDate: '2024-07-24T10:58:07.533Z'}; + let actualDays = getSmallestExpiryOrReview(role); + + expect(_.isEqual(role.memberExpiryDays, actualDays)).toBeTruthy(); + }); + + it('getSmallestExpiryOrReview check serviceExpiryDays value is picked up', async () => { + const role = {serviceExpiryDays: 9, lastReviewedDate: '2024-07-24T10:58:07.533Z'}; + let actualDays = getSmallestExpiryOrReview(role); + + expect(_.isEqual(role.serviceExpiryDays, actualDays)).toBeTruthy(); + }); + + it('getSmallestExpiryOrReview check memberReviewDays value is picked up', async () => { + const role = {memberReviewDays: 6, lastReviewedDate: '2024-07-24T10:58:07.533Z'}; + let actualDays = getSmallestExpiryOrReview(role); + + expect(_.isEqual(role.memberReviewDays, actualDays)).toBeTruthy(); + }); + + it('getSmallestExpiryOrReview check serviceReviewDays value is picked up', async () => { + const role = {serviceReviewDays: 10, lastReviewedDate: '2024-07-24T10:58:07.533Z'}; + let actualDays = getSmallestExpiryOrReview(role); + + expect(_.isEqual(role.serviceReviewDays, actualDays)).toBeTruthy(); + }); + + it('getSmallestExpiryOrReview check groupReviewDays value is picked up', async () => { + const role = {groupReviewDays: 8, lastReviewedDate: '2024-07-24T10:58:07.533Z'}; + let actualDays = getSmallestExpiryOrReview(role); + + expect(_.isEqual(role.groupReviewDays, actualDays)).toBeTruthy(); + }); + + it('getSmallestExpiryOrReview check groupExpiryDays value is picked up', async () => { + const role = {groupExpiryDays: 7, lastReviewedDate: '2024-07-24T10:58:07.533Z'}; + let actualDays = getSmallestExpiryOrReview(role); + + expect(_.isEqual(role.groupExpiryDays, actualDays)).toBeTruthy(); + }); + + it("getSmallestExpiryOrReview check smallest value picked, non 0, null doesn't produce error", async () => { + const role = { + memberExpiryDays: 0, + serviceExpiryDays: null, + memberReviewDays: 6, + serviceReviewDays: 10, + groupReviewDays: 8, + groupExpiryDays: 7, + lastReviewedDate: '2024-07-24T10:58:07.533Z' + }; + let actualDays = getSmallestExpiryOrReview(role); + + expect(_.isEqual(role.memberReviewDays, actualDays)).toBeTruthy(); + }); + + it("getSmallestExpiryOrReview when no expiry or review days assigned to the role, return 0", async () => { + const role = { + lastReviewedDate: '2024-07-24T10:58:07.533Z' + }; + let actualDays = getSmallestExpiryOrReview(role); + + expect(_.isEqual(0, actualDays)).toBeTruthy(); + }); + + it("isReviewRequired when no expiry or review days assigned to the role, return false", async () => { + const role = { + }; + let reviewRequired = isReviewRequired(role); + + expect(_.isEqual(false, reviewRequired)).toBeTruthy(); + }); + + it("isReviewRequired when lastReviewDate is more than 80% of memberExpiryDate ago - return true", async () => { + const role = { + lastReviewedDate: '2024-07-01T11:59:59.000Z', + memberExpiryDays: '10' + }; + // mock current date + jest.spyOn(moment.prototype, 'utc') + .mockReturnValue(moment('2024-07-09T12:00:00.000Z').utc()); + + let reviewRequired = isReviewRequired(role); + + expect(_.isEqual(true, reviewRequired)).toBeTruthy(); + }); + + it("isReviewRequired when lastReviewDate is less than 80% of memberExpiryDate ago - return false", async () => { + const role = { + lastReviewedDate: '2024-07-01T12:00:01.000Z', + memberExpiryDays: '10' + }; + // mock current date + // jest.spyOn(moment.prototype, 'utc') + // .mockReturnValue(moment('2024-07-09T12:00:00.000Z').utc()); + + let reviewRequired = isReviewRequired(role); + + expect(_.isEqual(false, reviewRequired)).toBeTruthy(); + }); }); diff --git a/ui/src/__tests__/components/role/__snapshots__/RoleRow.test.js.snap b/ui/src/__tests__/components/role/__snapshots__/RoleRow.test.js.snap index 732b88bbb14..6f53681b80f 100644 --- a/ui/src/__tests__/components/role/__snapshots__/RoleRow.test.js.snap +++ b/ui/src/__tests__/components/role/__snapshots__/RoleRow.test.js.snap @@ -117,9 +117,6 @@ exports[`RoleRow should render 1`] = ` viewBox="0 0 1024 1024" width="1.25em" > - - assignment-priority - diff --git a/ui/src/__tests__/components/role/__snapshots__/RoleTable.test.js.snap b/ui/src/__tests__/components/role/__snapshots__/RoleTable.test.js.snap index 98dfbc634c2..ae00fd9e4c5 100644 --- a/ui/src/__tests__/components/role/__snapshots__/RoleTable.test.js.snap +++ b/ui/src/__tests__/components/role/__snapshots__/RoleTable.test.js.snap @@ -200,9 +200,6 @@ exports[`RoleTable should render 1`] = ` viewBox="0 0 1024 1024" width="1.25em" > - - assignment-priority - @@ -396,9 +393,6 @@ exports[`RoleTable should render 1`] = ` viewBox="0 0 1024 1024" width="1.25em" > - - assignment-priority - diff --git a/ui/src/components/constants/constants.js b/ui/src/components/constants/constants.js index a12e916c032..3aff9dc7fa2 100644 --- a/ui/src/components/constants/constants.js +++ b/ui/src/components/constants/constants.js @@ -277,3 +277,5 @@ export const ENVIRONMENT_DROPDOWN_OPTIONS = [ export const MEMBER_AUTHORITY_FILTER_DISABLED = 1; export const MEMBER_AUTHORITY_SYSTEM_SUSPENDED = 2; export const MEMBER_ATHENZ_SYSTEM_DISABLED = 4; + +export const ROLE_PERCENTAGE_OF_DAYS_TILL_NEXT_REVIEW = 0.2; diff --git a/ui/src/components/role/RoleRow.js b/ui/src/components/role/RoleRow.js index ae15d7415c9..1bce9cfd758 100644 --- a/ui/src/components/role/RoleRow.js +++ b/ui/src/components/role/RoleRow.js @@ -27,6 +27,8 @@ import { css, keyframes } from '@emotion/react'; import { deleteRole } from '../../redux/thunks/roles'; import { connect } from 'react-redux'; import { selectDomainAuditEnabled } from '../../redux/selectors/domainData'; +import moment from 'moment-timezone'; +import { ROLE_PERCENTAGE_OF_DAYS_TILL_NEXT_REVIEW } from '../constants/constants'; const TDStyledName = styled.div` background-color: ${(props) => props.color}; @@ -93,6 +95,46 @@ const LeftSpan = styled.span` padding-left: 20px; `; +export function isReviewRequired(role) { + // determine last review or last modified date + const reviewData = role.lastReviewedDate ? {lastReviewedDate: role.lastReviewedDate} + : {lastReviewedDate: role.modified}; + // get smallest expiry or review days value for the role + const smallestExpiryOrReview = getSmallestExpiryOrReview(role); + + if (smallestExpiryOrReview === 0) { + // review or expiry days were not set in settings - no review required + return false; + } + + // get 20% of the smallest review period + reviewData.pct20 = Math.ceil(smallestExpiryOrReview * ROLE_PERCENTAGE_OF_DAYS_TILL_NEXT_REVIEW); + + const lastReviewedDate = moment(reviewData.lastReviewedDate, 'YYYY-MM-DDTHH:mm:ss.SSSZ'); + const now = moment().utc(); + + // check if expiry/review is coming up within 20% of the smallest review/expiry period + return now.subtract(smallestExpiryOrReview, 'days').add(reviewData.pct20, 'days').isAfter(lastReviewedDate); +} + +export function getSmallestExpiryOrReview(role){ + const values = [ + role.memberExpiryDays, + role.memberReviewDays, + role.groupExpiryDays, + role.groupReviewDays, + role.serviceExpiryDays, + role.serviceReviewDays + ].filter(obj => obj > 0); // pick only those that have days set and days > 0 + + if (values.length > 0) { + // pick the one with the smallest days value + return values.reduce((obj1, obj2) => obj1 < obj2 ? + obj1 : obj2); + } + return 0; +} + class RoleRow extends React.Component { constructor(props) { super(props); @@ -277,8 +319,7 @@ class RoleRow extends React.Component { ); - let reviewRequired = - role.reviewEnabled && (role.memberExpiryDays || role.serviceExpiry); + let reviewRequired = isReviewRequired(role); let roleTypeIcon = role.trust ? iconDelegated : ''; let roleDescriptionIcon = role.description ? iconDescription : ''; @@ -354,11 +395,12 @@ class RoleRow extends React.Component { isLink size={'1.25em'} verticalAlign={'text-bottom'} + enableTitle={false} /> } > - Review Members + {reviewRequired ? 'Role Review is required' : 'Review Members'}