84 license labels management#134
Open
amadulhaxxani wants to merge 11 commits intoclarin-v7from
Open
Conversation
Replace BaseDataService with IdentifiableDataService and add PutData/DeleteData support for ClarinLicenseLabel. Wire PutDataImpl and DeleteDataImpl in the constructor, expose put(), delete() and deleteByHref() methods, and update imports (remove BaseDataService, add PutData, DeleteData, IdentifiableDataService and NoContent). Enables updating and deleting Clarin License Labels via the data service.
Introduce a new License Labels section in the Clarin license table UI with a table, empty-state row, aria labels, and a ds-loading indicator (src/...component.html). Add CSS rule to size the actions column (src/...component.scss). In the component (src/...component.ts) add placeholder Observables (labels$, loading$) and stub methods editLabel and confirmDeleteLabel that log actions; data loading and wiring will be implemented in a follow-up task.
Add selectable labels UI and wiring for label management: - Template: add a radio column with aria labels, mark selected row, disable Edit/Delete unless a label is selected, and adjust colspan for empty state. Edit/Delete buttons now call no-arg handlers. - Component: introduce selectedLabel, labels$ and loading$ BehaviorSubjects, refreshLabels() to fetch all labels via clarinLicenseLabelService.findAll (with loading/error handling), selectLabel() and isSelected() helpers, and call refreshLabels() on init and after label creation. Add ngUnsubscribe and ngOnDestroy to clean up subscriptions (takeUntil). Update scan initial state to use a defaultListState and import SortOptions. - Tests: update spec to provide mockLicenseLabelListRD$ and spy findAll. These changes enable selecting a license label for future edit/delete actions and keep the label list in sync with the backend.
Replace the static license-labels table with a paginated, RemoteData-driven table and translated headers; add loading state and accessible SR-only labels. Wire up edit and delete flows: open DefineLicenseLabelFormComponent for edits (convert file input, call PUT, notify and refresh) and ConfirmationModalComponent for deletes (call DELETE, notify and refresh). Introduce labelsRD$ stream, pagination options, labelsRefresh$ trigger and initializeLabelsPaginationStream to reactively fetch pages. Enhance DefineLicenseLabelFormComponent to support edit mode (prefill form, boolean extended options, aria/id fixes) and update serializer to accept booleans and legacy string values. Update and extend unit tests to cover edit/delete flows and template changes. Add new English and Czech i18n keys for the labels UI and actions.
There was a problem hiding this comment.
Pull request overview
Adds full CRUD management for Clarin license labels in the Angular admin UI and extends the underlying data service to support PUT/DELETE, enabling admins to list, edit, and remove existing labels (Issue #84).
Changes:
- Extended
ClarinLicenseLabelDataServicewithput()/delete()/deleteByHref()support viaPutDataImplandDeleteDataImpl. - Added a paginated “License Labels” table to the license management screen with edit + delete actions and reactive refresh.
- Updated the “Define License Label” modal to support edit mode (pre-filled values) and added i18n keys for the new UI.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/assets/i18n/en.json5 | Adds English translations for the new labels section + CRUD messaging. |
| src/assets/i18n/cs.json5 | Adds Czech translations for the new labels section + CRUD messaging. |
| src/app/core/shared/clarin/clarin-license-label-extended-serializer.ts | Keeps compatibility by accepting both boolean and legacy string values. |
| src/app/core/data/clarin/clarin-license-label-data.service.ts | Implements PUT/DELETE operations for license labels via data service features. |
| src/app/clarin-licenses/clarin-license-table/modal/define-license-label-form/define-license-label-form.component.ts | Adds edit-mode input + form prefill; switches extended to boolean. |
| src/app/clarin-licenses/clarin-license-table/modal/define-license-label-form/define-license-label-form.component.html | Updates modal UI for create vs edit mode, accessibility fixes, and boolean select options. |
| src/app/clarin-licenses/clarin-license-table/modal/define-license-label-form/define-license-label-form.component.spec.ts | Adds unit tests for edit mode behavior and template conditions. |
| src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.ts | Adds labels table stream, pagination, refresh triggers, and edit/delete flows. |
| src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.html | Adds the license labels section/table with action buttons and loading state. |
| src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.scss | Adds a fixed-width actions column style for the labels table. |
| src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.spec.ts | Adds unit tests for edit/delete flows and row action rendering. |
Introduce a specific i18n key for license label load failures and use it in the labels pagination stream. Added a labelsLoadErrorKey constant in ClarinLicenseTableComponent and replaced usages of the create-error key when handling load failures. Also added the new "clarin.license.label.load.error" translations to en.json5 and cs.json5.
Call loadAllLicenses() when a label PUT succeeds so dependent license lists are refreshed. The component now only calls refreshLabels() and loadAllLicenses() if the RemoteData indicates success. Updated unit test to spy on loadAllLicenses and assert it is called on modal submit; adjusted method comment accordingly.
When a license label is successfully deleted, also reload the full license list to ensure UI reflects the change. Call loadAllLicenses() alongside refreshLabels() in the component, and update the spec to assert loadAllLicenses() is invoked (adding a spy and expectation).
Prevent the "no licenses" message from appearing while data is being fetched by adding a loading$ | async check to the empty-row *ngIf in clarin-license-table.component.html. The empty message now only shows when not loading and cLicenseLabels.page is empty, avoiding a flash of the empty state during load.
Prevent deleting license labels that are referenced by any license: template changes wrap the delete button in a span that conditionally shows an ngbTooltip and uses dsBtnDisabled with a conditional click handler. Component logic now loads all licenses (paged) on init, builds a Set of in-use label IDs (inUseLabelIds) and exposes isLabelInUse to drive the UI; helper methods fetchAllLicensePages, rebuildLabelUsageSet and loadAllLicensesForUsage were added along with an allLicensesRD$ stream. Unit tests were extended to cover linked vs unlinked label behavior (disabled state, tooltip, and preventing the confirmation modal), and i18n entries for the disabled-tooltip were added for English and Czech.
There was a problem hiding this comment.
Pull request overview
Adds full CRUD management for Clarin license labels in the admin UI (list + edit + delete), and extends the corresponding data service to support PUT/DELETE, completing the feature requested in Issue #84.
Changes:
- Extend
ClarinLicenseLabelDataServiceto supportput()anddelete()/deleteByHref(). - Add a paginated “License Labels” table with edit & delete flows (including confirmation modal and i18n).
- Update the define/edit label modal to support edit mode (pre-filled form, boolean
extended, accessibility fixes) and add/extend unit tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/assets/i18n/en.json5 | Adds new i18n keys for labels section/table and CRUD messaging. |
| src/assets/i18n/cs.json5 | Adds Czech translations for the new labels i18n keys. |
| src/app/core/shared/clarin/clarin-license-label-extended-serializer.ts | Makes serializer accept booleans while keeping legacy string compatibility. |
| src/app/core/data/clarin/clarin-license-label-data.service.ts | Adds put and delete capabilities via PutDataImpl / DeleteDataImpl. |
| src/app/clarin-licenses/clarin-license-table/modal/define-license-label-form/define-license-label-form.component.ts | Adds edit mode input and switches extended to boolean-backed select options. |
| src/app/clarin-licenses/clarin-license-table/modal/define-license-label-form/define-license-label-form.component.html | Updates modal title/buttons for create vs edit, fixes form control id/for, and fixes modal-body class name. |
| src/app/clarin-licenses/clarin-license-table/modal/define-license-label-form/define-license-label-form.component.spec.ts | Adds edit-mode tests and template assertions for edit/create mode text. |
| src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.ts | Implements labels list stream + refresh, edit/update flow, delete confirmation flow, and label “in use” derivation. |
| src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.html | Adds the labels management table (pagination, loading/empty states, action buttons, tooltips). |
| src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.spec.ts | Adds tests for edit/delete flows and for disabling delete when a label is in use. |
| src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.scss | Adds a fixed width for the labels actions column. |
Update clarin-license-table template to only render the labels empty-state row when the labels request has succeeded ((labelsRD$ | async)?.hasSucceeded). This prevents the empty message from appearing during failed or pending label requests. Add a unit test that simulates a failed labels request to verify the empty-state row is not shown, and import the needed createFailedRemoteDataObject helper for the test.
Add caching and locking around the expensive full license-usage crawl to avoid redundant requests. Introduce licenseUsageLoaded and licenseUsageLoading flags, a new ensureLicenseUsageLoaded(forceReload) helper, and a forceUsageReload parameter to loadAllLicenses so callers can explicitly invalidate the cache. Update create/edit/delete flows to force a usage reload, and set/clear the loading flags in the usage fetch success/error handlers. Add unit tests to verify that the full usage dataset is loaded only once across repeated reloads and that forcing a reload triggers a new fetch.
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
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.
Problem description
Issue #84 — License labels (short identifiers attached to licenses) had no management UI. Admins could create new labels via the existing Define License Label modal, but could not edit or delete existing labels. The data service also lacked
PUTandDELETEoperations entirely.Analysis
The feature was built incrementally across four commits on
84-license-labels-management:07fbf4d— ExtendedClarinLicenseLabelDataServicefromBaseDataServicetoIdentifiableDataService, implementingPutData,DeleteData,CreateData, andFindAllDatainterfaces. Addedput(),delete(), anddeleteByHref()methods backed byPutDataImpl/DeleteDataImpl.b1889a5— Added the license labels table section toclarin-license-table.component.htmlwith placeholder edit/delete button handlers.0f6fc9b— Replaced placeholder handlers with real logic:labelsRefresh$(BehaviorSubject),labelPaginationOptions(id: 'cLicenseLabels',pageSize: 10), andinitializeLabelsPaginationStream()usingcombineLatest+switchMap+takeUntil(ngUnsubscribe).95bf1c9— Completed the full CRUD cycle:editLabel()opensDefineLicenseLabelFormComponentpre-filled via@Input() clarinLicenseLabel,doUpdateLabel()callsput(),confirmDeleteLabel()opensConfirmationModalComponentand callsdelete()on confirmation. Replaced all hardcoded English strings withclarin.license.label.*i18n keys in bothen.json5andcs.json5. Fixedmodal-bodyCSS typo (wasmodal-boy) and added properid/forpairs on all modal form controls. Added null-guard early returns indefineNewLicense,editLicense, anddefineLicenseLabel. Added.labels-actions-columnSCSS rule.Validation
yarn run lint --quiet→ ✅ All files passyarn run build:prod→ ✅ Succeeded (no new warnings)yarn run test:headless→ ✅ 5374 completed, 4 skippedCopilot review