LNK-4711: Informative validation of Measure/Validation Artifacts - #1374
Merged
Conversation
… artifacts Modifications to UI to allow user to click on a measure and see the info about the measure's related artifacts.
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughThis PR introduces a feature enabling system administrators to view measure dependencies and their availability within the system. It adds a backend REST endpoint that inspects measure bundles to identify related artifacts, returning artifact metadata with version and library information, complemented by a frontend modal component to display these results. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant AdminUI as Admin UI
participant Modal as Related Artifacts Modal
participant Service as Measure Definition Service
participant Controller as MeasureDefinitionController
participant Validation as MeasureValidationService
participant Repo as Repository
participant DB as Database
User->>AdminUI: Click measure ID button
AdminUI->>Modal: Open modal with measure ID
activate Modal
Modal->>Service: getRelatedArtifacts(id)
activate Service
Service->>Controller: GET /{id}/relatedArtifact
activate Controller
Controller->>Validation: getRelatedArtifacts(id)
activate Validation
Validation->>Repo: findById(id)
Repo->>DB: Query measure definition
DB-->>Repo: Return measure with bundle
Repo-->>Validation: MeasureDefinition
Validation->>Validation: Parse bundle, extract<br/>related artifacts,<br/>determine found status
Validation-->>Controller: List<RelatedArtifactInfo>
deactivate Validation
Controller-->>Service: HTTP 200 + artifacts
deactivate Controller
Service-->>Modal: Observable<IRelatedArtifact[]>
deactivate Service
Modal->>Modal: Update artifacts list,<br/>set loading = false
deactivate Modal
Modal-->>User: Display artifacts table
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Web/Admin.UI/src/app/components/measure-def/measure-def-config-form/measure-def-config-form.component.ts (1)
35-38: DuplicateMatIconModuleimport.
MatIconModuleis imported twice in the imports array (lines 35 and 38).Proposed fix
imports: [ MatIconModule, MatFormFieldModule, MatInputModule, - MatIconModule, MatChipsModule,
🤖 Fix all issues with AI agents
In
`@Java/measureeval/src/main/java/com/lantanagroup/link/measureeval/controllers/MeasureDefinitionController.java`:
- Around line 167-172: Add the GET /{id}/relatedArtifact path to the OpenAPI
spec in docs/domains/Report/services/MeasureEvalService/openapi.yml: mirror the
style of other measure-definition paths and document OperationId (e.g.,
getRelatedArtifacts), summary "Get related artifacts for a measure definition",
a path parameter "id" (string, required) and the 200 response returning an array
of the RelatedArtifactInfo schema (ensure RelatedArtifactInfo model/schema is
defined or referenced exactly as used by the
MeasureDefinitionController.getRelatedArtifacts method), plus appropriate tags
("Measure Definitions") and example/description fields consistent with existing
endpoints.
In
`@Web/Admin.UI/src/app/components/measure-def/measure-def-config-form/measure-def-config-form.component.html`:
- Around line 42-44: The template uses an anchor tag for a click-only action
which is semantically incorrect; replace the <a mat-button color="primary"
(click)="openRelatedArtifactsModal(element)">{{ element.id }}</a> with a <button
mat-button> variant so it is keyboard and screen-reader accessible and still
calls openRelatedArtifactsModal(element) (keep the same click handler and
displayed {{ element.id }}). Ensure any CSS or styling targeting the anchor is
updated to target the button if needed.
🧹 Nitpick comments (7)
Java/measureeval/src/main/java/com/lantanagroup/link/measureeval/services/MeasureValidationService.java (1)
119-122: Simplify the count operation.The
mapToInt(info -> 1).sum()pattern can be replaced with the more idiomatic.count()method.♻️ Suggested simplification
- int notFoundCount = relatedArtifactInfos.stream() - .filter(info -> !info.isFound()) - .mapToInt(info -> 1) - .sum(); + long notFoundCount = relatedArtifactInfos.stream() + .filter(info -> !info.isFound()) + .count();Web/Admin.UI/src/app/components/measure-def/related-artifacts-modal/related-artifacts-modal.component.scss (2)
24-26:::ng-deepis deprecated in Angular.While
::ng-deepstill works, it has been deprecated. Consider using global styles in a theme file or the component'sencapsulation: ViewEncapsulation.Nonefor tooltip styling that needs to pierce shadow DOM boundaries.
15-18: Consider using Angular Material theme variable for consistency.The hardcoded color
#f44336works but using a theme variable likemat.get-color-from-palette($warn-palette)would ensure consistency with the application's theme and simplify future theme changes.Java/measureeval/src/main/java/com/lantanagroup/link/measureeval/controllers/MeasureDefinitionController.java (2)
167-172: Missing user tracing and authentication principal for consistency.Other endpoints in this controller capture the
@AuthenticationPrincipal PrincipalUserand set span attributes for user tracing. This endpoint lacks that pattern, which may create gaps in observability.Proposed fix
`@GetMapping`("/{id}/relatedArtifact") `@Operation`(summary = "Get related artifacts for a measure definition", tags = {"Measure Definitions"}) `@Parameter`(name = "id", description = "The ID of the measure definition", required = true) -public List<RelatedArtifactInfo> getRelatedArtifacts(`@PathVariable` String id) { +public List<RelatedArtifactInfo> getRelatedArtifacts(`@AuthenticationPrincipal` PrincipalUser user, `@PathVariable` String id) { + if (user != null) { + Span currentSpan = Span.current(); + currentSpan.setAttribute("user", user.getEmailAddress()); + } return validationService.getRelatedArtifacts(id); }
167-167: Consider using plural formrelatedArtifactsin the endpoint path.The endpoint returns
List<RelatedArtifactInfo>, so using plural/relatedArtifactswould be more consistent with REST conventions for collection resources.Web/Admin.UI/src/app/components/measure-def/related-artifacts-modal/related-artifacts-modal.component.html (1)
7-35: Consider adding an empty state message when no related artifacts are found.If
relatedArtifactsis an empty array, the table will render with just headers and no rows, which may confuse users. Adding an empty state message would improve UX.Proposed enhancement
<table mat-table [dataSource]="relatedArtifacts" *ngIf="!loading" class="mat-elevation-z8"> <!-- ... existing columns ... --> <tr mat-header-row *matHeaderRowDef="displayedColumns"></tr> <tr mat-row *matRowDef="let row; columns: displayedColumns;"></tr> + <tr class="mat-row" *matNoDataRow> + <td class="mat-cell" [attr.colspan]="displayedColumns.length">No related artifacts found.</td> + </tr> </table>Web/Admin.UI/src/app/components/measure-def/related-artifacts-modal/related-artifacts-modal.component.ts (1)
37-47: Add error state handling to provide user feedback on failure.The error handler silently sets
loading = falsewithout storing or displaying an error message. Users will see an empty table with no indication that a failure occurred, making it impossible to distinguish between "no artifacts" and "failed to load."Consider adding an
errorMessageproperty and displaying it in the template:♻️ Suggested improvement
export class RelatedArtifactsModalComponent implements OnInit { relatedArtifacts: IRelatedArtifact[] = []; displayedColumns: string[] = ['name', 'version', 'url', 'found']; loading = true; + errorMessage: string | null = null; constructor( `@Inject`(MAT_DIALOG_DATA) public data: { id: string }, private measureDefinitionService: MeasureDefinitionService ) {} ngOnInit(): void { this.measureDefinitionService.getRelatedArtifacts(this.data.id).subscribe({ next: (artifacts) => { this.relatedArtifacts = artifacts; this.loading = false; }, - error: () => { + error: (err) => { this.loading = false; + this.errorMessage = 'Failed to load related artifacts. Please try again.'; + console.error('Error loading related artifacts:', err); } }); }Then display the error in the template when
errorMessageis set.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
Java/measureeval/src/main/java/com/lantanagroup/link/measureeval/controllers/MeasureDefinitionController.javaJava/measureeval/src/main/java/com/lantanagroup/link/measureeval/models/RelatedArtifactInfo.javaJava/measureeval/src/main/java/com/lantanagroup/link/measureeval/services/MeasureValidationService.javaJava/measureeval/src/test/java/com/lantanagroup/link/measureeval/services/MeasureValidationServiceTest.javaWeb/Admin.UI/src/app/components/measure-def/measure-def-config-form/measure-def-config-form.component.htmlWeb/Admin.UI/src/app/components/measure-def/measure-def-config-form/measure-def-config-form.component.tsWeb/Admin.UI/src/app/components/measure-def/related-artifacts-modal/related-artifacts-modal.component.htmlWeb/Admin.UI/src/app/components/measure-def/related-artifacts-modal/related-artifacts-modal.component.scssWeb/Admin.UI/src/app/components/measure-def/related-artifacts-modal/related-artifacts-modal.component.tsWeb/Admin.UI/src/app/interfaces/measure-definition/related-artifact.interface.tsWeb/Admin.UI/src/app/services/gateway/measure-definition/measure.service.ts
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
**: Pull requests that have "TECH_DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates,
and logging improvements. These TECH_DEBT PRs must not affect core functionality. All PRs that are not considered technical debt must include information on what
testing was performed in the description of the PR. If it does not, ask the author to provide details on what testing was performed.
When reviewing code, suggest unit tests using XUnit in the following scenarios:
- If/Else or Switch/Case blocks are introduced or modified — ensure each branch has a corresponding unit test.
- Logic that depends on service or interface configuration — suggest tests to validate different implementations are correctly resolved.
- No network activity (HTTP calls, sockets, etc.) should appear in unit tests. Recommend using mocks (via Moq) for any external communication.
Large unit tests should be avoided; keeping unit tests small and focused on targeted business logic (i.e. string sanitization)
**: Pull requests that have DOCS in the title should only contain changes related to documentation within the /docs folder or in .md files through-out the code-base. The description
of the PR should specify what documentation was updated. Documentation updates should use EventCatalog.dev structure, where service-specific functionality should be described
in the service's index.mdx (i.e. /services/XXX/index.mdx or /domains/XXX/services/YYY/index.mdx). Configurations that are shared by multiple services should be
reflected in the /docs/docs/config files.
Files:
Web/Admin.UI/src/app/interfaces/measure-definition/related-artifact.interface.tsWeb/Admin.UI/src/app/components/measure-def/measure-def-config-form/measure-def-config-form.component.htmlWeb/Admin.UI/src/app/services/gateway/measure-definition/measure.service.tsJava/measureeval/src/main/java/com/lantanagroup/link/measureeval/models/RelatedArtifactInfo.javaWeb/Admin.UI/src/app/components/measure-def/related-artifacts-modal/related-artifacts-modal.component.scssWeb/Admin.UI/src/app/components/measure-def/measure-def-config-form/measure-def-config-form.component.tsWeb/Admin.UI/src/app/components/measure-def/related-artifacts-modal/related-artifacts-modal.component.tsJava/measureeval/src/main/java/com/lantanagroup/link/measureeval/services/MeasureValidationService.javaJava/measureeval/src/main/java/com/lantanagroup/link/measureeval/controllers/MeasureDefinitionController.javaJava/measureeval/src/test/java/com/lantanagroup/link/measureeval/services/MeasureValidationServiceTest.javaWeb/Admin.UI/src/app/components/measure-def/related-artifacts-modal/related-artifacts-modal.component.html
🧠 Learnings (4)
📓 Common learnings
Learnt from: sdmcgeown
Repo: lantanagroup/link-cloud PR: 0
File: :0-0
Timestamp: 2025-07-24T17:49:23.117Z
Learning: PR `#1004` in the Link Cloud project introduced comprehensive normalization operation testing functionality including a new test operation dialog, JSON validation, resource type validation, and real-time testing capabilities that require documentation updates across multiple files in the docs folder.
Learnt from: arianamihailescu
Repo: lantanagroup/link-cloud PR: 0
File: :0-0
Timestamp: 2025-08-13T17:27:27.383Z
Learning: The MeasureEvalService in the Link Cloud project has comprehensive OpenAPI documentation at docs/domains/Report/services/MeasureEvalService/openapi.yml that needs to be updated when API endpoints change, particularly for measure-definition endpoints.
Learnt from: seanmcilvenna
Repo: lantanagroup/link-cloud PR: 0
File: :0-0
Timestamp: 2025-07-30T22:50:26.140Z
Learning: PR `#1030` in the Link Cloud project introduced Category and Issue Retrieval Operations to the Validation Service, including new ResultService for business logic separation, CategoryController and ResultController endpoints, and models for CategorySummaryModel and CategoryIssueModel with uncategorized issue handling.
📚 Learning: 2025-05-13T19:11:06.305Z
Learnt from: amphillipsLGC
Repo: lantanagroup/link-cloud PR: 836
File: Web/Admin.UI/src/app/components/audit/audit-dashboard/audit-dashboard.component.ts:129-133
Timestamp: 2025-05-13T19:11:06.305Z
Learning: In this Angular application, error handling for HTTP requests is implemented at the service level using a centralized ErrorHandlingService. Components don't need to implement their own error handling when calling service methods as errors are already handled by the service through RxJS catchError operator.
Applied to files:
Web/Admin.UI/src/app/services/gateway/measure-definition/measure.service.ts
📚 Learning: 2025-08-13T17:27:27.383Z
Learnt from: arianamihailescu
Repo: lantanagroup/link-cloud PR: 0
File: :0-0
Timestamp: 2025-08-13T17:27:27.383Z
Learning: The MeasureEvalService in the Link Cloud project has comprehensive OpenAPI documentation at docs/domains/Report/services/MeasureEvalService/openapi.yml that needs to be updated when API endpoints change, particularly for measure-definition endpoints.
Applied to files:
Java/measureeval/src/main/java/com/lantanagroup/link/measureeval/services/MeasureValidationService.javaJava/measureeval/src/main/java/com/lantanagroup/link/measureeval/controllers/MeasureDefinitionController.javaJava/measureeval/src/test/java/com/lantanagroup/link/measureeval/services/MeasureValidationServiceTest.java
📚 Learning: 2025-08-05T17:06:51.464Z
Learnt from: smailliwcs
Repo: lantanagroup/link-cloud PR: 1042
File: Java/validation/src/main/java/com/lantanagroup/link/validation/services/ReadyForValidationConsumer.java:120-121
Timestamp: 2025-08-05T17:06:51.464Z
Learning: In the Link Cloud validation service, the Report service contract guarantees that a valid response will never contain a null bundle from model.getBundle(), making null checks unnecessary in the ReadyForValidationConsumer.
Applied to files:
Java/measureeval/src/test/java/com/lantanagroup/link/measureeval/services/MeasureValidationServiceTest.java
🧬 Code graph analysis (4)
Web/Admin.UI/src/app/services/gateway/measure-definition/measure.service.ts (1)
Web/Admin.UI/src/app/interfaces/measure-definition/related-artifact.interface.ts (1)
IRelatedArtifact(1-7)
Web/Admin.UI/src/app/components/measure-def/measure-def-config-form/measure-def-config-form.component.ts (1)
Web/Admin.UI/src/app/interfaces/measure-definition/measure-definition-config-model.interface.ts (1)
IMeasureDefinitionConfigModel(1-5)
Web/Admin.UI/src/app/components/measure-def/related-artifacts-modal/related-artifacts-modal.component.ts (2)
Web/Admin.UI/src/app/components/measure-def/measure-def-config-form/measure-def-config-form.component.ts (1)
Component(27-191)Web/Admin.UI/src/app/interfaces/measure-definition/related-artifact.interface.ts (1)
IRelatedArtifact(1-7)
Java/measureeval/src/main/java/com/lantanagroup/link/measureeval/services/MeasureValidationService.java (2)
Web/Admin.UI/src/app/services/gateway/measure-definition/measure.service.ts (1)
getRelatedArtifacts(37-43)Web/Admin.UI/src/app/components/measure-def/measure-def-config-form/measure-def-config-form.component.ts (1)
bundle(90-92)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Smoke Test with Docker Compose
- GitHub Check: Integration Tests
- GitHub Check: Unit Tests for DotNet
- GitHub Check: Analyze (java-kotlin)
- GitHub Check: Analyze (csharp)
- GitHub Check: Build Documentation
🔇 Additional comments (13)
Java/measureeval/src/main/java/com/lantanagroup/link/measureeval/models/RelatedArtifactInfo.java (1)
1-17: LGTM!The model is clean and well-structured. The fields align correctly with the frontend
IRelatedArtifactinterface, and initializinglibrariesas aHashMapprevents null pointer exceptions.Java/measureeval/src/main/java/com/lantanagroup/link/measureeval/services/MeasureValidationService.java (2)
37-127: Consider updating OpenAPI documentation.Based on learnings, the MeasureEvalService has comprehensive OpenAPI documentation at
docs/domains/Report/services/MeasureEvalService/openapi.yml. Since this PR adds a new endpoint (GET /{id}/relatedArtifact), the OpenAPI specification should be updated to document the new endpoint, request/response schemas, and possible error codes (404, 400).
129-142: LGTM!The helper method correctly extracts canonical URLs with version suffixes for MetadataResource types using idiomatic pattern matching.
Java/measureeval/src/test/java/com/lantanagroup/link/measureeval/services/MeasureValidationServiceTest.java (5)
33-43: LGTM!Good test coverage for the 404 case with proper exception validation including status code and reason message.
45-59: LGTM!Properly tests the missing bundle scenario with correct status code and message validation.
61-136: LGTM!Comprehensive test that covers multiple scenarios including:
- Artifacts found vs not found in bundle
- Version extraction from URLs
- Library associations
- Ignored CQF URLs and non-DEPENDSON types
The test is thorough without being excessive.
138-168: LGTM!Good edge case test verifying that non-MetadataResource types (like Patient) are correctly handled and won't be found by canonical URL lookup.
170-208: LGTM!Clear and focused test for the CQF URL prefix filtering logic. Good coverage of multiple ignored prefixes alongside a non-ignored artifact.
Web/Admin.UI/src/app/services/gateway/measure-definition/measure.service.ts (1)
37-43: LGTM!The new method follows the established service patterns with proper error handling via the centralized
ErrorHandlingService. The endpoint URL correctly matches the backend controller path.Web/Admin.UI/src/app/interfaces/measure-definition/related-artifact.interface.ts (1)
1-7: LGTM!The interface correctly mirrors the Java
RelatedArtifactInfomodel with appropriate TypeScript types and optional modifiers for nullable fields.Web/Admin.UI/src/app/components/measure-def/related-artifacts-modal/related-artifacts-modal.component.html (1)
1-39: LGTM overall!The modal template is well-structured with appropriate loading state handling, clear visual indicators for found/not-found artifacts, and proper Angular Material component usage.
Web/Admin.UI/src/app/components/measure-def/measure-def-config-form/measure-def-config-form.component.ts (1)
100-107: LGTM!The
openRelatedArtifactsModalmethod is clean and follows Angular Material dialog patterns correctly. The dialog dimensions are appropriate for displaying tabular artifact data.Web/Admin.UI/src/app/components/measure-def/related-artifacts-modal/related-artifacts-modal.component.ts (1)
49-52: LGTM! Consider adding a unit test for this helper.The helper correctly handles the undefined case and formats the libraries for tooltip display. Per coding guidelines, logic like this benefits from unit test coverage to validate the formatting behavior.
Example test cases to consider:
undefinedinput returns empty string- Empty object
{}returns empty string- Single entry formats correctly
- Multiple entries join with newlines
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…del, update service and tests, and implement UI for viewing package TX dependencies
…, and improve package/resource display
seanmcilvenna
changed the title
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🛠️ Description of Changes
🧪 Testing Performed
Built and ran in docker-compose. Viewed the screen with its changes and clicked around to make sure it's functioning as expected. Screen looks good.
🧑🔬 Unit Testing
📓 Documentation Updated
N/A
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.