Skip to content

feat(es2): Load cedar template by ID instead of via the list endpoint #1022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
adlius wants to merge 2 commits into
base: hotfix/26.12.2
Choose a base branch
from
Open

feat(es2): Load cedar template by ID instead of via the list endpoint #1022

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c703d0
feat(es2): Load cedar template by ID instead of via the list endpoint
adlius Jun 27, 2026
49771e7
feat(es2): Fix same issue on overview page
adlius Jun 27, 2026
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
44 changes: 13 additions & 31 deletions src/app/features/metadata/metadata.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { MetadataResourceEnum } from '@osf/shared/enums/metadata-resource.enum';
import { ResourceType } from '@osf/shared/enums/resource-type.enum';
import { CustomConfirmationService } from '@osf/shared/services/custom-confirmation.service';
import { CustomDialogService } from '@osf/shared/services/custom-dialog.service';
import { MetadataService } from '@osf/shared/services/metadata.service';
import { SignpostingService } from '@osf/shared/services/signposting.service';
import { ToastService } from '@osf/shared/services/toast.service';
import { CollectionsSelectors, GetProjectSubmissions } from '@osf/shared/stores/collections';
Expand Down Expand Up @@ -129,6 +130,7 @@ export class MetadataComponent implements OnInit, OnDestroy {
private readonly customConfirmationService = inject(CustomConfirmationService);
private readonly environment = inject(ENVIRONMENT);
private readonly signpostingService = inject(SignpostingService);
private readonly metadataService = inject(MetadataService);

private readonly activeFlags = select(UserSelectors.getActiveFlags);
readonly collectionSubmissionWithCedar = computed(() =>
Expand Down Expand Up @@ -234,23 +236,6 @@ export class MetadataComponent implements OnInit, OnDestroy {
this.handleRouteBasedTabSelection();
});

effect(() => {
const templates = this.cedarTemplates();
const selectedRecord = this.selectedCedarRecord();

if (selectedRecord && templates?.data && !this.selectedCedarTemplate()) {
const templateId = selectedRecord.relationships?.template?.data?.id;
if (templateId) {
const template = templates.data.find((t) => t.id === templateId);
if (template) {
this.selectedCedarTemplate.set(template);
} else if (templates.links?.next) {
this.actions.getCedarTemplates(templates.links.next);
}
}
}
});

effect(() => {
const metadata = this.metadata();

Expand Down Expand Up @@ -567,13 +552,11 @@ export class MetadataComponent implements OnInit, OnDestroy {

private loadCedarRecord(recordId: string): void {
const records = this.cedarRecords();
const templates = this.cedarTemplates();
if (!records) {
return;
}

const record = records.find((r) => r.id === recordId);

if (!record) {
return;
}
Expand All @@ -582,21 +565,20 @@ export class MetadataComponent implements OnInit, OnDestroy {
this.cedarFormReadonly.set(true);

const templateId = record.relationships?.template?.data?.id;

if (templateId && templates?.data) {
const template = templates.data.find((t) => t.id === templateId);
if (template) {
this.selectedCedarTemplate.set(template);
} else {
this.selectedCedarTemplate.set(null);
this.actions.getCedarTemplates();
}
} else {
this.selectedCedarTemplate.set(null);
this.actions.getCedarTemplates();
if (templateId) {
this.loadCedarTemplateById(templateId);
}
}

private loadCedarTemplateById(templateId: string): void {
this.metadataService
.getMetadataCedarTemplate(templateId)
.pipe(takeUntilDestroyed(this.destroyRef))
.subscribe((response) => {
this.selectedCedarTemplate.set(response.data);
});
Comment on lines +574 to +579

@nsemets nsemets Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Use one shared action GetCedarMetadataTemplatesByIds here and in project-overview-metadata — same cache in store, different selectors for reading. Move template loading out of the component: dispatch ids, read from store, no subscribe.

}

private handleRouteBasedTabSelection(): void {
const recordId = this.activeRoute.snapshot.paramMap.get('recordId');

Expand Down
41 changes: 30 additions & 11 deletions ...ject/overview/components/project-overview-metadata/project-overview-metadata.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,13 @@ import { TranslatePipe } from '@ngx-translate/core';
import { Button } from 'primeng/button';

import { DatePipe } from '@angular/common';
import { ChangeDetectionStrategy, Component, computed, effect, inject } from '@angular/core';
import { ChangeDetectionStrategy, Component, computed, DestroyRef, effect, inject, signal } from '@angular/core';
import { takeUntilDestroyed } from '@angular/core/rxjs-interop';
import { Router, RouterLink } from '@angular/router';

import { UserSelectors } from '@core/store/user';
import {
GetCedarMetadataRecords,
GetCedarMetadataTemplates,
GetCustomItemMetadata,
MetadataSelectors,
} from '@osf/features/metadata/store';
import { CedarMetadataDataTemplateJsonApi } from '@osf/features/metadata/models';
import { GetCedarMetadataRecords, GetCustomItemMetadata, MetadataSelectors } from '@osf/features/metadata/store';
import { AffiliatedInstitutionsViewComponent } from '@osf/shared/components/affiliated-institutions-view/affiliated-institutions-view.component';
import { ContributorsListComponent } from '@osf/shared/components/contributors-list/contributors-list.component';
import { FundersListComponent } from '@osf/shared/components/funders-list/funders-list.component';
Expand All @@ -27,6 +24,7 @@ import { TruncatedTextComponent } from '@osf/shared/components/truncated-text/tr
import { CurrentResourceType, ResourceType } from '@osf/shared/enums/resource-type.enum';
import { LanguageLabelPipe } from '@osf/shared/pipes/language-label.pipe';
import { ResourceTypeGeneralLabelPipe } from '@osf/shared/pipes/resource-type-general-label.pipe';
import { MetadataService } from '@osf/shared/services/metadata.service';
import { CollectionsSelectors, GetProjectSubmissions } from '@osf/shared/stores/collections';
import {
ContributorsSelectors,
Expand Down Expand Up @@ -74,6 +72,8 @@ import { OverviewSupplementsComponent } from '../overview-supplements/overview-s
})
export class ProjectOverviewMetadataComponent {
private readonly router = inject(Router);
private readonly destroyRef = inject(DestroyRef);
private readonly metadataService = inject(MetadataService);

readonly currentProject = select(ProjectOverviewSelectors.getProject);
readonly isAnonymous = select(ProjectOverviewSelectors.isProjectAnonymous);
Expand All @@ -97,10 +97,11 @@ export class ProjectOverviewMetadataComponent {
readonly isProjectSubmissionsLoading = select(CollectionsSelectors.getCurrentProjectSubmissionsLoading);
readonly activeFlags = select(UserSelectors.getActiveFlags);
readonly cedarRecords = select(MetadataSelectors.getCedarRecords);
private readonly cedarTemplatesResponse = select(MetadataSelectors.getCedarTemplates);
readonly cedarTemplates = computed(() => this.cedarTemplatesResponse()?.data ?? null);
readonly isCedarMode = computed(() => this.activeFlags().includes(COLLECTION_SUBMISSION_WITH_CEDAR));

private readonly cedarTemplatesMap = signal<Map<string, CedarMetadataDataTemplateJsonApi>>(new Map());
readonly cedarTemplates = computed(() => [...this.cedarTemplatesMap().values()]);

readonly resourceType = CurrentResourceType.Projects;
readonly dateFormat = 'MMM d, y, h:mm a';

Expand All @@ -116,14 +117,14 @@ export class ProjectOverviewMetadataComponent {
getBibliographicContributors: GetBibliographicContributors,
loadMoreBibliographicContributors: LoadMoreBibliographicContributors,
getCedarRecords: GetCedarMetadataRecords,
getCedarTemplates: GetCedarMetadataTemplates,
});

constructor() {
effect(() => {
const project = this.currentProject();

if (project?.id) {
this.cedarTemplatesMap.set(new Map());
this.actions.getBibliographicContributors(project.id, ResourceType.Project);
this.actions.getInstitutions(project.id);
this.actions.getIdentifiers(project.id);
Expand All @@ -132,10 +133,28 @@ export class ProjectOverviewMetadataComponent {
this.actions.getProjectSubmissions(project.id);
this.actions.getLicense(project.licenseId);
this.actions.getCedarRecords(project.id, ResourceType.Project);
this.actions.getCedarTemplates();
this.actions.getCustomItemMetadata(project.id);
}
});

effect(() => {
const records = this.cedarRecords();
if (!records?.length) return;

const currentMap = this.cedarTemplatesMap();
const missingIds = [
...new Set(records.map((r) => r.relationships?.template?.data?.id).filter((id): id is string => !!id)),

@nsemets nsemets Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do we need new Set here? Doesn't the back end already return unique IDs?

].filter((id) => !currentMap.has(id));

missingIds.forEach((id) => {
this.metadataService
.getMetadataCedarTemplate(id)
.pipe(takeUntilDestroyed(this.destroyRef))
.subscribe((response) => {
this.cedarTemplatesMap.update((map) => new Map(map).set(id, response.data));
});
});
});
Comment on lines +149 to +157

@nsemets nsemets Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Move cedar template loading into NGXS: dispatch GetCedarMetadataTemplatesByIds(templateIds) when records load, cache templates in store, and read them with a selector. The component should only dispatch and select — no local Map, no subscribe.
We can discuss it if needed.

@nsemets nsemets Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also why do we need to get each template by ID instead of via the list?

@brianjgeiger brianjgeiger Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"Also why do we need to get each template by ID instead of via the list?"

This is standard procedure when working with the v2 API. If we have an ID for something, then we use the detail endpoint with the ID to get the item. This prevents us from having to request more data than we need for the individual item, and it prevents us from having to iterate through items to find the thing we need.

}

onCustomCitationUpdated(citation: string): void {
Expand Down
7 changes: 7 additions & 0 deletions src/app/shared/services/metadata.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { inject, Injectable } from '@angular/core';
import { ENVIRONMENT } from '@core/provider/environment.provider';
import { CedarRecordsMapper, MetadataMapper, RorMapper } from '@osf/features/metadata/mappers';
import {
CedarMetadataDataTemplateJsonApi,
CedarMetadataRecord,
CedarMetadataRecordJsonApi,
CedarMetadataTemplateJsonApi,
Expand Down Expand Up @@ -103,6 +104,12 @@ export class MetadataService {
);
}

getMetadataCedarTemplate(id: string): Observable<{ data: CedarMetadataDataTemplateJsonApi }> {

@nsemets nsemets Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
getMetadataCedarTemplate(id: string): Observable<{ data: CedarMetadataDataTemplateJsonApi }> {
getMetadataCedarTemplate(id: string): Observable<ResponseDataJsonApi<CedarMetadataDataTemplateJsonApi>> {

@nsemets nsemets Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Use this interface.

return this.jsonApiService.get<{ data: CedarMetadataDataTemplateJsonApi }>(

@nsemets nsemets Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return this.jsonApiService.get<{ data: CedarMetadataDataTemplateJsonApi }>(
return this.jsonApiService.get<ResponseDataJsonApi<CedarMetadataDataTemplateJsonApi>>(

`${this.apiDomainUrl}/_/cedar_metadata_templates/${id}/`
);
}

getMetadataCedarRecords(
resourceId: string,
resourceType: ResourceType,
Expand Down
Loading