Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions projects/lib/portal-options/models/luigi-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface PortalNodeContext extends NodeContext {
translationTable?: any;
namespaceId?: string;
entity?: Resource;
entityName?: string;
entityId?: string;
entityContext?: PortalEntityContext;
}
2 changes: 1 addition & 1 deletion projects/lib/portal-options/models/luigi-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ import { PortalNodeContext } from './luigi-context';
import { LuigiNode } from '@openmfp/portal-ui-lib';

export interface PortalLuigiNode extends LuigiNode {
context?: PortalNodeContext;
context: PortalNodeContext;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify that making context required doesn't break existing code.

Changing context from optional to required is a breaking change. All existing code that creates PortalLuigiNode instances must now provide a context property.

Run the following script to verify all usages of PortalLuigiNode provide a context:


🏁 Script executed:

#!/bin/bash
# Description: Find all PortalLuigiNode instantiations and verify they include context property

# Search for object literals typed as PortalLuigiNode or assignments/returns
rg -n -A 10 -B 2 --type=ts 'as PortalLuigiNode|: PortalLuigiNode\[?\]|Promise<PortalLuigiNode'

Length of output: 1339


I need to examine the complete file content and search for all usages of PortalLuigiNode to verify the breaking change impact. Let me gather this information:

Now let me examine the complete file to verify if context is being provided in all PortalLuigiNode instances:


---

🏁 Script executed:

```shell
#!/bin/bash
# Get the complete custom-global-nodes.service.ts file to check if context is provided
cat projects/lib/portal-options/services/custom-global-nodes.service.ts

Length of output: 1827


I found a critical issue! The nested overview object (lines 42-47) does NOT have a context property. Let me search for all other usages of PortalLuigiNode in the codebase to determine the full scope:


---

🏁 Script executed:

```shell
#!/bin/bash
# Search for all PortalLuigiNode usages across the codebase
# Look for type annotations, casting, and interface implementations
rg -n --type=ts 'PortalLuigiNode' -g '!node_modules' | head -50

Length of output: 4488


Let me examine the service implementations and test files to see if they instantiate PortalLuigiNode objects without context:


---

🏁 Script executed:

```shell
#!/bin/bash
# Check node-context-processing.service.spec.ts for PortalLuigiNode instantiations
cat projects/lib/portal-options/services/node-context-processing.service.spec.ts | head -150

Length of output: 4707



---

🏁 Script executed:

```shell
#!/bin/bash
# Get the rest of the test file to see all instantiations
cat projects/lib/portal-options/services/node-context-processing.service.spec.ts | tail -200

Length of output: 6025


Add context property to nested overview node in custom-global-nodes.service.ts.

Making context required is indeed a breaking change. However, the actual breaking point exists in the code itself: the nested overview node object (lines 42-47 in projects/lib/portal-options/services/custom-global-nodes.service.ts) is missing the context property. This will fail TypeScript compilation when the type change is applied. Add context: {} as PortalNodeContext to this nested object to resolve the compilation error.

🤖 Prompt for AI Agents
In projects/lib/portal-options/models/luigi-node.ts around line 5, the
PortalNodeContext property was made required; update the nested overview node in
projects/lib/portal-options/services/custom-global-nodes.service.ts (lines
~42-47) to include a context property by adding context: {} as PortalNodeContext
to that nested object so it satisfies the new required type and restores
TypeScript compilation.

parent?: PortalLuigiNode;
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,26 @@ describe('CrdGatewayKcpPatchResolver', () => {
).toHaveBeenCalledWith(`${kcpRootOrgsPath}:org1:acc1:acc2:acc3`);
});

