Fix/concept relations slow - #209
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Uncancelled public-frontend requests can overwrite the current concept with stale relation data.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Optimizes concept-relation loading across the backend and both Angular frontends.
Changes:
- Batches dataset resolution and adds lightweight entity loading.
- Returns dataset metadata with structure references.
- Reuses initial paged results and adds local loading indicators.
File summaries
| File | Description |
|---|---|
frontend/public-ui/src/app/shared/templates/description/description.view.template.component.ts |
Adds relation-loading state. |
frontend/public-ui/src/app/shared/templates/description/description.view.template.component.html |
Displays the relation spinner. |
frontend/public-ui/src/app/shared/interceptors/background-request.ts |
Suppresses the global spinner for marked requests. |
frontend/public-ui/src/app/shared/concept-relation-table/concept-relation-table.component.ts |
Consumes enriched, preloaded references. |
frontend/public-ui/src/app/shared/concept-relation-table/concept-relation-table.component.html |
Adds pagination loading UI. |
frontend/public-ui/src/app/concepts/concept-detail/description/concept-detail-description.component.ts |
Fetches the initial reference page and count. |
frontend/public-ui/src/app/concepts/concept-detail/description/concept-detail-description.component.html |
Passes references and paging metadata. |
frontend/public-ui/src/app/app.module.ts |
Registers the background interceptor. |
frontend/admin-ui/src/app/shared/templates/view-templates/description/description.view.template.component.ts |
Adds relation-loading state. |
frontend/admin-ui/src/app/shared/templates/view-templates/description/description.view.template.component.html |
Displays the relation spinner. |
frontend/admin-ui/src/app/shared/interceptors/background-request.ts |
Suppresses the global spinner for marked requests. |
frontend/admin-ui/src/app/shared/concept-relation-table/concept-relation-table.component.ts |
Uses enriched reference data. |
frontend/admin-ui/src/app/shared/concept-relation-table/concept-relation-table.component.html |
Adds pagination loading UI. |
frontend/admin-ui/src/app/concepts/services/concept.service.ts |
Loads references with paging metadata. |
frontend/admin-ui/src/app/concepts/description/description.component.ts |
Tracks reference loading and paging state. |
frontend/admin-ui/src/app/concepts/description/description.component.html |
Supplies preloaded references to the table. |
frontend/admin-ui/src/app/app.module.ts |
Registers the background interceptor. |
build/ts-client/generated/BfsIopAdminApiClient.g.ts |
Adds dataset metadata to the generated contract. |
backend/src/Data/Bfs.Iop.DataAccess/Contracts/IDatasetsService.cs |
Adds configurable include depth. |
backend/src/Data/Bfs.Iop.DataAccess/Contracts/EntityIncludeLevel.cs |
Exposes entity inclusion levels. |
backend/src/Data/Bfs.Iop.DataAccess.Relational/Services/EntityIncludeLevel.cs |
Removes the superseded internal enum. |
backend/src/Data/Bfs.Iop.DataAccess.Relational/Services/DatasetsService.cs |
Applies the requested inclusion level. |
backend/src/Data/Bfs.Iop.DataAccess.Abstractions/DatasetReferenceModel.cs |
Defines compact dataset reference metadata. |
backend/src/Core/Bfs.Iop.Core/Services/RelationsCountService.cs |
Uses minimal dataset loading. |
backend/src/Core/Bfs.Iop.Core/CommandHandlers/PublicServices/GetGetPublicServiceIsDescribedAtCommandHandler.cs |
Preserves cancellation-token binding. |
backend/src/Core/Bfs.Iop.Core.LinkedData/Services/DatasetModelTripleStoreProcessService.cs |
Resolves referenced datasets in one batch. |
backend/src/Core/Bfs.Iop.Core.Abstractions/Models/IopConceptStructureReferenceModel.cs |
Adds enriched dataset information. |
Review details
- Files reviewed: 26/29 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
OK for my part |
Canadas24
left a comment
There was a problem hiding this comment.
I gave you the idea of the EntityIncluddeLevel, but we have to find another way
There was a problem hiding this comment.
🟡 Changes recommended
The generated TypeScript artifact does not match the backend contract or the frontend property accesses.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 23/26 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The generated contract is inconsistent with its consumers, and asynchronous responses can expose stale concept relations.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
frontend/admin-ui/src/app/concepts/services/concept.service.ts:103
- These callbacks can publish a response for an obsolete concept.
ConceptViewComponentcallsload(..., true)whenever route params change, but this service neither cancels the prior structure-reference request nor verifies itsid; a slower earlier response can therefore replace the new concept's rows and paging data. Track the active request id or switch/cancel requests before emitting tostructureReferences.
backend/src/Data/Bfs.Iop.DataAccess.Relational/Services/DatasetsService.cs:127 - Add a
DatasetsServiceTestscase for this new authorization-sensitive projection, covering that unreadable dataset IDs are excluded and that identifier/title/publisher fields are returned for readable datasets. This method now gates both relation counts and structure-reference details, while the existing service test fixture currently exercises only deletion behavior.
frontend/public-ui/src/app/concepts/concept-detail/description/concept-detail-description.component.ts:95
- Reset the previous concept's relation state when starting this request. On route reuse, if the new request fails, the error handler only clears
structureReferencesLoading; the old count and rows remain and are then rendered under the new concept. Clearing the cached values here prevents stale relations from being exposed after an error.
this.structureReferencesLoading = true;
- Files reviewed: 23/26 changed files
- Comments generated: 0 new
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The canonical generated client has contract mismatches, and stale admin requests can display references for the wrong concept.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
frontend/admin-ui/src/app/concepts/services/concept.service.ts:102
- A response from an earlier
load(A)can arrive afterload(B)and publish A's references into this shared replay subject. The route reuses the concept view component, so this can end B's loading state and display rows/paging from the wrong concept. Track the latest requested id (or cancel prior requests withswitchMap) and ignore both success and error callbacks from stale loads, as the public view already does.
- Files reviewed: 23/26 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Changes:
EntityIncludeLevelonGetDatasetsso callers that only need a title or an id don't pull in 33Includes.RelationsCountServicebenefits too./api/Datasetsat all, and no longer callsstructure-references/countbeforestructure-references(the paged call already returns the total in its headers).@yujie-ZHENG-BFS can you review the frontend?
@Canadas24 can you review the backend?