Copilot commented on code in PR #8336:
URL: https://github.com/apache/texera/pull/8336#discussion_r3911521098
##########
frontend/src/app/dashboard/service/user/resource-registry/resource-registry.service.spec.ts:
##########
@@ -247,4 +248,36 @@ describe("ResourceRegistryService", () => {
expect(registry.entryLink(entry({ type: EntityType.Dataset, id: undefined
}), 42)).toEqual([]);
expect(registry.entryLink(entry({ type: EntityType.Workflow, id: "draft"
}), 42)).toEqual([]);
});
+
+ /**
+ * `hubRoute` is optional on the descriptor contract, and the shipped kinds
happen to declare
+ * both routes or neither — so nothing had ever asked what a
private-page-only kind links to.
+ * The answer must not depend on the viewer: with nowhere else to send them,
the private page is
+ * the only link there is, and the access check further down would otherwise
route an outsider to
+ * `undefined`. Descriptors reach the registry by injection, so the kind is
supplied as one.
+ */
+ it("links a kind with a private page and no hub page straight to its private
page", () => {
+ TestBed.resetTestingModule();
+ TestBed.configureTestingModule({
+ imports: [HttpClientTestingModule],
+ providers: [
+ { provide: DownloadService, useValue: downloadService },
+ { provide: WorkflowPersistService, useValue: workflowPersistService },
+ { provide: DatasetService, useValue: datasetService },
+ { provide: ModelService, useValue: modelService },
+ {
+ provide: FileResourceDescriptor,
+ useValue: { type: EntityType.File, privateRoute: "/private-files",
isOwner: () => true },
+ },
Review Comment:
The overridden FileResourceDescriptor `useValue` is missing required
descriptor fields (e.g. `iconType`). This works today because `entryLink`
doesn't read them, but it makes the test less robust and can cause runtime
failures if the registry constructor or other code starts touching required
fields.
##########
frontend/src/app/hub/component/browse-section/browse-section.component.spec.ts:
##########
@@ -149,6 +156,58 @@ describe("BrowseSectionComponent", () => {
expect(component.getCoverImage(entity)).toBe(component.defaultBackground);
});
+
+ // `this.resourceRegistry.find(entity.type)?.coverUrl` carries two guards,
and a mixed section
+ // can trip either. A workflow's cover is a data URL carried on the entry
itself, so
+ // WorkflowResourceDescriptor deliberately declares no `coverUrl`; and a
kind the registry does
+ // not carry at all has no descriptor to ask, which is why this is `find`,
not `get` — one such
+ // row must not take the whole section's covers down, exactly as
`routeFor` five lines up
Review Comment:
This block comment is a bit misleading for what the test actually validates:
the relevant condition is that `descriptor?.coverUrl` may be undefined either
because the kind is unregistered or because the descriptor intentionally
exposes no `coverUrl` (e.g. workflow). Rewording it to focus on those two cases
will make the intent clearer and avoid implying the component uses
`coverImageUrl` directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