it('should use kcpPath from node context if provided', async () => {
it('should set kcpPath for node context if not provided', async () => {
const node: PortalLuigiNode = {
context: {},
parent: undefined,
} as any;

await resolver.resolveCrdGatewayKcpPath(node);

expect(node.context.kcpPath).toEqual('root:orgs:org1');
});

it('should not set kcpPath for node context if present', async () => {
const node: PortalLuigiNode = {
context: { kcpPath: 'customPath' },
parent: undefined,
} as any;

await resolver.resolveCrdGatewayKcpPath(node);

expect(
gatewayServiceMock.updateCrdGatewayUrlWithEntityPath,
).toHaveBeenCalledWith('customPath');
expect(node.context.kcpPath).toEqual('customPath');
});

it('should handle node without entity metadata', async () => {
Expand All @@ -84,43 +93,7 @@ describe('CrdGatewayKcpPatchResolver', () => {
).toHaveBeenCalledWith(`${kcpRootOrgsPath}:org1`);
});

describe('resolveCrdGatewayKcpPathForNextAccountEntity', () => {
it('should return early if kind is not Account', async () => {
const nextNode: PortalLuigiNode = {
context: {},
parent: undefined,
} as any;

await resolver.resolveCrdGatewayKcpPathForNextAccountEntity(
'leafAcc',
'Project',
nextNode,
);

expect(
gatewayServiceMock.updateCrdGatewayUrlWithEntityPath,
).not.toHaveBeenCalled();
expect(envConfigServiceMock.getEnvConfig).not.toHaveBeenCalled();
});

it('should return early if entityId is empty', async () => {
const nextNode: PortalLuigiNode = {
context: {},
parent: undefined,
} as any;

await resolver.resolveCrdGatewayKcpPathForNextAccountEntity(
'',
'Account',
nextNode,
);

expect(
gatewayServiceMock.updateCrdGatewayUrlWithEntityPath,
).not.toHaveBeenCalled();
expect(envConfigServiceMock.getEnvConfig).not.toHaveBeenCalled();
});

describe('resolveCrdGatewayKcpPath', () => {
it('should aggregate parent Account entities and append entityId', async () => {
const nextNode: PortalLuigiNode = {
context: {},
Expand All @@ -142,38 +115,12 @@ describe('CrdGatewayKcpPatchResolver', () => {
},
} as any;

await resolver.resolveCrdGatewayKcpPathForNextAccountEntity(
'leafAcc',
'Account',
nextNode,
);
await resolver.resolveCrdGatewayKcpPath(nextNode, 'leafAcc', 'Account');

expect(envConfigServiceMock.getEnvConfig).toHaveBeenCalled();
expect(
gatewayServiceMock.updateCrdGatewayUrlWithEntityPath,
).toHaveBeenCalledWith(`${kcpRootOrgsPath}:org1:acc1:acc2:leafAcc`);
});

it('should use kcpPath from node context if provided (override)', async () => {
const nextNode: PortalLuigiNode = {
context: { kcpPath: 'overridePath' },
parent: {
context: {
entity: { metadata: { name: 'accParent' }, __typename: 'Account' },
},
parent: undefined,
},
} as any;

await resolver.resolveCrdGatewayKcpPathForNextAccountEntity(
'leafAcc',
'Account',
nextNode,
);

expect(
gatewayServiceMock.updateCrdGatewayUrlWithEntityPath,
).toHaveBeenCalledWith('overridePath');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,13 @@ export class CrdGatewayKcpPatchResolver {
private gatewayService = inject(GatewayService);
private envConfigService = inject(EnvConfigService);

public async resolveCrdGatewayKcpPathForNextAccountEntity(
entityId: string,
kind: string,
public async resolveCrdGatewayKcpPath(
nextNode: PortalLuigiNode,
entityId?: string,
kind?: string,
) {
if (kind !== 'Account' || !entityId) {
return;
}

let entityKcpPath = `:${entityId}`;
let node: PortalLuigiNode | undefined = nextNode.parent;

let entityKcpPath = kind !== 'Account' || !entityId ? '' : `:${entityId}`;
let node: PortalLuigiNode | undefined = nextNode;
do {
const entity = node?.context?.entity;
if (entity?.metadata?.name && entity['__typename'] === 'Account') {
Expand All @@ -30,25 +25,11 @@ export class CrdGatewayKcpPatchResolver {
} while (node);

const org = (await this.envConfigService.getEnvConfig()).idpName;
const kcpPath =
nextNode.context?.kcpPath || `${kcpRootOrgsPath}:${org}${entityKcpPath}`;
const kcpPath = `${kcpRootOrgsPath}:${org}${entityKcpPath}`;
this.gatewayService.updateCrdGatewayUrlWithEntityPath(kcpPath);
}

public async resolveCrdGatewayKcpPath(nextNode: PortalLuigiNode) {
let entityKcpPath = '';
let node: PortalLuigiNode | undefined = nextNode;
do {
const entity = node.context?.entity;
if (entity?.metadata?.name && entity['__typename'] === 'Account') {
entityKcpPath = `:${entity.metadata.name}${entityKcpPath}`;
}
node = node.parent;
} while (node);

const org = (await this.envConfigService.getEnvConfig()).idpName;
const kcpPath =
nextNode.context?.kcpPath || `${kcpRootOrgsPath}:${org}${entityKcpPath}`;
this.gatewayService.updateCrdGatewayUrlWithEntityPath(kcpPath);
if (!nextNode.context.kcpPath) {
nextNode.context.kcpPath = kcpPath;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,17 @@ export class CustomGlobalNodesServiceImpl implements CustomGlobalNodesService {
showBreadcrumbs: false,
hideSideNav: true,
hideFromNav: true,
context: {} as PortalNodeContext,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify that an empty context is appropriate for the users node.

The users node is assigned an empty context cast as PortalNodeContext. According to the interface definition, PortalNodeContext extends NodeContext and includes portalContext: PortalContext as a required field. Casting an empty object bypasses type safety and may cause runtime errors if code attempts to access context.portalContext.

Please verify:

  1. Whether portalContext is truly required in PortalNodeContext
  2. If this empty context is populated elsewhere before being accessed
  3. Whether a minimal valid context should be provided instead

Run this script to check how context is accessed for these nodes:


🏁 Script executed:

#!/bin/bash
# Description: Find usages of users node context to verify if portalContext is accessed

# Search for context access patterns in node processing
rg -n -A 5 --type=ts 'users.*context|pathSegment.*users' | head -50

# Check PortalNodeContext interface definition for required fields
ast-grep --pattern 'interface PortalNodeContext {
  $$$
}'

Length of output: 613


I need to search more thoroughly for the interface definition and context usage patterns.The web search returned generic TypeScript interface information. I need to search for the specific PortalNodeContext definition in the codebase instead.


🏁 Script executed:

#!/bin/bash
# Find PortalNodeContext interface definition
rg -n 'interface PortalNodeContext|type PortalNodeContext' --type=ts | head -20

# Also search for NodeContext interface to understand the hierarchy
rg -n 'interface NodeContext|type NodeContext' --type=ts | head -20

# Search for imports of these types
rg -n 'import.*PortalNodeContext' --type=ts | head -20

Length of output: 621


🏁 Script executed:

#!/bin/bash
# Look for how context is accessed on nodes
rg -n 'context\.portalContext|context\[' --type=ts | head -30

# Search for where these nodes are processed/consumed
rg -n 'getCustomGlobalNodes|\.context' projects/lib/portal-options/services/custom-global-nodes.service.ts -A 5 -B 5

Length of output: 441


🏁 Script executed:

#!/bin/bash
# Check if there's any runtime context initialization
rg -n 'portalContext|PortalContext' --type=ts | grep -i 'init\|assign\|set\|populate' | head -20

Length of output: 0


🏁 Script executed:

#!/bin/bash
# View the PortalNodeContext and NodeContext interface definitions
cat projects/lib/portal-options/models/luigi-context.ts

# Also check what NodeContext is (might be from Luigi library)
rg -n 'interface NodeContext|type NodeContext|from.*NodeContext' --type=ts -B 2 -A 10 | head -40

Length of output: 490


🏁 Script executed:

#!/bin/bash
# Check node-context-processing.service.ts to see how contexts are actually used
cat projects/lib/portal-options/services/node-context-processing.service.ts | head -100

Length of output: 2832


🏁 Script executed:

#!/bin/bash
# Search for actual consumption of getCustomGlobalNodes and how contexts are accessed
rg -n 'getCustomGlobalNodes' --type=ts -A 10 -B 2 | head -50

Length of output: 1339


🏁 Script executed:

#!/bin/bash
# View full custom-global-nodes.service.ts to see all node definitions
cat projects/lib/portal-options/services/custom-global-nodes.service.ts

Length of output: 1827


🏁 Script executed:

#!/bin/bash
# Search for where processNodeContext is called to understand which nodes it applies to
rg -n 'processNodeContext' --type=ts -B 5 -A 5 | head -60

Length of output: 6062


Provide valid portalContext for nodes that will be processed by Luigi.

The users node (line 33) and its child :userId node (line 43) have empty contexts cast to PortalNodeContext. The PortalNodeContext interface requires portalContext: PortalContext, which contains crdGatewayApiUrl: string. The :userId node has a defineEntity field, which means Luigi will call processNodeContext during navigation. In node-context-processing.service.ts (lines 48-49), the code directly accesses ctx.portalContext.crdGatewayApiUrl without null checks. An empty context will cause a runtime error: "Cannot read property 'crdGatewayApiUrl' of undefined".

Populate these contexts with at least portalContext, or ensure Luigi has access to the required values before processing these nodes.

🤖 Prompt for AI Agents
In projects/lib/portal-options/services/custom-global-nodes.service.ts around
lines 33 and 43, the users node and its child :userId node are using empty
contexts cast to PortalNodeContext which lacks the required
portalContext.portalContext.crdGatewayApiUrl and will cause a runtime error when
Luigi calls processNodeContext; fix by populating the contexts with at least
portalContext including crdGatewayApiUrl (e.g. context: { portalContext: {
crdGatewayApiUrl: '<your-crd-gateway-url>' } } as PortalNodeContext) or
alternatively ensure the node definitions do not rely on
defineEntity/processNodeContext until a valid portalContext is available (add a
lazy resolver or null-checks before accessing
ctx.portalContext.crdGatewayApiUrl).

entityType: 'global',
children: [
{
context: {
profileUserId: ':profileUserId',
} as unknown as PortalNodeContext,
pathSegment: ':userId',
hideSideNav: true,
hideFromNav: true,
defineEntity: {
id: 'user',
contextKey: 'profileUserId',
},
pathSegment: ':profileUserId',
hideSideNav: true,
hideFromNav: true,
context: {} as PortalNodeContext,
children: [
{
pathSegment: 'overview',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ describe('LuigiExtendedGlobalContextConfigServiceImpl', () => {
kcpCA: 'dW5kZWZpbmVk',
organization: 'test-org',
entityId: 'originClusterId/test-org',
entityName: 'test-org',
});

expect(mockResourceService.readAccountInfo).toHaveBeenCalledWith({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export class LuigiExtendedGlobalContextConfigServiceImpl
organizationId: `${organizationOriginClusterId}/${entityId}`,
kcpCA: btoa(accountInfo?.spec?.clusterInfo?.ca),
entityId: `${organizationOriginClusterId}/${entityId}`, // if no entity selected the entityId is the same as the organizationId
entityName: entityId,
};
} catch (e) {
console.error(`Failed to read entity ${entityId} from ${operation}`, e);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { TestBed } from '@angular/core/testing';
import { Resource } from '@openmfp/portal-ui-lib';
import { ResourceService } from '@platform-mesh/portal-ui-lib/services';
import { of, throwError } from 'rxjs';
import { PortalNodeContext } from '../models/luigi-context';
import { PortalLuigiNode } from '../models/luigi-node';
import { CrdGatewayKcpPatchResolver } from './crd-gateway-kcp-patch-resolver.service';
import { NodeContextProcessingServiceImpl } from './node-context-processing.service';
import { TestBed } from '@angular/core/testing';
import { Resource } from '@openmfp/portal-ui-lib';
import { ResourceService } from '@platform-mesh/portal-ui-lib/services';
import { of, throwError } from 'rxjs';

describe('NodeContextProcessingServiceImpl', () => {
let service: NodeContextProcessingServiceImpl;
Expand All @@ -18,7 +18,7 @@ describe('NodeContextProcessingServiceImpl', () => {
} as unknown as jest.Mocked<ResourceService>;

mockCrdGatewayKcpPatchResolver = {
resolveCrdGatewayKcpPathForNextAccountEntity: jest.fn(),
resolveCrdGatewayKcpPath: jest.fn(),
} as unknown as jest.Mocked<CrdGatewayKcpPatchResolver>;

TestBed.configureTestingModule({
Expand Down Expand Up @@ -65,7 +65,7 @@ describe('NodeContextProcessingServiceImpl', () => {
await service.processNodeContext(entityId, entityNode, ctx);

expect(
mockCrdGatewayKcpPatchResolver.resolveCrdGatewayKcpPathForNextAccountEntity,
mockCrdGatewayKcpPatchResolver.resolveCrdGatewayKcpPath,
).toHaveBeenCalledWith(entityId, 'TestKind', entityNode);
expect(mockResourceService.read).toHaveBeenCalled();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { ResourceService } from '@platform-mesh/portal-ui-lib/services';
import { replaceDotsAndHyphensWithUnderscores } from '@platform-mesh/portal-ui-lib/utils';
import { firstValueFrom } from 'rxjs';


@Injectable({
providedIn: 'root',
})
Expand All @@ -30,10 +29,10 @@ export class NodeContextProcessingServiceImpl
return;
}

await this.crdGatewayKcpPatchResolver.resolveCrdGatewayKcpPathForNextAccountEntity(
await this.crdGatewayKcpPatchResolver.resolveCrdGatewayKcpPath(
entityNode.parent || ({} as PortalLuigiNode),
entityId,
kind,
entityNode,
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Runtime error when parent is undefined.

When entityNode.parent is undefined, the code passes an empty object {} cast as PortalLuigiNode. This empty object lacks the required context property. Subsequently, resolveCrdGatewayKcpPath attempts to set nextNode.context.kcpPath (line 31-33 in the resolver), which will throw a runtime error because context is undefined on the empty object.

Apply this diff to provide a valid fallback with an initialized context:

     await this.crdGatewayKcpPatchResolver.resolveCrdGatewayKcpPath(
-      entityNode.parent || ({} as PortalLuigiNode),
+      entityNode.parent || ({ context: {} } as PortalLuigiNode),
       entityId,
       kind,
     );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await this.crdGatewayKcpPatchResolver.resolveCrdGatewayKcpPath(
entityNode.parent || ({} as PortalLuigiNode),
entityId,
kind,
entityNode,
);
await this.crdGatewayKcpPatchResolver.resolveCrdGatewayKcpPath(
entityNode.parent || ({ context: {} } as PortalLuigiNode),
entityId,
kind,
);
🤖 Prompt for AI Agents
In projects/lib/portal-options/services/node-context-processing.service.ts
around lines 32 to 36, the code currently casts an empty object when
entityNode.parent is undefined, which lacks the required context and causes
resolveCrdGatewayKcpPath to throw; change the fallback to pass a valid
PortalLuigiNode instance whose context property is initialized (for example,
context should be an object with kcpPath initialized to an empty array) so that
nextNode.context.kcpPath can be safely read/assigned.


const operation = replaceDotsAndHyphensWithUnderscores(group);
Expand Down Expand Up @@ -67,12 +66,12 @@ export class NodeContextProcessingServiceImpl

// update the current already calculated by Luigi context for a node
ctx.entity = entity;
ctx.entityName = entityId;
ctx.entityId = `${entity.metadata?.annotations?.['kcp.io/cluster']}/${entityId}`;
// update the node context of sa node to contain the entity for future context calculations
if (entityNode.context) {
entityNode.context.entity = entity;
entityNode.context.entityId = ctx.entityId;
}
entityNode.context.entity = entity;
entityNode.context.entityName = ctx.entityName;
entityNode.context.entityId = ctx.entityId;
} catch (e) {
console.error(`Not able to read entity ${entityId} from ${operation}`);
}
Expand Down
20 changes: 0 additions & 20 deletions projects/lib/services/resource/gateway.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { GatewayService } from './gateway.service';
import { TestBed } from '@angular/core/testing';
import { LuigiCoreService } from '@openmfp/portal-ui-lib';


describe('GatewayService', () => {
let service: GatewayService;
let mockLuigiCoreService: any;
Expand Down Expand Up @@ -85,7 +84,6 @@ describe('GatewayService', () => {
},
token: 'token',
accountId: 'entityId',
kcpPath: ':org1',
};
const result = service.resolveKcpPath(nodeContext, true);
expect(result).toBe(':org1');
Expand Down Expand Up @@ -130,8 +128,6 @@ describe('GatewayService', () => {
});

it('should show error alert and return empty string for invalid URL', () => {
const showAlertSpy = jest.spyOn(mockLuigiCoreService, 'showAlert');

const nodeContext = {
portalContext: {
crdGatewayApiUrl: 'https://example.com/invalid-url',
Expand All @@ -143,15 +139,9 @@ describe('GatewayService', () => {
const result = service.resolveKcpPath(nodeContext);

expect(result).toBe('');
expect(showAlertSpy).toHaveBeenCalledWith({
text: 'Could not get current KCP path from gateway URL',
type: 'error',
});
});

it('should show error alert and return empty string for URL without /graphql suffix', () => {
const showAlertSpy = jest.spyOn(mockLuigiCoreService, 'showAlert');

const nodeContext = {
portalContext: {
crdGatewayApiUrl: 'https://example.com/:org1:acc1/api',
Expand All @@ -163,15 +153,9 @@ describe('GatewayService', () => {
const result = service.resolveKcpPath(nodeContext);

expect(result).toBe('');
expect(showAlertSpy).toHaveBeenCalledWith({
text: 'Could not get current KCP path from gateway URL',
type: 'error',
});
});

it('should show error alert and return empty string for empty URL', () => {
const showAlertSpy = jest.spyOn(mockLuigiCoreService, 'showAlert');

const nodeContext = {
portalContext: {
crdGatewayApiUrl: '',
Expand All @@ -183,10 +167,6 @@ describe('GatewayService', () => {
const result = service.resolveKcpPath(nodeContext);

expect(result).toBe('');
expect(showAlertSpy).toHaveBeenCalledWith({
text: 'Could not get current KCP path from gateway URL',
type: 'error',
});
});
});
});
Loading