Skip to content

Commit e4fe329

Browse files
nabramovitznorman-abramovitz
authored andcommitted
Fix strict-sweep behavior regressions caught by unit tests (#5446)
The strict sweep changed a few runtime values to satisfy types; restore the original behavior instead (verified against the existing specs): - row-state: getDefaultRowState message restored to null (was changed to undefined); RowState.message typed string | null. - cf-user roles: createUserRoleInOrg/InSpace args made optional so an unset role stays undefined (was changed to false, which the role-change diff would treat as an explicit removal). Restored undefined defaults. - auth-data: applySessionScopes is called unconditionally again (it no-ops on an absent user); widened its param to SessionUser | undefined. A guard had stopped it firing when the verified session carried no user. - deploy-app wizard: kept the idiomatic undefined for the initial unset applicationSource/sourceType (functionally identical to the old null for these optional fields — every consumer uses ?./filter(!!x)); updated the two spec assertions that encoded the ngrx-era null. Verified: ng build clean; store + cloud-foundry + core test projects all green (2204 tests, 0 failures).
1 parent 1f9b924 commit e4fe329

6 files changed

Lines changed: 22 additions & 17 deletions

File tree

src/frontend/packages/cloud-foundry/src/services/domain-data/cf-deploy-app-data.service.spec.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@ describe('CfDeployAppDataService', () => {
2626

2727
it('starts with the default empty wizard state', () => {
2828
expect(svc.cfDetails()).toBeNull();
29-
expect(svc.sourceType()).toBeNull();
30-
expect(svc.applicationSource()).toEqual({ type: null });
29+
// Optional wizard fields start unset (undefined) until the user picks a source.
30+
expect(svc.sourceType()).toBeUndefined();
31+
expect(svc.applicationSource()).toBeUndefined();
3132
expect(svc.projectExists()).toEqual({ checking: false, exists: false, error: false, name: '' });
3233
});
3334

@@ -52,7 +53,7 @@ describe('CfDeployAppDataService', () => {
5253
svc.setCfDetails({ cloudFoundry: 'cf-1', org: 'org-1', space: 'space-1' });
5354
svc.resetState();
5455
expect(svc.cfDetails()).toBeNull();
55-
expect(svc.sourceType()).toBeNull();
56+
expect(svc.sourceType()).toBeUndefined();
5657
});
5758

5859
// The legacy CheckProjectExists action + DeployAppEffects pipeline is

src/frontend/packages/cloud-foundry/src/services/domain-data/cf-users-roles-data.service.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ export function createDefaultOrgRoles(orgGuid: string, orgName: string): IUserPe
333333
return {
334334
name: orgName,
335335
orgGuid,
336-
permissions: createUserRoleInOrg(false, false, false, false),
336+
permissions: createUserRoleInOrg(undefined, undefined, undefined, undefined),
337337
spaces: {},
338338
};
339339
}
@@ -344,7 +344,7 @@ export function createDefaultSpaceRoles(orgGuid: string, orgName: string, spaceG
344344
spaceGuid,
345345
orgGuid,
346346
orgName,
347-
permissions: createUserRoleInSpace(false, false, false),
347+
permissions: createUserRoleInSpace(undefined, undefined, undefined),
348348
};
349349
}
350350

src/frontend/packages/cloud-foundry/src/store/types/cf-user.types.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,12 @@ export class UserRoleInOrg {
9696
* UserRoleInOrg, thus can create roles without this workaround function. See
9797
* https://github.com/Microsoft/TypeScript/wiki/What%27s-new-in-TypeScript#constant-named-properties for details
9898
*/
99-
export function createUserRoleInOrg(manager: boolean, billingManager: boolean, auditor: boolean, user: boolean): UserRoleInOrg {
100-
const res = {} as Record<OrgUserRoleNames, boolean>;
99+
export function createUserRoleInOrg(
100+
manager?: boolean, billingManager?: boolean, auditor?: boolean, user?: boolean
101+
): UserRoleInOrg {
102+
// Args are optional: an unset role stays `undefined` (distinct from an
103+
// explicit `false`, which the role-change diff would treat as a removal).
104+
const res = {} as Record<OrgUserRoleNames, boolean | undefined>;
101105
res[OrgUserRoleNames.MANAGER] = manager;
102106
res[OrgUserRoleNames.BILLING_MANAGERS] = billingManager;
103107
res[OrgUserRoleNames.AUDITOR] = auditor;
@@ -143,8 +147,10 @@ export interface UserRoleInSpace {
143147
* https://github.com/Microsoft/TypeScript/wiki/What%27s-new-in-TypeScript#constant-named-properties for details
144148
*
145149
*/
146-
export function createUserRoleInSpace(manager: boolean, auditor: boolean, developer: boolean): UserRoleInSpace {
147-
const res = {} as Record<SpaceUserRoleNames, boolean>;
150+
export function createUserRoleInSpace(manager?: boolean, auditor?: boolean, developer?: boolean): UserRoleInSpace {
151+
// Args are optional: an unset role stays `undefined` (distinct from an
152+
// explicit `false`, which the role-change diff would treat as a removal).
153+
const res = {} as Record<SpaceUserRoleNames, boolean | undefined>;
148154
res[SpaceUserRoleNames.MANAGER] = manager;
149155
res[SpaceUserRoleNames.DEVELOPER] = developer;
150156
res[SpaceUserRoleNames.AUDITOR] = auditor;

src/frontend/packages/core/src/shared/components/signal-list/row-state.types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ export interface RowsState {
55
export interface RowState {
66
busy?: boolean;
77
error?: boolean;
8-
message?: string;
8+
message?: string | null;
99
blocked?: boolean;
1010
highlighted?: boolean;
1111
deleting?: boolean;
@@ -19,5 +19,5 @@ export const getDefaultRowState = (): RowState => ({
1919
error: false,
2020
blocked: false,
2121
deleting: false,
22-
message: undefined
22+
message: null
2323
});

src/frontend/packages/store/src/services/auth-data.service.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -247,11 +247,9 @@ export class AuthDataService {
247247
// signal source of truth (replaces the former SESSION_VERIFIED reducer
248248
// case). CF endpoint admin scopes are propagated separately by
249249
// CfEndpointRoleSyncService observing the sessionData signal.
250-
// applySessionScopes no-ops on an absent user; guard here so the optional
251-
// sessionData.user matches the collaborator's required param.
252-
if (sessionData.user) {
253-
this.rolesData.applySessionScopes(sessionData.user);
254-
}
250+
// applySessionScopes accepts an absent user (it no-ops internally); call
251+
// it unconditionally so verification always feeds the roles slice.
252+
this.rolesData.applySessionScopes(sessionData.user);
255253
this.patch({
256254
error: false,
257255
errorResponse: '',

src/frontend/packages/store/src/services/current-user-roles-data.service.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ export class CurrentUserRolesDataService {
7070
* the reducer's `CURRENT_USER_ROLES_SESSION_VERIFIED` case
7171
* (`applyInternalScopes`). No-op on internal roles when `user` is absent.
7272
*/
73-
applySessionScopes(user: SessionUser): void {
73+
applySessionScopes(user: SessionUser | undefined): void {
7474
if (!user) {
7575
return;
7676
}

0 commit comments

Comments
 (0)