[Port to dtq-dev] 84 license labels management#1299
Open
kosarko wants to merge 2 commits into
Open
Conversation
* Add PUT and DELETE to ClarinLicenseLabel service
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.
* Add license labels table and placeholders
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 label selection and refresh logic
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.
* Add license labels management and i18n
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.
* Use dedicated i18n key for label load errors
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.
* Refresh licenses after label update
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.
* Reload licenses after label deletion
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).
* Hide empty table row while loading
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.
* Disable delete for labels in use with tooltip
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.
* Show labels empty row only on successful load
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.
* Optimize license usage loading and tests
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.
* F1: disable label Delete until usage crawl completes
The in-use set (inUseLabelIds) is empty until the async license usage
crawl finishes, so during that window an in-use label's Delete button
was briefly enabled. Add a usageReady$ stream that only emits true once
the crawl succeeds and gate every row's Delete button on it: all Delete
buttons stay disabled (with a "checking usage" tooltip) until usage
resolves, after which in-use labels stay disabled and unused ones enable.
On failure the buttons stay disabled so there is no unsafe window.
Frontend-only mitigation; the usage crawl itself is unchanged.
* F2: de-wrap double-nested modal in both license forms
Both define-license-form and define-license-label-form nested their own
.modal > .modal-dialog > .modal-content inside ng-bootstrap's
NgbModalWindow, which already provides those wrappers. The duplicate
chrome collapsed ngb's .modal-content and pinned the inner position:fixed
.modal to the top of the viewport instead of centering.
Remove the redundant inner wrappers so ngb owns the modal chrome, delete
the now-dead `.modal { display: ... }` scss workarounds in both forms,
fix the `modal-boy` -> `modal-body` typo in define-license-form, and open
all four form modals with { centered: true } so the dialog is centered.
* F3: surface newly created label without manual paging
The labels endpoint lists labels in ascending insertion order and ignores
the sort param, so a freshly created label received the highest id and
landed on the last pagination page, invisible to the admin after Save.
After a successful create, jump the labels table to the last page (derived
from the current total plus the one just added) so the new row is shown
immediately.
* F4: allow clearing a label icon on edit and preview the current icon
The label edit form could keep or replace an icon but never remove one,
and gave no indication of the current icon. Add a "Remove current icon"
checkbox and a small preview (reusing secureImageData) shown in edit mode
when the label has an icon. When clear is requested and no new file is
chosen, the table sends an empty icon array through the existing PUT path
to remove it; selecting a new file still takes precedence.
* F5: center the delete-confirmation modal and complete cs.json5 label parity
Pass { centered: true } when opening ConfirmationModalComponent from
confirmDeleteLabel() so the delete-confirmation dialog is vertically
centered like the four license/label form dialogs, removing the
top-aligned visual inconsistency.
Also add the three Czech translations that were missing from cs.json5
(table.delete.checking-tooltip, edit.icon.preview, edit.icon.clear) so
the en/cs label key sets are back in parity.
* test: expect centered option in delete-confirmation modal spec
The F5 change passes { centered: true } to modalService.open; update the
assertion to match so the label delete flow spec passes.
---------
Co-authored-by: kosarko <ko_ok@centrum.cz>
(cherry picked from commit 87cb7a7)
* Disable delete for in-use licenses, add tooltip Prevent deleting licenses that are attached to bitstreams by disabling the Delete button and showing a tooltip explaining why. Adds UI markup (ngbTooltip wrapper, aria-label, dsBtnDisabled and guarded click), a new isSelectedLicenseInUse() helper and a check in deleteLicense(). Adds unit tests covering disabled/enabled states and click behavior, and provides i18n strings for English and Czech for the disabled-tooltip. * Use hasNoValue for license id check Replace isNull with hasNoValue when validating selectedLicense.id in deleteLicense to correctly handle undefined/null values. Also add hasNoValue to the imports from shared/empty.util. * Use NgbTooltip instance in delete button spec Update clarin-license-table component spec to import NgbTooltip and retrieve the tooltip instance from the delete wrapper's injector. Replace the previous assertion that inspected the DOM's ng-reflect-ngb-tooltip attribute with an assertion on deleteTooltip.ngbTooltip to verify the expected translation key. This makes the test more robust by avoiding reliance on Angular's rendered reflection attribute. (cherry picked from commit 42c4fcc)
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
2 participants
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.
Port of ufal#134 by @amadulhaxxani to
dtq-dev.Note that this needs dataquest-dev/DSpace#1325 on the backend