feat(es2): Load cedar template by ID instead of via the list endpoint#1022
feat(es2): Load cedar template by ID instead of via the list endpoint#1022adlius wants to merge 2 commits into
Conversation
| ); | ||
| } | ||
|
|
||
| getMetadataCedarTemplate(id: string): Observable<{ data: CedarMetadataDataTemplateJsonApi }> { |
There was a problem hiding this comment.
| getMetadataCedarTemplate(id: string): Observable<{ data: CedarMetadataDataTemplateJsonApi }> { | |
| getMetadataCedarTemplate(id: string): Observable<ResponseDataJsonApi<CedarMetadataDataTemplateJsonApi>> { |
| } | ||
|
|
||
| getMetadataCedarTemplate(id: string): Observable<{ data: CedarMetadataDataTemplateJsonApi }> { | ||
| return this.jsonApiService.get<{ data: CedarMetadataDataTemplateJsonApi }>( |
There was a problem hiding this comment.
| return this.jsonApiService.get<{ data: CedarMetadataDataTemplateJsonApi }>( | |
| return this.jsonApiService.get<ResponseDataJsonApi<CedarMetadataDataTemplateJsonApi>>( |
| missingIds.forEach((id) => { | ||
| this.metadataService | ||
| .getMetadataCedarTemplate(id) | ||
| .pipe(takeUntilDestroyed(this.destroyRef)) | ||
| .subscribe((response) => { | ||
| this.cedarTemplatesMap.update((map) => new Map(map).set(id, response.data)); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Also why do we need to get each template by ID instead of via the list?
There was a problem hiding this comment.
"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.
|
|
||
| const currentMap = this.cedarTemplatesMap(); | ||
| const missingIds = [ | ||
| ...new Set(records.map((r) => r.relationships?.template?.data?.id).filter((id): id is string => !!id)), |
There was a problem hiding this comment.
Do we need new Set here? Doesn't the back end already return unique IDs?
| this.metadataService | ||
| .getMetadataCedarTemplate(templateId) | ||
| .pipe(takeUntilDestroyed(this.destroyRef)) | ||
| .subscribe((response) => { | ||
| this.selectedCedarTemplate.set(response.data); | ||
| }); |
There was a problem hiding this comment.
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.
3 participants
Purpose
Summary of Changes
Screenshot(s)
Side Effects
QA Notes