From 580981e1549b1b30aca02346dfbe9f9911d1ac4c Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 30 Oct 2025 10:33:16 -0400 Subject: [PATCH] refactor(dirt): remove newApplications validation from OrganizationReportSummary type guard Removes redundant newApplications field validation from the OrganizationReportSummary type guard and related test cases. **Changes:** - Remove "newApplications" from allowed keys in isOrganizationReportSummary() - Remove newApplications array validation logic - Remove newApplications validation from validateOrganizationReportSummary() - Remove 2 test cases for newApplications validation - Remove newApplications field from 8 test data objects **Rationale:** The newApplications field was removed from OrganizationReportSummary type definition because it's derived data that can be calculated from applicationData (filtering where reviewedDate === null). The data is now accessed via the reactive newApplications$ observable instead of being stored redundantly in the summary object. **Impact:** - No functional changes - UI continues to display new applications via observable - Type guard now correctly validates the actual OrganizationReportSummary structure - Eliminates data redundancy and maintains single source of truth - All 43 tests passing --- .../domain/risk-insights-type-guards.spec.ts | 45 ------------------- .../domain/risk-insights-type-guards.ts | 11 +---- 2 files changed, 1 insertion(+), 55 deletions(-) diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.spec.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.spec.ts index 32505088818..92d3ae3d8fc 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.spec.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.spec.ts @@ -158,7 +158,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1", "app-2"], }; expect(() => validateOrganizationReportSummary(validData)).not.toThrow(); @@ -174,7 +173,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], }; expect(() => validateOrganizationReportSummary(invalidData)).toThrow( @@ -186,7 +184,6 @@ describe("Risk Insights Type Guards", () => { const invalidData = { totalMemberCount: 10, // missing multiple fields - newApplications: ["app-1"], }; expect(() => validateOrganizationReportSummary(invalidData)).toThrow( @@ -204,43 +201,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], - }; - - expect(() => validateOrganizationReportSummary(invalidData)).toThrow( - /Invalid OrganizationReportSummary/, - ); - }); - - it("should throw error for non-array newApplications", () => { - const invalidData = { - totalMemberCount: 10, - totalApplicationCount: 5, - totalAtRiskMemberCount: 2, - totalAtRiskApplicationCount: 1, - totalCriticalApplicationCount: 3, - totalCriticalMemberCount: 4, - totalCriticalAtRiskMemberCount: 1, - totalCriticalAtRiskApplicationCount: 1, - newApplications: "not-an-array", - }; - - expect(() => validateOrganizationReportSummary(invalidData)).toThrow( - /Invalid OrganizationReportSummary.*newApplications/, - ); - }); - - it("should throw error for empty string in newApplications", () => { - const invalidData = { - totalMemberCount: 10, - totalApplicationCount: 5, - totalAtRiskMemberCount: 2, - totalAtRiskApplicationCount: 1, - totalCriticalApplicationCount: 3, - totalCriticalMemberCount: 4, - totalCriticalAtRiskMemberCount: 1, - totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1", "", "app-3"], // empty string }; expect(() => validateOrganizationReportSummary(invalidData)).toThrow( @@ -541,7 +501,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], }; expect(isOrganizationReportSummary(validData)).toBe(true); }); @@ -556,7 +515,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], }; expect(isOrganizationReportSummary(invalidData)).toBe(false); }); @@ -571,7 +529,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], }; expect(isOrganizationReportSummary(invalidData)).toBe(false); }); @@ -586,7 +543,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], }; expect(isOrganizationReportSummary(invalidData)).toBe(false); }); @@ -601,7 +557,6 @@ describe("Risk Insights Type Guards", () => { totalCriticalMemberCount: 4, totalCriticalAtRiskMemberCount: 1, totalCriticalAtRiskApplicationCount: 1, - newApplications: ["app-1"], extraField: "should be rejected", }; expect(isOrganizationReportSummary(invalidData)).toBe(false); diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.ts index b1d2550d4fa..597afee0a84 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/domain/risk-insights-type-guards.ts @@ -179,7 +179,6 @@ export function isOrganizationReportSummary(obj: any): obj is OrganizationReport "totalCriticalMemberCount", "totalCriticalAtRiskMemberCount", "totalCriticalAtRiskApplicationCount", - "newApplications", ]; const actualKeys = Object.keys(obj); const hasUnexpectedProps = actualKeys.some((key) => !allowedKeys.includes(key)); @@ -227,12 +226,7 @@ export function isOrganizationReportSummary(obj: any): obj is OrganizationReport Number.isFinite(obj.totalCriticalAtRiskApplicationCount) && Number.isSafeInteger(obj.totalCriticalAtRiskApplicationCount) && obj.totalCriticalAtRiskApplicationCount >= 0 && - obj.totalCriticalAtRiskApplicationCount <= MAX_COUNT && - Array.isArray(obj.newApplications) && - obj.newApplications.length <= MAX_ARRAY_LENGTH && - obj.newApplications.every( - (app: any) => typeof app === "string" && app.length > 0 && app.length <= MAX_STRING_LENGTH, - ) + obj.totalCriticalAtRiskApplicationCount <= MAX_COUNT ); } @@ -344,9 +338,6 @@ export function validateOrganizationReportSummary(data: any): OrganizationReport if (typeof data?.totalCriticalAtRiskApplicationCount !== "number") { missingFields.push("totalCriticalAtRiskApplicationCount (number)"); } - if (!Array.isArray(data?.newApplications)) { - missingFields.push("newApplications (string[])"); - } throw new Error( `Invalid OrganizationReportSummary: ${missingFields.length > 0 ? `missing or invalid fields: ${missingFields.join(", ")}` : "structure validation failed"}`,