diff --git a/.dspace-skills b/.dspace-skills new file mode 160000 index 00000000000..2a62a2ebff8 --- /dev/null +++ b/.dspace-skills @@ -0,0 +1 @@ +Subproject commit 2a62a2ebff88a05acf78d374003032b455843066 diff --git a/.github/actions/import-db/action.yml b/.github/actions/import-db/action.yml index 4e17c221365..3897dcad688 100644 --- a/.github/actions/import-db/action.yml +++ b/.github/actions/import-db/action.yml @@ -18,6 +18,11 @@ inputs: description: 'Admin password for DSpace' required: true type: string + LOGDIR: + description: 'Directory for log files' + required: false + type: string + default: '' runs: using: "composite" diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 11c90a843fc..4c00808d2d8 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -109,48 +109,6 @@ jobs: /bin/bash -c 'while [[ "$(curl -s -o /dev/null -w ''%{http_code}'' http://dev-5.pc:8$INSTANCE/repository/server/api)" != "200" ]]; do sleep 5; done' - import-5: - runs-on: dspace-dep-1 - if: inputs.IMPORT - needs: deploy-5 - env: - INSTANCE: '5' - ENVFILE: /opt/dspace-envs/.env.dspace.dev-5 - steps: - - uses: ./.github/actions/import-db - with: - INSTANCE: ${{ env.INSTANCE }} - DATADIR: /opt/dspace-data/clarin-dspace-oxford/ - ASSETSTORE: /opt/dspace-data/clarin-dspace-oxford/assetstore/ - LOGDIR: /log/ - ADMIN_PASSWORD: ${{ secrets.DSPACE_ADMIN_PASSWORD }} - - - name: dspace basic command - run: | - export DNAME=dspace$INSTANCE - docker logs -n 50 $DNAME - - echo "dspace version:" - docker exec $DNAME /bin/bash -c "cd /dspace/bin && ./dspace version" - - echo "dspace cleanup:" - docker exec $DNAME /bin/bash -c "cd /dspace/bin && ./dspace cleanup -v" - - echo "dspace reindex solr:" - docker exec $DNAME /bin/bash -c "cd /dspace/bin && ./dspace index-discovery -b" - - echo "dspace reindex OAI-PMH:" - docker exec $DNAME /bin/bash -c "cd /dspace/bin && ./dspace oai import -c" - - echo "dspace checker:" - docker exec $DNAME /bin/bash -c "cd /dspace/bin && ./dspace checker -v -l" - - - name: dspace healthcheck - run: | - export DNAME=dspace$INSTANCE - echo "dspace healthcheck:" - docker exec $DNAME /bin/bash -c "cd /dspace/bin && ./dspace healthcheck -v" - import-8: runs-on: dspace-dep-1 if: inputs.IMPORT @@ -202,6 +160,7 @@ jobs: rest-tests-after-deploy8: runs-on: ubuntu-latest needs: playwright-after-deploy8 + if: '!inputs.IMPORT' timeout-minutes: 120 env: GH_TOKEN: ${{ secrets.DEPLOY_DEV5_GH_ACTION_DISPATCH }} @@ -214,7 +173,7 @@ jobs: --repo "$REPO" \ --ref master \ -f CUSTOMER="${{ github.ref_name }}" \ - -f URL="http://dev-5.pc:85/repository/server/api" 2>&1) + -f URL="http://dev-5.pc:88/repository/server/api" 2>&1) RUN_ID=$(echo "$OUTPUT" | grep -oE '/runs/[0-9]+' | grep -oE '[0-9]+') @@ -243,7 +202,7 @@ jobs: --repo "$REPO" \ --ref master \ -f CUSTOMER="${{ github.ref_name }}" \ - -f URL="http://dev-5.pc:85/repository/server/api" 2>&1) + -f URL="http://dev-5.pc:88/repository/server/api" 2>&1) RUN_ID=$(echo "$OUTPUT" | grep -oE '/runs/[0-9]+' | grep -oE '[0-9]+') diff --git a/.github/workflows/new_issue_label.yml b/.github/workflows/new_issue_label.yml new file mode 100644 index 00000000000..b759334e22f --- /dev/null +++ b/.github/workflows/new_issue_label.yml @@ -0,0 +1,17 @@ +name: Auto Label New Issues + +on: + issues: + types: [opened] + +jobs: + label-issue: + runs-on: ubuntu-latest + permissions: + issues: write + + steps: + - name: Auto Label + uses: dataquest-dev/gh-actions/start@main + with: + github-token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/trigger-ui-tests.yml b/.github/workflows/trigger-ui-tests.yml index f0d53a564b8..6940cdbcf4c 100644 --- a/.github/workflows/trigger-ui-tests.yml +++ b/.github/workflows/trigger-ui-tests.yml @@ -24,7 +24,7 @@ jobs: git fetch --prune origin BRANCHES=$(git ls-remote --heads origin | awk -F'/' '{print $3"/"$4}' | grep '^customer/') - SKIP_BRANCHES=("customer/uk" "customer/mendelu-v7" "customer/palo-docker" "customer/palo-docker-rebase" "customer/palo-docker-typo-fix") + SKIP_BRANCHES=("customer/jcu" "customer/uk" "customer/mendelu-v7" "customer/palo-docker" "customer/palo-docker-rebase" "customer/palo-docker-typo-fix") for branch in $(echo "$BRANCHES" | sed -e 's/[\[\]"]//g' -e 's/,/\n/g'); do if [[ " ${SKIP_BRANCHES[@]} " =~ " ${branch} " ]]; then diff --git a/.gitmodules b/.gitmodules index 16fbb4e8d5d..3c7bc45c1fb 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,7 @@ [submodule "python_data_import/lib"] path = python_data_import/lib url = https://github.com/dataquest-dev/dspace-blackbox-testing.git +[submodule ".dspace-skills"] + path = .dspace-skills + url = git@github.com:dataquest-dev/dspace-skills.git + branch = main diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000000..5e55e477c0b --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,16 @@ + + +## Private team playbooks (dataquest) + +This repo vendors dataquest's private AI knowledge base as the `.dspace-skills/` git submodule. **If +`.dspace-skills/` is present**, treat **`.dspace-skills/AGENTS.md`** as the authoritative agent guide for this repo: +read it first, then load the matching profile (`.dspace-skills/profiles/frontend.md` for dspace-angular, +`.dspace-skills/profiles/backend.md` for DSpace) and pull skills from `.dspace-skills/skills/` on demand. Start any +PR/backport/test task from `.dspace-skills/SKILLS.md`. + +If `.dspace-skills/` is empty (you don't have access, e.g. an outside contributor), ignore this section +and proceed with the public project conventions. + +To enable: `git submodule update --init .dspace-skills` (requires access to +`dataquest-dev/dspace-skills`). + diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000000..5e55e477c0b --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,16 @@ + + +## Private team playbooks (dataquest) + +This repo vendors dataquest's private AI knowledge base as the `.dspace-skills/` git submodule. **If +`.dspace-skills/` is present**, treat **`.dspace-skills/AGENTS.md`** as the authoritative agent guide for this repo: +read it first, then load the matching profile (`.dspace-skills/profiles/frontend.md` for dspace-angular, +`.dspace-skills/profiles/backend.md` for DSpace) and pull skills from `.dspace-skills/skills/` on demand. Start any +PR/backport/test task from `.dspace-skills/SKILLS.md`. + +If `.dspace-skills/` is empty (you don't have access, e.g. an outside contributor), ignore this section +and proceed with the public project conventions. + +To enable: `git submodule update --init .dspace-skills` (requires access to +`dataquest-dev/dspace-skills`). + diff --git a/angular.json b/angular.json index a57b7582109..570c438844c 100644 --- a/angular.json +++ b/angular.json @@ -40,6 +40,7 @@ "aot": true, "assets": [ "src/assets", + "src/static-files", "src/robots.txt" ], "styles": [ diff --git a/cypress/e2e/submission-ui.cy.ts b/cypress/e2e/submission-ui.cy.ts index 1b6f9b3ca3b..d26ddfe60a2 100644 --- a/cypress/e2e/submission-ui.cy.ts +++ b/cypress/e2e/submission-ui.cy.ts @@ -283,6 +283,12 @@ describe('Create a new submission', () => { // DON'T upload any file - just check the license section is accessible createItemProcess.checkLicenseSelectionValue('Select a License ...'); + // Allow frontend to propagate the initial section status (no file => valid) + // before asserting the header icon. Mirrors the pattern used in the + // sibling "license validation when file is uploaded" test above. + cy.wait(1000); + cy.get('div[id="section_clarin-license"]').find('.card-header').should('be.visible'); + // Verify warning and error icons do NOT exist cy.get('div[id="section_clarin-license"]') .find('.card-header') @@ -294,9 +300,11 @@ describe('Create a new submission', () => { .find('.fa-exclamation-circle.text-danger') .should('not.exist'); + // Green check must eventually appear (retry-ability with a longer timeout + // handles any remaining async settle of the section status observable). cy.get('div[id="section_clarin-license"]') .find('.card-header') - .find('.fa-check-circle.text-success') + .find('.fa-check-circle.text-success', { timeout: 15000 }) .should('be.visible'); }); diff --git a/docker/docker-compose-rest.yml b/docker/docker-compose-rest.yml index 635af00d997..99530983b7c 100644 --- a/docker/docker-compose-rest.yml +++ b/docker/docker-compose-rest.yml @@ -175,7 +175,7 @@ services: cp -r /opt/solr/server/solr/configsets/search/* search precreate-core statistics /opt/solr/server/solr/configsets/statistics cp -r /opt/solr/server/solr/configsets/statistics/* statistics - exec solr -p 8983 -f -m 4g + exec solr -p 8983 -f -m "${SOLR_HEAP:-4g}" volumes: # Commented out because there are a lot of files in the assetstore assetstore: diff --git a/docs/agents.md b/docs/agents.md new file mode 100644 index 00000000000..3dfd36e8173 --- /dev/null +++ b/docs/agents.md @@ -0,0 +1,493 @@ +# DSpace Angular – Agent & Human Collaboration Guide + +> **Purpose**: A universal playbook for fixing **any** frontend issue through AI agent + human collaboration. +> The human prepares the environment and provides the issue. The agent investigates the problem visually +> (Playwright MCP), implements the fix, verifies it, and iterates until all CI checks pass. +> +> **Architecture**: Only the Angular frontend runs locally (for hot reload and code changes). +> The backend services (DSpace REST API, PostgreSQL, Solr) run in Docker containers. + +--- + +## 1. Project Facts + +| Property | Value | +|----------|-------| +| Framework | Angular 15 + Angular Universal (SSR) | +| Language | TypeScript 4.8 | +| Package manager | **Yarn 1.x** — never use `npm` | +| Node.js | **18.x** (`nvm use 18`) — Node 20+ breaks `eslint-plugin-jsdoc` | +| Unit tests | Jasmine / Karma (~5 300 specs) | +| E2E tests | Cypress 13, Chrome headless | +| Main branch | `dtq-dev` | +| Repo | `dataquest-dev/dspace-angular` | + +--- + +## 2. Human Setup — Do This Before Handing Off to the Agent + +### 2.1 One-Time Setup + +1. Install **Docker Desktop** and start it. +2. Install **nvm** (or nvm-windows): + ```bash + nvm install 18 + nvm use 18 + ``` +3. Install Yarn: `npm install -g yarn` +4. Clone the repo: + ```bash + git clone https://github.com/dataquest-dev/dspace-angular.git + cd dspace-angular + ``` + +### 2.2 Before Every Session + +1. **Start the backend services in Docker** (REST API, PostgreSQL, Solr — needed for e2e and live verification): + ```bash + docker compose -p ci -f docker/docker-compose-ci.yml up -d + docker compose -p ci -f docker/cli.yml -f docker/cli.assetstore.yml run --rm dspace-cli + ``` + Verify: `curl http://localhost:8080/server/api/core/sites` returns JSON. + > The frontend is NOT started here — it runs locally (see §3). + +2. **Node version**: `node --version` → must be `v18.x` + +3. **Heap size** (prevents OOM): + - PowerShell: `$env:NODE_OPTIONS='--max-old-space-size=4096'` + - Bash: `export NODE_OPTIONS='--max-old-space-size=4096'` + +4. **Install dependencies**: + ```bash + yarn install --frozen-lockfile + ``` + If Cypress download fails: `CYPRESS_INSTALL_BINARY=0 yarn install --frozen-lockfile` + +5. **Create a feature branch**: + ```bash + git checkout dtq-dev && git pull + git checkout -b / + ``` + +6. **Open the project in VS Code** with Copilot / agent enabled. + +### 2.3 Prompt Template — Copy, Fill In, Paste to Agent + +``` +Here is the GitHub issue to fix: + +Read the agent guide at docs/agents.md first. + +Environment info: +- Backend services (REST API, Solr, PostgreSQL) are running in Docker on default ports +- Frontend will run locally (you start it with yarn start:dev or yarn serve:ssr) +- Admin credentials if necessary for the issue: / + +Please: +1. Read the issue and understand the problem +2. Use Playwright MCP to navigate to the affected page and visually confirm the bug +3. Investigate the codebase to find the root cause +4. Implement a fix +5. Use Playwright MCP again to verify the fix is working +6. Run all CI checks (lint → circ-deps → build → unit tests) and iterate until all pass +7. Only commit and push when everything is green + +If the issue involves a specific page, here is the direct URL: +``` + +That's it. Everything below is the agent's responsibility. + +--- + +## 3. Agent Workflow + +### 3.1 The Loop: Investigate → Fix → Verify → CI + +``` +1. READ the issue — understand what's broken and where +2. DETECT with Playwright MCP — navigate to the affected page, take snapshots, + confirm the bug visually (duplicate IDs, broken layout, wrong behavior, etc.) +3. SEARCH the codebase — find the root cause in the source files +4. IMPLEMENT the fix — minimal, focused changes +5. VERIFY with Playwright MCP — navigate to the same page, confirm the fix works +6. RUN CI checks in order — fix any failures, re-verify after each code change +7. COMMIT & PUSH — only when everything passes +``` + +### 3.2 CI Pipeline — Run In This Order + +Every step must pass before proceeding to the next. + +| # | Step | Command | Time | Pass Criteria | +|---|------|---------|------|---------------| +| 1 | Lint | `yarn run lint --quiet` | ~76s | Exit 0, "All files pass linting." | +| 2 | Circular deps | `yarn run check-circ-deps` | ~34s | "No circular dependency found!" | +| 3 | Build | `yarn run build:prod` | ~6.5min | Two bundles (browser + server), no `Error:` lines | +| 4 | Unit tests | `yarn run test:headless` | ~3.5min | ~5300 specs, 0 failures | +| 5 | E2E tests | See §3.5 | ~5min | No new failures vs. `dtq-dev` baseline | + +**If any step fails**: fix the code → rerun **that same step** → only proceed when green. + +### 3.3 Running a Single Unit Test + +```bash +yarn run test:headless --include='**/path/to/component.spec.ts' +``` + +### 3.4 PowerShell Trap — Circular Dependency Check + +The `check-circ-deps` script uses `madge --exclude` with regex containing `|`. PowerShell interprets `|` as a pipeline operator. Use the stop-parsing token: + +```powershell +npx --% madge --exclude "(bitstream|bundle|collection|config-submission-form|eperson|item|version)\.model\.ts$" --circular --extensions ts ./ +``` + +### 3.5 E2E Tests (Requires Docker Backend) + +Start the SSR server: +```bash +yarn run build:prod +yarn run serve:ssr & +# Wait for http://localhost:4000 to respond +``` + +Run public-page specs (always safe, no login needed): +```bash +npx cypress run --spec "cypress/e2e/footer.cy.ts,cypress/e2e/header.cy.ts,cypress/e2e/pagenotfound.cy.ts,cypress/e2e/browse-by-title.cy.ts,cypress/e2e/browse-by-author.cy.ts,cypress/e2e/browse-by-subject.cy.ts,cypress/e2e/community-list.cy.ts,cypress/e2e/search-page.cy.ts" --browser chrome +``` + +On PowerShell, set the base URL first: +```powershell +$env:CYPRESS_BASE_URL="http://localhost:4000" +``` + +### 3.6 E2E Test Categories + +| Category | Spec files | Requires | +|----------|-----------|----------| +| **Public pages** (always safe) | `footer`, `header`, `pagenotfound`, `browse-by-*`, `community-list`, `search-page`, `feedback` | Frontend only | +| **Data-dependent** | `collection-page`, `community-page`, `item-page` | Backend + Demo Entities assetstore | +| **Login-required** | `submission*`, `admin-*`, `my-dspace`, `profile-page`, `handle-page`, `health-page` | Backend + Demo Entities + valid credentials | + +### 3.7 Commit & Push + +```bash +git add +# NEVER commit: config/config.yml, .env.* files, coverage/, cypress/videos/ +git commit -m "fix: " +git push origin ufal/ +``` + +--- + +## 4. Using Playwright MCP for Detection & Verification + +Playwright MCP is the agent's **eyes**. Use it to **see** the problem and **confirm** the fix. + +### 4.1 Navigating to a Page + +``` +browser_navigate → http://localhost:4000/path/to/page +browser_snapshot → get the accessibility tree of the page +``` + +### 4.2 Running JavaScript on the Page + +Use `browser_evaluate` to inspect the DOM. Examples: + +**Check for duplicate HTML IDs**: +```javascript +async (page) => { + return await page.evaluate(() => { + const ids = [...document.querySelectorAll('[id]')].map(el => el.id).filter(Boolean); + const counts = {}; + ids.forEach(id => { counts[id] = (counts[id] || 0) + 1; }); + const dupes = Object.entries(counts).filter(([_, c]) => c > 1); + return { total: ids.length, unique: new Set(ids).size, duplicates: dupes }; + }); +} +``` + +**Verify specific selectors still work** (backward compatibility): +```javascript +async (page) => { + return await page.evaluate(() => { + const selectors = ['input#dc_title', 'label[for=dc_title]', '.some-class']; + return selectors.map(s => `${s}: ${document.querySelectorAll(s).length}`); + }); +} +``` + +**Check console errors**: +```javascript +async (page) => { + const errors = []; + page.on('console', msg => { if (msg.type() === 'error') errors.push(msg.text()); }); + await page.reload(); + await page.waitForTimeout(3000); + return errors; +} +``` + +### 4.3 Logging In (When Needed) + +Some pages require authentication (submission forms, admin panels). +Test credentials are defined in `cypress.config.ts` (look for `DSPACE_TEST_ADMIN_USER` / `DSPACE_TEST_ADMIN_PASSWORD`). + +1. Navigate to the login page +2. Dismiss the DiscoJuice/Shibboleth overlay if it appears: + ```javascript + await page.evaluate(() => { + document.querySelectorAll('[class*="discojuice"]').forEach(el => el.remove()); + }); + ``` +3. Fill in credentials and submit the login form +4. Navigate to the target page + +### 4.4 Screenshots for Evidence + +Use `browser_take_screenshot` before and after the fix to document the change. + +### 4.5 Dev Server vs. SSR Mode + +| Mode | Command | CORS | Hot Reload | +|------|---------|------|------------| +| Dev server | `yarn start:dev` | ⚠️ Must match backend's `dspace.ui.url` port | ✅ Yes | +| SSR mode | `yarn build:prod` → `yarn serve:ssr` | ✅ No issues (server-side proxy) | ❌ No | + +**Prefer SSR mode** for Playwright verification. It avoids CORS issues and reflects the real production behavior. Use dev server only when you need rapid iteration. + +--- + +## 5. Known Pre-Existing E2E Failures + +Before panicking about a red test, check if it was already failing on `dtq-dev`: + +| Symptom | Affected Specs | Cause | +|---------|---------------|-------| +| DiscoJuice `display: none` click fails | Login-dependent tests | Docker env popup issue | +| `cy.wait('@viewevent')` timeout | `collection-page.cy.ts` | Matomo not configured | +| Entity redirect fails | `item-page.cy.ts` | Backend routing issue | +| `link-in-text-block` a11y violation | `privacy.cy.ts`, `end-user-agreement.cy.ts` | CSS styling issue | + +**Rule**: Only fix failures that YOUR changes caused. If a test was already failing on `dtq-dev`, leave it alone. + +--- + +## 6. Pitfalls & Lessons Learned + +These come from real agent sessions. Read them before writing code. + +### 6.1 Angular Template Binding + +```html + +
+ + +
+``` + +This applies to all `aria-*` and custom HTML attributes. Always use `[attr.X]="expression"` instead of `X="{{ interpolation }}"`. + +### 6.2 Dynamic HTML IDs Must Be Valid + +HTML `id` attributes must not contain whitespace or special characters. Sanitize dynamic data: +```typescript +sanitizedName = rawName.replace(/\s+/g, ''); +``` + +### 6.3 Conditional `aria-describedby` + +If a description element is conditionally rendered (`*ngIf`), the `aria-describedby` pointing to it must also be conditional: +```html +{{ label }} + +``` + +### 6.4 Never Change Form Element IDs Unless You Have To + +Submission forms use IDs like `input#dc_title`, `label[for=local_hasCMDI]` etc. These are referenced by Cypress e2e selectors and `formModel` definitions. Changing them cascades into dozens of broken tests. Unless the issue specifically requires it, leave form IDs alone. + +### 6.5 `aria-labelledby` Must Match Label IDs + +Many child form components reference `'label_' + model.id` in `[attr.aria-labelledby]`. If you change a label's `[id]`, you break accessibility wiring in 13+ template files. Keep label IDs stable — if deduplicating, change the form control ID, not the label. + +### 6.6 Static State Leaks Between Tests + +This project uses `{ teardown: { destroyAfterEach: false } }` (see `src/test.ts`), so `ngOnDestroy` does NOT run between unit tests. Any static registry or singleton must be explicitly cleared in the global `afterEach` in `src/test.ts`, or it will leak state across specs. + +### 6.7 When to Revert Your Approach + +If your change causes dozens of cascading test failures, **stop and rethink**. It's cheaper to revert the whole approach than to chase 50 broken selectors. A minimal, targeted fix is always better than a sweeping refactor. + +### 6.8 PowerShell-Specific Issues + +| Issue | Solution | +|-------|---------| +| `check-circ-deps` fails with pipe error | Use `npx --%` stop-parsing token | +| `&&` not valid in PowerShell 5.1 | Use `;` to chain commands | +| Env vars don't persist | Use `$env:VAR='val'` before each command | + +### 6.9 Build Warnings to Ignore + +These are pre-existing and harmless: +- `Warning: X.component.ts is unused` (theme components) +- `Warning: CommonJS or AMD dependencies` +- `Warning: bundle exceeded maximum budget` + +Real errors always start with `Error:`. + +### 6.10 SSR Compatibility + +All code must work with Angular Universal. Never use `document` or `window` directly: +```typescript +import { isPlatformBrowser } from '@angular/common'; +if (isPlatformBrowser(this.platformId)) { + // browser-only code +} +``` + +### 6.11 Always Check `.spec.ts` When Changing Templates + +When changing an `id`, `class`, or tag in a template, check the corresponding `.spec.ts`: +```bash +grep -n "By.css" src/app/.../my-component.component.spec.ts +``` +If a test uses a selector you changed, update it or the test will fail. + +--- + +## 7. Example: Duplicate HTML IDs in ng-dynamic-forms + +> This section documents a real fix for reference. The patterns here apply to similar problems. + +**Problem**: The submission edit page renders dynamic form fields via `@ng-dynamic-forms`. When the same metadata field appears in multiple `DynamicRowGroupModel` groups, each instance gets the **same HTML ID**, producing duplicates. + +**Root cause**: `@ng-dynamic-forms/core` `getElementId(model)` only handles `DynamicFormArrayGroupModel` parents. It does NOT handle `DynamicFormGroupModel` / `DynamicRowGroupModel` parents, even though they have unique auto-generated IDs. + +**Solution**: A `UniqueIdRegistry` static class that: +1. First occurrence of a base ID → returns the original ID (preserves Cypress selectors) +2. Subsequent occurrences → appends `_1`, `_2`, etc. via a monotonic counter +3. Components call `register()` on init and `release()` on destroy + +The `get id()` override was added to `DsDynamicFormControlContainerComponent` and `DsDynamicScrollableDropdownComponent`. + +**Also fixed**: `id="license_option_{{ license.id }}"` → `[id]="'license_option_' + license.id"` (interpolation rendered empty because Angular consumed it before DOM render). + +--- + +## 8. Docker Reference (Backend Services Only) + +Docker runs only the backend services. The Angular frontend always runs locally. + +### Compose Files + +| File | Purpose | +|------|--------| +| `docker-compose-ci.yml` | Backend stack for CI/testing (DSpace REST + PostgreSQL + Solr) | +| `docker-compose.yml` | Full stack dev (includes a frontend container — but we run frontend locally instead) | +| `docker-compose-rest.yml` | REST backend only | +| `cli.yml` + `cli.assetstore.yml` | Load Demo Entities test data | + +### Default Ports + +| Service | Port | Where | +|---------|------|-------| +| DSpace REST API | 8080 | Docker | +| PostgreSQL | 5432 | Docker | +| Solr | 8983 | Docker | +| Angular Frontend | 4000 | **Local** (not Docker) | + +### Test Variables + +Test credentials and UUIDs for e2e tests are defined in `cypress.config.ts` under the `env` section. Look for `DSPACE_TEST_ADMIN_USER`, `DSPACE_TEST_ADMIN_PASSWORD`, `DSPACE_TEST_COMMUNITY`, etc. These are standard DSpace demo-instance values — do **not** replace them with real credentials. + +### Connecting the Local Frontend to Docker Backend + +The backend's `dspace.ui.url` determines CORS allowed origins. Your local dev server port **must match** this URL, or browser XHR requests will be blocked. + +1. Check which UI port the backend expects (look at the Docker compose env vars for `dspace.ui.url`) +2. Edit `config/config.yml` temporarily to match those ports +3. Run: `yarn start:dev` +4. **Never commit** `config/config.yml` changes — add it to your local `.gitignore` or always `git checkout` before committing + +Alternatively, use SSR mode (`yarn build:prod` → `yarn serve:ssr`) which proxies API calls server-side and avoids CORS entirely. This is the **recommended approach** for Playwright verification. + +### Docker Commands + +```bash +# Start +docker compose -p ci -f docker/docker-compose-ci.yml up -d +docker compose -p ci -f docker/cli.yml -f docker/cli.assetstore.yml run --rm dspace-cli + +# Verify +curl http://localhost:8080/server/api/core/sites + +# Stop +docker compose -p ci -f docker/docker-compose-ci.yml down +``` + +--- + +## 9. Common Errors & Solutions + +| Error | Fix | +|-------|-----| +| `The engine "node" is incompatible` | `nvm use 18` | +| `Cypress App could not be downloaded` | `CYPRESS_INSTALL_BINARY=0 yarn install --frozen-lockfile` | +| Out of memory during build | `$env:NODE_OPTIONS='--max-old-space-size=4096'` | +| `yarn.lock` merge conflicts | `git checkout --theirs yarn.lock; yarn install; git add yarn.lock` | +| `Can't bind to 'aria-labelledby'` | Use `[attr.aria-labelledby]="expr"` not interpolation | +| Cypress can't connect | Run `yarn serve:ssr` first; check `CYPRESS_BASE_URL` | +| CORS errors with dev server | Match port to backend's `dspace.ui.url`, or use SSR mode | +| `madge` pipe error on PowerShell | Use `npx --%` stop-parsing token | +| Test fails with `Cannot find element` | You renamed an ID — update the test selector too | +| DiscoJuice overlay blocks login | `document.querySelector('.discojuice-overlay')?.remove()` | + +--- + +## 10. Code Conventions + +| Convention | Rule | +|------------|------| +| Component prefix | `ds-` | +| Strings | Single quotes | +| Indentation | 2 spaces | +| Imports | Specific files, not barrels | +| Lodash | `import get from 'lodash/get'` | +| SSR | No raw `document`/`window` — use `isPlatformBrowser()` | +| Tests | Co-located `.spec.ts` files | +| JSDoc | Required on all new public methods | + +--- + +## 11. Quick Reference + +```bash +# Setup +nvm use 18 +yarn install --frozen-lockfile + +# CI validation (run in order) +yarn run lint --quiet +yarn run check-circ-deps +yarn run build:prod +yarn run test:headless + +# Single test +yarn run test:headless --include='**/path/to/test.spec.ts' + +# Dev server +yarn run start:dev + +# E2E +yarn run build:prod +yarn run serve:ssr & +npx cypress run --spec "cypress/e2e/footer.cy.ts" --browser chrome + +# Docker backend +docker compose -p ci -f docker/docker-compose-ci.yml up -d +docker compose -p ci -f docker/cli.yml -f docker/cli.assetstore.yml run --rm dspace-cli +docker compose -p ci -f docker/docker-compose-ci.yml down +``` diff --git a/src/app/bitstream-page/edit-bitstream-page/edit-bitstream-page.component.spec.ts b/src/app/bitstream-page/edit-bitstream-page/edit-bitstream-page.component.spec.ts index c81926d83d9..542124b2b63 100644 --- a/src/app/bitstream-page/edit-bitstream-page/edit-bitstream-page.component.spec.ts +++ b/src/app/bitstream-page/edit-bitstream-page/edit-bitstream-page.component.spec.ts @@ -414,6 +414,19 @@ describe('EditBitstreamPageComponent', () => { comp.navigateToItemEditBitstreams(); expect(router.navigate).toHaveBeenCalledWith([getEntityEditRoute(null, 'some-uuid1'), 'bitstreams']); }); + + it('should resolve itemId from bundle.item when query param is missing', () => { + const mockItem = new Item(); + mockItem.uuid = 'resolved-uuid-456'; + const mockBundle = { + item: createSuccessfulRemoteDataObject$(mockItem) + }; + (comp as any).bundle = mockBundle; + comp.itemId = undefined; + comp.navigateToItemEditBitstreams(); + fixture.detectChanges(); + expect(router.navigate).toHaveBeenCalledWith([getEntityEditRoute(null, 'resolved-uuid-456'), 'bitstreams']); + }); }); }); diff --git a/src/app/bitstream-page/edit-bitstream-page/edit-bitstream-page.component.ts b/src/app/bitstream-page/edit-bitstream-page/edit-bitstream-page.component.ts index 082ffd55a99..b33b2c72776 100644 --- a/src/app/bitstream-page/edit-bitstream-page/edit-bitstream-page.component.ts +++ b/src/app/bitstream-page/edit-bitstream-page/edit-bitstream-page.component.ts @@ -420,23 +420,38 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy { ); const bundle$ = bitstream$.pipe( - switchMap((bitstream: Bitstream) => bitstream.bundle), - getFirstSucceededRemoteDataPayload(), + switchMap((bitstream: Bitstream) => { + if (hasValue(bitstream) && hasValue(bitstream.bundle)) { + return bitstream.bundle.pipe(getFirstSucceededRemoteDataPayload()); + } + return observableOf(undefined); + }), ); const primaryBitstream$ = bundle$.pipe( - hasValueOperator(), - switchMap((bundle: Bundle) => this.bitstreamService.findByHref(bundle._links.primaryBitstream.href)), - getFirstSucceededRemoteDataPayload(), + switchMap((bundle: Bundle) => { + if (hasValue(bundle) && hasValue(bundle._links) && hasValue(bundle._links.primaryBitstream) && hasValue(bundle._links.primaryBitstream.href)) { + return this.bitstreamService.findByHref(bundle._links.primaryBitstream.href).pipe(getFirstSucceededRemoteDataPayload()); + } + return observableOf(undefined); + }), ); const item$ = bundle$.pipe( - switchMap((bundle: Bundle) => bundle.item), - getFirstSucceededRemoteDataPayload(), + switchMap((bundle: Bundle) => { + if (hasValue(bundle) && hasValue(bundle.item)) { + return bundle.item.pipe(getFirstSucceededRemoteDataPayload()); + } + return observableOf(undefined); + }), ); const format$ = bitstream$.pipe( - switchMap(bitstream => bitstream.format), - getFirstSucceededRemoteDataPayload(), + switchMap((bitstream: Bitstream) => { + if (hasValue(bitstream) && hasValue(bitstream.format)) { + return bitstream.format.pipe(getFirstSucceededRemoteDataPayload()); + } + return observableOf(undefined); + }), ); this.subs.push( @@ -449,15 +464,23 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy { ).subscribe(([bitstream, bundle, primaryBitstream, item, format]) => { this.bitstream = bitstream as Bitstream; this.bundle = bundle; - this.selectedFormat = format; + if (hasValue(format)) { + this.selectedFormat = format; + } // hasValue(primaryBitstream) because if there's no primaryBitstream on the bundle it will // be a success response, but empty this.primaryBitstreamUUID = hasValue(primaryBitstream) ? primaryBitstream.uuid : null; - this.itemId = item.uuid; + if (hasValue(item)) { + this.itemId = item.uuid; + } this.setIiifStatus(this.bitstream); }), format$.pipe(take(1)).subscribe( - (format) => this.originalFormat = format, + (format) => { + if (hasValue(format)) { + this.originalFormat = format; + } + }, ), ); @@ -722,7 +745,33 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy { * otherwise retrieve the item ID based on the owning bundle's link */ navigateToItemEditBitstreams() { - this.router.navigate([getEntityEditRoute(this.entityType, this.itemId), 'bitstreams']); + const navigate = () => this.router.navigate([getEntityEditRoute(this.entityType, this.itemId), 'bitstreams']); + + if (hasValue(this.itemId)) { + navigate(); + return; + } + + if (hasValue(this.bundle) && hasValue(this.bundle.item)) { + this.subs.push( + this.bundle.item.pipe( + getFirstCompletedRemoteData(), + ).subscribe((itemRd: RemoteData) => { + if (itemRd.hasSucceeded && hasValue(itemRd.payload)) { + this.itemId = itemRd.payload.uuid; + if (!hasValue(this.entityType)) { + this.entityType = itemRd.payload.firstMetadataValue('dspace.entity.type'); + } + navigate(); + } else { + this.location.back(); + } + }), + ); + return; + } + + this.location.back(); } /** @@ -732,26 +781,38 @@ export class EditBitstreamPageComponent implements OnInit, OnDestroy { */ setIiifStatus(bitstream: Bitstream) { + if (!hasValue(bitstream) || !hasValue(bitstream.bundle) || !hasValue(bitstream.format)) { + this.isIIIF = false; + return; + } + const regexExcludeBundles = /OTHERCONTENT|THUMBNAIL|LICENSE/; const regexIIIFItem = /true|yes/i; - const isImage$ = this.bitstream.format.pipe( + const isImage$ = bitstream.format.pipe( getFirstSucceededRemoteData(), map((format: RemoteData) => format.payload.mimetype.includes('image/'))); - const isIIIFBundle$ = this.bitstream.bundle.pipe( + const isIIIFBundle$ = bitstream.bundle.pipe( getFirstSucceededRemoteData(), map((bundle: RemoteData) => this.dsoNameService.getName(bundle.payload).match(regexExcludeBundles) == null)); - const isEnabled$ = this.bitstream.bundle.pipe( - getFirstSucceededRemoteData(), - map((bundle: RemoteData) => bundle.payload.item.pipe( - getFirstSucceededRemoteData(), - map((item: RemoteData) => - (item.payload.firstMetadataValue('dspace.iiif.enabled') && - item.payload.firstMetadataValue('dspace.iiif.enabled').match(regexIIIFItem) !== null) - )))); + const isEnabled$ = bitstream.bundle.pipe( + getFirstSucceededRemoteDataPayload(), + switchMap((bundle: Bundle) => { + if (hasValue(bundle) && hasValue(bundle.item)) { + return bundle.item.pipe( + getFirstSucceededRemoteDataPayload(), + map((item: Item) => { + const iiifEnabledValue = item.firstMetadataValue('dspace.iiif.enabled'); + return hasValue(iiifEnabledValue) && iiifEnabledValue.match(regexIIIFItem) !== null; + }) + ); + } + return observableOf(false); + }) + ); const iiifSub = combineLatest( isImage$, diff --git a/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.html b/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.html index 729b0f781d5..8cc2aaee145 100644 --- a/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.html +++ b/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.html @@ -5,7 +5,7 @@ [(ngModel)]="searchingLicenseName" class="form-control ng-pristine ng-valid ng-touched" [placeholder]="'clarin-license.button.search.placeholder' | translate"/> - + @@ -76,11 +76,92 @@
- + + +
+ +
+

{{'clarin.license.label.section.title' | translate}}

+ +
+ +
+ + + + + + + + + + + + + + + + + + + + + + +
{{'clarin.license.label.table.header.label' | translate}}{{'clarin.license.label.table.header.title' | translate}}{{'clarin.license.label.table.header.extended' | translate}}{{'clarin.license.label.table.header.icon' | translate}}{{'clarin.license.label.table.header.actions' | translate}}
{{label?.label}}{{label?.title}}{{label?.extended ? ('clarin.license.label.table.boolean.yes' | translate) : ('clarin.license.label.table.boolean.no' | translate)}} + + {{label?.icon?.length > 0 ? ('clarin.license.label.table.icon.available' | translate) : ('clarin.license.label.table.icon.none' | translate)}} + + + + + + + +
{{'clarin.license.label.table.empty' | translate}}
+
+ + + {{'clarin.license.label.table.loading' | translate}} + +
+
diff --git a/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.scss b/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.scss index b4dab6de9bf..2ea1fa73dbf 100644 --- a/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.scss +++ b/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.scss @@ -15,3 +15,7 @@ width: 3.5%; max-width: 3.5%; } + +.labels-actions-column { + width: 11rem; +} diff --git a/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.spec.ts b/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.spec.ts index 0cc824bdd8b..aec589559f0 100644 --- a/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.spec.ts +++ b/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.spec.ts @@ -1,9 +1,11 @@ -import { ComponentFixture, fakeAsync, TestBed } from '@angular/core/testing'; +import { ComponentFixture, fakeAsync, TestBed, tick } from '@angular/core/testing'; +import { By } from '@angular/platform-browser'; import { ClarinLicenseTableComponent } from './clarin-license-table.component'; import { NotificationsServiceStub } from '../../shared/testing/notifications-service.stub'; import { ClarinLicenseDataService } from '../../core/data/clarin/clarin-license-data.service'; import { RequestService } from '../../core/data/request.service'; -import { of as observableOf } from 'rxjs'; +import { EventEmitter } from '@angular/core'; +import { of as observableOf, throwError } from 'rxjs'; import { SharedModule } from '../../shared/shared.module'; import { CommonModule } from '@angular/common'; import { ReactiveFormsModule } from '@angular/forms'; @@ -14,23 +16,32 @@ import { PaginationServiceStub } from '../../shared/testing/pagination-service.s import { NotificationsService } from '../../shared/notifications/notifications.service'; import { defaultPagination } from '../clarin-license-table-pagination'; import { ClarinLicenseLabelDataService } from '../../core/data/clarin/clarin-license-label-data.service'; -import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; +import { NgbActiveModal, NgbModal, NgbTooltip } from '@ng-bootstrap/ng-bootstrap'; import { HostWindowService } from '../../shared/host-window.service'; import { HostWindowServiceStub } from '../../shared/testing/host-window-service.stub'; import { createdLicenseLabelRD$, createdLicenseRD$, mockExtendedLicenseLabel, + mockLicenseLabelListRD$, mockLicense, mockLicenseRD$, mockNonExtendedLicenseLabel, successfulResponse } from '../../shared/testing/clarin-license-mock'; import {GroupDataService} from '../../core/eperson/group-data.service'; -import {createSuccessfulRemoteDataObject$} from '../../shared/remote-data.utils'; +import {createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$} from '../../shared/remote-data.utils'; +import { createFailedRemoteDataObject$, createNoContentRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { createFailedRemoteDataObject } from '../../shared/remote-data.utils'; import {createPaginatedList} from '../../shared/testing/utils.test'; import {LinkHeadService} from '../../core/services/link-head.service'; import {ConfigurationDataService} from '../../core/data/configuration-data.service'; import {ConfigurationProperty} from '../../core/shared/configuration-property.model'; import {SearchConfigurationService} from '../../core/shared/search/search-configuration.service'; +import { DefineLicenseLabelFormComponent } from './modal/define-license-label-form/define-license-label-form.component'; +import { ConfirmationModalComponent } from '../../shared/confirmation-modal/confirmation-modal.component'; +import { ClarinLicenseLabel } from '../../core/shared/clarin/clarin-license-label.model'; +import { ClarinLicense } from '../../core/shared/clarin/clarin-license.model'; +import { buildPaginatedList } from '../../core/data/paginated-list.model'; +import { PageInfo } from '../../core/shared/page-info.model'; describe('ClarinLicenseTableComponent', () => { let component: ClarinLicenseTableComponent; @@ -40,10 +51,14 @@ describe('ClarinLicenseTableComponent', () => { let clarinLicenseLabelDataService: ClarinLicenseLabelDataService; let requestService: RequestService; let notificationService: NotificationsServiceStub; - let modalStub: NgbActiveModal; + let activeModalStub: NgbActiveModal; + let modalServiceStub: jasmine.SpyObj; let groupsDataService: GroupDataService; let service: ConfigurationDataService; let searchConfigurationServiceStub: SearchConfigurationService; + let labelEditModalRef: any; + let labelDeleteModalRef: any; + let paginationServiceStub: PaginationServiceStub; beforeEach(async () => { notificationService = new NotificationsServiceStub(); @@ -51,18 +66,41 @@ describe('ClarinLicenseTableComponent', () => { findAll: mockLicenseRD$, create: createdLicenseRD$, put: createdLicenseRD$, + delete: createNoContentRemoteDataObject$(), searchBy: mockLicenseRD$, getLinkPath: observableOf('') }); clarinLicenseLabelDataService = jasmine.createSpyObj('clarinLicenseLabelService', { - create: createdLicenseLabelRD$ + create: createdLicenseLabelRD$, + findAll: mockLicenseLabelListRD$, + put: createdLicenseLabelRD$, + delete: observableOf({ hasSucceeded: true }) }); requestService = jasmine.createSpyObj('requestService', { send: observableOf('response'), getByUUID: observableOf(successfulResponse), generateRequestId: observableOf('123456'), }); - modalStub = jasmine.createSpyObj('modalService', ['close', 'open']); + activeModalStub = jasmine.createSpyObj('activeModal', ['close', 'open']); + modalServiceStub = jasmine.createSpyObj('modalService', ['open']); + labelEditModalRef = { + componentInstance: {}, + result: Promise.resolve(null) + }; + labelDeleteModalRef = { + componentInstance: { + response: new EventEmitter() + } + }; + modalServiceStub.open.and.callFake((modalComponent) => { + if (modalComponent === DefineLicenseLabelFormComponent) { + return labelEditModalRef; + } + if (modalComponent === ConfirmationModalComponent) { + return labelDeleteModalRef; + } + return { componentInstance: {}, result: Promise.resolve(null) } as any; + }); groupsDataService = jasmine.createSpyObj('groupsDataService', { findListByHref: createSuccessfulRemoteDataObject$(createPaginatedList([])), getGroupRegistryRouterLink: '' @@ -85,6 +123,7 @@ describe('ClarinLicenseTableComponent', () => { updateFixedFilter: jasmine.createSpy('updateFixedFilter'), setPaginationId: jasmine.createSpy('setPaginationId') }); + paginationServiceStub = new PaginationServiceStub(); await TestBed.configureTestingModule({ imports: [ @@ -99,9 +138,10 @@ describe('ClarinLicenseTableComponent', () => { { provide: RequestService, useValue: requestService }, { provide: ClarinLicenseDataService, useValue: clarinLicenseDataService }, { provide: ClarinLicenseLabelDataService, useValue: clarinLicenseLabelDataService }, - { provide: PaginationService, useValue: new PaginationServiceStub() }, + { provide: PaginationService, useValue: paginationServiceStub }, { provide: NotificationsService, useValue: notificationService }, - { provide: NgbActiveModal, useValue: modalStub }, + { provide: NgbActiveModal, useValue: activeModalStub }, + { provide: NgbModal, useValue: modalServiceStub }, { provide: HostWindowService, useValue: new HostWindowServiceStub(0) }, { provide: GroupDataService, useValue: groupsDataService }, { provide: LinkHeadService, useValue: linkHeadService }, @@ -181,4 +221,399 @@ describe('ClarinLicenseTableComponent', () => { expect((component as any).clarinLicenseService.searchBy).toHaveBeenCalled(); expect((component as ClarinLicenseTableComponent).licensesRD$).not.toBeNull(); }); + + describe('license delete button', () => { + const getDeleteControls = () => { + const actionsRow = fixture.debugElement.query(By.css('.mt-2')); + const deleteWrapper = actionsRow.query(By.css('.btn-group.pr-1:last-child span')); + const deleteButton = deleteWrapper.query(By.css('button.btn-danger')); + return { deleteWrapper, deleteButton }; + }; + + beforeEach(() => { + (clarinLicenseDataService.delete as jasmine.Spy).calls.reset(); + }); + + it('should disable delete button and expose tooltip when selected license has bitstreams', () => { + component.selectedLicense = Object.assign({}, mockLicense, { bitstreams: 2 }); + fixture.detectChanges(); + + const { deleteWrapper, deleteButton } = getDeleteControls(); + const deleteTooltip = deleteWrapper.injector.get(NgbTooltip); + + expect(deleteButton.attributes['aria-disabled']).toBe('true'); + expect(deleteButton.nativeElement.classList.contains('disabled')).toBeTrue(); + expect((deleteWrapper.nativeElement as HTMLElement).getAttribute('tabindex')).toBe('0'); + expect(deleteTooltip.ngbTooltip as string).toContain('clarin-license.button.delete-l'); + }); + + it('should not call delete when clicking disabled delete button', () => { + component.selectedLicense = Object.assign({}, mockLicense, { bitstreams: 1 }); + fixture.detectChanges(); + + const { deleteButton } = getDeleteControls(); + deleteButton.nativeElement.click(); + + expect((clarinLicenseDataService.delete as jasmine.Spy)).not.toHaveBeenCalled(); + }); + + it('should enable delete button and call delete when selected license has no bitstreams', () => { + component.selectedLicense = Object.assign({}, mockLicense, { bitstreams: 0 }); + fixture.detectChanges(); + + const { deleteWrapper, deleteButton } = getDeleteControls(); + deleteButton.nativeElement.click(); + + expect(deleteButton.attributes['aria-disabled']).toBe('false'); + expect(deleteButton.nativeElement.classList.contains('disabled')).toBeFalse(); + expect((deleteWrapper.nativeElement as HTMLElement).getAttribute('tabindex')).toBeNull(); + expect((clarinLicenseDataService.delete as jasmine.Spy)).toHaveBeenCalledWith(String(mockLicense.id)); + }); + }); + + describe('label edit flow', () => { + beforeEach(() => { + notificationService.success.calls.reset(); + notificationService.error.calls.reset(); + (clarinLicenseLabelDataService.put as jasmine.Spy).calls.reset(); + }); + + it('should open edit modal with the selected label when editLabel is called', () => { + component.editLabel(mockExtendedLicenseLabel); + + expect(modalServiceStub.open).toHaveBeenCalledWith(DefineLicenseLabelFormComponent, { centered: true }); + expect(labelEditModalRef.componentInstance.clarinLicenseLabel).toBe(mockExtendedLicenseLabel); + }); + + it('should call clarinLicenseLabelService.put with updated label on modal submit', fakeAsync(() => { + const refreshSpy = spyOn(component, 'refreshLabels').and.stub(); + const reloadLicensesSpy = spyOn(component, 'loadAllLicenses').and.stub(); + labelEditModalRef.result = Promise.resolve({ + label: 'EDIT', + title: 'Edited title', + extended: false + }); + + component.editLabel(mockExtendedLicenseLabel); + tick(); + + expect((clarinLicenseLabelDataService.put as jasmine.Spy)).toHaveBeenCalled(); + const putArgument = (clarinLicenseLabelDataService.put as jasmine.Spy).calls.mostRecent().args[0]; + expect(putArgument.id).toBe(mockExtendedLicenseLabel.id); + expect(putArgument._links).toEqual(mockExtendedLicenseLabel._links); + expect(putArgument.label).toBe('EDIT'); + expect(putArgument.title).toBe('Edited title'); + expect(putArgument.extended).toBeFalse(); + expect(notificationService.success).toHaveBeenCalled(); + expect(refreshSpy).toHaveBeenCalled(); + expect(reloadLicensesSpy).toHaveBeenCalled(); + })); + + it('should clear the icon when clearIcon is set and no new file is selected', fakeAsync(() => { + spyOn(component, 'refreshLabels').and.stub(); + spyOn(component, 'loadAllLicenses').and.stub(); + const labelWithIcon = Object.assign(new ClarinLicenseLabel(), { + ...mockExtendedLicenseLabel, + icon: [1, 2, 3] + }); + + component.editLicenseLabel({ + label: 'CLR', + title: 'Cleared icon', + extended: false, + clearIcon: true + }, labelWithIcon); + tick(); + + const putArgument = (clarinLicenseLabelDataService.put as jasmine.Spy).calls.mostRecent().args[0]; + expect(putArgument.icon).toEqual([]); + expect(notificationService.success).toHaveBeenCalled(); + })); + + it('should keep the existing icon when clearIcon is not set and no new file is selected', fakeAsync(() => { + spyOn(component, 'refreshLabels').and.stub(); + spyOn(component, 'loadAllLicenses').and.stub(); + const labelWithIcon = Object.assign(new ClarinLicenseLabel(), { + ...mockExtendedLicenseLabel, + icon: [1, 2, 3] + }); + + component.editLicenseLabel({ + label: 'KEP', + title: 'Kept icon', + extended: false, + clearIcon: false + }, labelWithIcon); + tick(); + + const putArgument = (clarinLicenseLabelDataService.put as jasmine.Spy).calls.mostRecent().args[0]; + expect(putArgument.icon).toBeUndefined(); + })); + + it('should show error notification on failed edit', fakeAsync(() => { + spyOn(component, 'refreshLabels').and.stub(); + (clarinLicenseLabelDataService.put as jasmine.Spy).and.returnValue(createFailedRemoteDataObject$('put failed', 500)); + + component.editLicenseLabel({ + label: 'ERR', + title: 'Failed title', + extended: true + }, mockExtendedLicenseLabel); + tick(); + + expect(notificationService.error).toHaveBeenCalled(); + })); + }); + + describe('label create pagination', () => { + it('should jump to the last labels page after a successful create so the new label is visible', () => { + paginationServiceStub.updateRoute.calls.reset(); + const refreshSpy = spyOn(component, 'refreshLabels').and.stub(); + // 25 existing labels, page size 10 -> after adding one (26) the new label is on page 3. + (component as any).labelsRD$.next( + createSuccessfulRemoteDataObject(buildPaginatedList( + Object.assign(new PageInfo(), { totalElements: 25, elementsPerPage: 10 }), [])) + ); + + component.createClarinLicenseLabel(mockNonExtendedLicenseLabel, [], 'ok', 'err'); + + expect(paginationServiceStub.updateRoute).toHaveBeenCalledWith( + (component as any).labelPaginationOptions.id, { page: 3 }); + expect(refreshSpy).toHaveBeenCalled(); + }); + }); + + describe('label delete flow', () => { + beforeEach(() => { + notificationService.success.calls.reset(); + notificationService.error.calls.reset(); + (clarinLicenseLabelDataService.delete as jasmine.Spy).calls.reset(); + labelDeleteModalRef.componentInstance.response = new EventEmitter(); + }); + + it('should open confirmation modal when confirmDeleteLabel is called', () => { + component.confirmDeleteLabel(mockNonExtendedLicenseLabel); + + expect(modalServiceStub.open).toHaveBeenCalledWith(ConfirmationModalComponent, { centered: true }); + expect(labelDeleteModalRef.componentInstance.headerLabel).toBe('clarin.license.label.delete.confirm.title'); + expect(labelDeleteModalRef.componentInstance.infoLabel).toBe('clarin.license.label.delete.confirm.message'); + expect(labelDeleteModalRef.componentInstance.dso.name).toBe(mockNonExtendedLicenseLabel.label); + }); + + it('should call clarinLicenseLabelService.delete with correct id on confirmation', fakeAsync(() => { + const refreshSpy = spyOn(component, 'refreshLabels').and.stub(); + const reloadLicensesSpy = spyOn(component, 'loadAllLicenses').and.stub(); + (clarinLicenseLabelDataService.delete as jasmine.Spy).and.returnValue(createNoContentRemoteDataObject$()); + + component.confirmDeleteLabel(mockNonExtendedLicenseLabel); + labelDeleteModalRef.componentInstance.response.emit(true); + tick(); + + expect((clarinLicenseLabelDataService.delete as jasmine.Spy)).toHaveBeenCalledWith(String(mockNonExtendedLicenseLabel.id)); + expect(notificationService.success).toHaveBeenCalled(); + expect(refreshSpy).toHaveBeenCalled(); + expect(reloadLicensesSpy).toHaveBeenCalled(); + })); + + it('should show error notification on failed delete', () => { + spyOn(component, 'refreshLabels').and.stub(); + (clarinLicenseLabelDataService.delete as jasmine.Spy).and.returnValue(throwError(() => new Error('delete failed'))); + + component.confirmDeleteLabel(mockNonExtendedLicenseLabel); + labelDeleteModalRef.componentInstance.response.emit(true); + + expect(notificationService.error).toHaveBeenCalled(); + }); + + it('should not call delete service when confirmation is cancelled', () => { + component.confirmDeleteLabel(mockNonExtendedLicenseLabel); + labelDeleteModalRef.componentInstance.response.emit(false); + + expect((clarinLicenseLabelDataService.delete as jasmine.Spy)).not.toHaveBeenCalled(); + }); + }); + + describe('label row actions', () => { + const linkedLabel = Object.assign(new ClarinLicenseLabel(), { + id: 200, + label: 'LNKD', + title: 'Linked', + extended: false, + icon: null, + _links: { + self: { + href: 'url.linked' + } + } + }); + + const unlinkedLabel = Object.assign(new ClarinLicenseLabel(), { + id: 201, + label: 'UNLK', + title: 'Unlinked', + extended: false, + icon: null, + _links: { + self: { + href: 'url.unlinked' + } + } + }); + + const linkedLicense = Object.assign(new ClarinLicense(), { + ...mockLicense, + clarinLicenseLabel: linkedLabel, + extendedClarinLicenseLabels: [] + }); + + beforeEach(() => { + (component as any).labelsRD$.next( + createSuccessfulRemoteDataObject(buildPaginatedList(new PageInfo(), [linkedLabel, unlinkedLabel])) + ); + (component as any).inUseLabelIds = new Set([String(linkedLicense.clarinLicenseLabel.id)]); + (component as any).labelUsageReady$.next(true); + fixture.detectChanges(); + }); + + it('should disable delete on all rows until the usage crawl has finished', () => { + (component as any).labelUsageReady$.next(false); + fixture.detectChanges(); + + const labelRows = fixture.debugElement.queryAll(By.css('.labels-section tbody tr')); + const deleteButtons = labelRows.map((row) => row.queryAll(By.css('button'))[1]); + + deleteButtons.forEach((deleteButton) => { + expect(deleteButton.attributes['aria-disabled']).toBe('true'); + expect(deleteButton.nativeElement.classList.contains('disabled')).toBeTrue(); + }); + }); + + it('should disable delete button and expose tooltip for linked labels', () => { + fixture.detectChanges(); + + const labelRows = fixture.debugElement.queryAll(By.css('.labels-section tbody tr')); + const linkedRowDeleteWrapper = labelRows[0].query(By.css('td:last-child span')); + const linkedRowButtons = labelRows[0].queryAll(By.css('button')); + const linkedDeleteButton = linkedRowButtons[1]; + + expect(linkedRowButtons.length).toBe(2); + expect(linkedDeleteButton.attributes['aria-disabled']).toBe('true'); + expect(linkedDeleteButton.nativeElement.classList.contains('disabled')).toBeTrue(); + expect((linkedRowDeleteWrapper.nativeElement as HTMLElement).getAttribute('tabindex')).toBe('0'); + expect((linkedRowDeleteWrapper.nativeElement as HTMLElement).getAttribute('ng-reflect-ngb-tooltip')).toContain('clarin.license.label.table.del'); + }); + + it('should not open confirmation modal when clicking disabled delete on linked label', () => { + const labelRows = fixture.debugElement.queryAll(By.css('.labels-section tbody tr')); + const linkedDeleteButton = labelRows[0].queryAll(By.css('button'))[1]; + + modalServiceStub.open.calls.reset(); + (clarinLicenseLabelDataService.delete as jasmine.Spy).calls.reset(); + + linkedDeleteButton.nativeElement.click(); + fixture.detectChanges(); + + expect(modalServiceStub.open).not.toHaveBeenCalledWith(ConfirmationModalComponent); + expect((clarinLicenseLabelDataService.delete as jasmine.Spy)).not.toHaveBeenCalled(); + }); + + it('should keep delete button enabled for unlinked labels', () => { + const labelRows = fixture.debugElement.queryAll(By.css('.labels-section tbody tr')); + const unlinkedRowDeleteWrapper = labelRows[1].query(By.css('td:last-child span')); + const unlinkedRowDeleteButton = labelRows[1].queryAll(By.css('button'))[1]; + + expect(unlinkedRowDeleteButton.attributes['aria-disabled']).toBe('false'); + expect(unlinkedRowDeleteButton.nativeElement.classList.contains('disabled')).toBeFalse(); + expect((unlinkedRowDeleteWrapper.nativeElement as HTMLElement).getAttribute('tabindex')).toBeNull(); + }); + }); + + it('should not show labels empty-state row when labels request failed', () => { + (component as any).loading$.next(false); + (component as any).labelsRD$.next(createFailedRemoteDataObject('labels load failed', 500)); + fixture.detectChanges(); + + const emptyStateRow = fixture.debugElement.queryAll(By.css('.labels-section tbody tr')) + .find((row) => row.nativeElement.textContent.includes('clarin.license.label.table.empty')); + + expect(emptyStateRow).toBeUndefined(); + }); + + describe('license usage loading performance', () => { + it('should load full usage dataset only once across repeated table reloads', () => { + (component as any).labelUsageReady$.next(false); + (component as any).licenseUsageLoading = false; + + const usageSpy = spyOn(component, 'loadAllLicensesForUsage').and.callFake(() => { + (component as any).licenseUsageLoading = false; + (component as any).labelUsageReady$.next(true); + }); + + component.loadAllLicenses(); + component.loadAllLicenses(); + + expect(usageSpy).toHaveBeenCalledTimes(1); + }); + + it('should force usage dataset reload when explicitly requested', () => { + (component as any).labelUsageReady$.next(false); + (component as any).licenseUsageLoading = false; + + const usageSpy = spyOn(component, 'loadAllLicensesForUsage').and.callFake(() => { + (component as any).licenseUsageLoading = false; + (component as any).labelUsageReady$.next(true); + }); + + component.loadAllLicenses(); + component.loadAllLicenses({ forceUsageReload: true }); + + expect(usageSpy).toHaveBeenCalledTimes(2); + }); + }); + + it('should reset pagination to page 1 when the search term changes', () => { + paginationServiceStub.pagination.id = defaultPagination.id; + paginationServiceStub.pagination.currentPage = 2; + paginationServiceStub.pagination.pageSize = 10; + paginationServiceStub.pagination.pageSizeOptions = defaultPagination.pageSizeOptions; + (component as any).clarinLicenseService.searchBy.calls.reset(); + paginationServiceStub.resetPage.calls.reset(); + + component.searchingLicenseName = 'Universal'; + + component.searchLicenses(); + + expect(paginationServiceStub.resetPage).toHaveBeenCalledWith(defaultPagination.id); + expect((component as any).clarinLicenseService.searchBy).toHaveBeenCalledWith( + 'byNameLike', + jasmine.objectContaining({ + currentPage: 1, + elementsPerPage: 10, + }), + false + ); + }); + + it('should not reset pagination when searching with the same term', () => { + paginationServiceStub.pagination.id = defaultPagination.id; + paginationServiceStub.pagination.currentPage = 2; + paginationServiceStub.pagination.pageSize = 10; + paginationServiceStub.pagination.pageSizeOptions = defaultPagination.pageSizeOptions; + (component as any).clarinLicenseService.searchBy.calls.reset(); + paginationServiceStub.resetPage.calls.reset(); + (component as any).previousSearchTerm = 'Universal'; + component.searchingLicenseName = 'Universal'; + + component.searchLicenses(); + + expect(paginationServiceStub.resetPage).not.toHaveBeenCalled(); + expect((component as any).clarinLicenseService.searchBy).toHaveBeenCalledWith( + 'byNameLike', + jasmine.objectContaining({ + currentPage: 2, + elementsPerPage: 10, + }), + false + ); + }); }); diff --git a/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.ts b/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.ts index 143f991aab1..6e37453cd91 100644 --- a/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.ts +++ b/src/app/clarin-licenses/clarin-license-table/clarin-license-table.component.ts @@ -1,11 +1,11 @@ -import { Component, OnInit } from '@angular/core'; +import { Component, OnDestroy, OnInit } from '@angular/core'; import { PaginationComponentOptions } from '../../shared/pagination/pagination-component-options.model'; -import { BehaviorSubject, combineLatest as observableCombineLatest } from 'rxjs'; +import { BehaviorSubject, combineLatest as observableCombineLatest, Observable, of, Subject } from 'rxjs'; import { RemoteData } from '../../core/data/remote-data'; import { PaginatedList } from '../../core/data/paginated-list.model'; import { ClarinLicense } from '../../core/shared/clarin/clarin-license.model'; import { getFirstCompletedRemoteData, getFirstSucceededRemoteData } from '../../core/shared/operators'; -import { scan, switchMap } from 'rxjs/operators'; +import { switchMap, take, takeUntil } from 'rxjs/operators'; import { PaginationService } from '../../core/pagination/pagination.service'; import { ClarinLicenseDataService } from '../../core/data/clarin/clarin-license-data.service'; import { defaultPagination, defaultSortConfiguration } from '../clarin-license-table-pagination'; @@ -15,13 +15,15 @@ import { DefineLicenseLabelFormComponent } from './modal/define-license-label-fo import { ClarinLicenseConfirmationSerializer } from '../../core/shared/clarin/clarin-license-confirmation-serializer'; import { NotificationsService } from '../../shared/notifications/notifications.service'; import { TranslateService } from '@ngx-translate/core'; -import { isNull } from '../../shared/empty.util'; +import { hasNoValue, isNull } from '../../shared/empty.util'; import { ClarinLicenseLabel } from '../../core/shared/clarin/clarin-license-label.model'; import { ClarinLicenseLabelDataService } from '../../core/data/clarin/clarin-license-label-data.service'; import { ClarinLicenseLabelExtendedSerializer } from '../../core/shared/clarin/clarin-license-label-extended-serializer'; import { ClarinLicenseRequiredInfoSerializer } from '../../core/shared/clarin/clarin-license-required-info-serializer'; import cloneDeep from 'lodash/cloneDeep'; import { RequestParam } from '../../core/cache/models/request-param.model'; +import { ConfirmationModalComponent } from '../../shared/confirmation-modal/confirmation-modal.component'; +import { DSpaceObject } from '../../core/shared/dspace-object.model'; /** * Component for managing clarin licenses and defining clarin license labels. @@ -31,7 +33,7 @@ import { RequestParam } from '../../core/cache/models/request-param.model'; templateUrl: './clarin-license-table.component.html', styleUrls: ['./clarin-license-table.component.scss'] }) -export class ClarinLicenseTableComponent implements OnInit { +export class ClarinLicenseTableComponent implements OnInit, OnDestroy { constructor(private paginationService: PaginationService, private clarinLicenseService: ClarinLicenseDataService, @@ -41,6 +43,12 @@ export class ClarinLicenseTableComponent implements OnInit { private notificationService: NotificationsService, private translateService: TranslateService,) { } + /** + * Full licenses dataset used by frontend-only label usage derivation. + */ + allLicensesRD$: BehaviorSubject>> = + new BehaviorSubject>>(null); + /** * The list of ClarinLicense object as BehaviorSubject object */ @@ -67,9 +75,73 @@ export class ClarinLicenseTableComponent implements OnInit { */ searchingLicenseName = ''; + /** + * RemoteData stream for license labels table. + */ + labelsRD$: BehaviorSubject>> = + new BehaviorSubject>>(null); + + /** + * Loading state for labels table. + */ + loading$ = new BehaviorSubject(false); + + /** + * Single source of truth for whether the full license usage crawl has finished building the + * in-use set. Emits true once the crawl completes successfully; until then the label Delete + * buttons stay disabled so an in-use label is never deletable during the crawl window. + * Read synchronously via `.value` as the re-crawl guard, and bound reactively in the template. + */ + labelUsageReady$ = new BehaviorSubject(false); + + /** + * Pagination configuration for labels table. + */ + labelPaginationOptions: PaginationComponentOptions = Object.assign(new PaginationComponentOptions(), { + id: 'cLicenseLabels', + currentPage: 1, + pageSize: 10 + }); + + /** + * Triggers a labels reload without changing pagination state. + */ + private labelsRefresh$ = new BehaviorSubject(undefined); + + /** + * Label ids currently linked from at least one license. + */ + private inUseLabelIds = new Set(); + + /** + * Page size used to retrieve all licenses for usage analysis. + */ + private readonly allLicensesPageSize = 100; + + /** + * Indicates whether a full usage crawl is currently in flight. + */ + private licenseUsageLoading = false; + + /** + * Stores the previous search term to detect when a new search should reset pagination. + */ + private previousSearchTerm = ''; + + /** + * Emits when component is destroyed to clean up subscriptions. + */ + private ngUnsubscribe = new Subject(); + ngOnInit(): void { this.initializePaginationOptions(); this.loadAllLicenses(); + this.initializeLabelsPaginationStream(); + } + + ngOnDestroy(): void { + this.ngUnsubscribe.next(); + this.ngUnsubscribe.complete(); } // define license @@ -77,7 +149,7 @@ export class ClarinLicenseTableComponent implements OnInit { * Pop up the License modal where the user fill in the License data. */ openDefineLicenseForm() { - const defineLicenseModalRef = this.modalService.open(DefineLicenseFormComponent); + const defineLicenseModalRef = this.modalService.open(DefineLicenseFormComponent, { centered: true }); defineLicenseModalRef.result.then((result: ClarinLicense) => { this.defineNewLicense(result); @@ -95,6 +167,7 @@ export class ClarinLicenseTableComponent implements OnInit { const errorMessageContentDef = 'clarin-license.define-license.notification.error-content'; if (isNull(clarinLicense)) { this.notifyOperationStatus(clarinLicense, successfulMessageContentDef, errorMessageContentDef); + return; } // convert string value from the form to the number @@ -109,7 +182,7 @@ export class ClarinLicenseTableComponent implements OnInit { .subscribe((defineLicenseResponse: RemoteData) => { // check payload and show error or successful this.notifyOperationStatus(defineLicenseResponse, successfulMessageContentDef, errorMessageContentDef); - this.loadAllLicenses(); + this.loadAllLicenses({ forceUsageReload: true }); }); } @@ -123,7 +196,7 @@ export class ClarinLicenseTableComponent implements OnInit { } // pass the actual clarin license values to the define-clarin-license modal - const editLicenseModalRef = this.modalService.open(DefineLicenseFormComponent); + const editLicenseModalRef = this.modalService.open(DefineLicenseFormComponent, { centered: true }); editLicenseModalRef.componentInstance.name = this.selectedLicense.name; editLicenseModalRef.componentInstance.definition = this.selectedLicense.definition; editLicenseModalRef.componentInstance.confirmation = this.selectedLicense.confirmation; @@ -147,6 +220,7 @@ export class ClarinLicenseTableComponent implements OnInit { const errorMessageContentDef = 'clarin-license.edit-license.notification.error-content'; if (isNull(clarinLicense)) { this.notifyOperationStatus(clarinLicense, successfulMessageContentDef, errorMessageContentDef); + return; } const clarinLicenseObj = new ClarinLicense(); @@ -171,7 +245,7 @@ export class ClarinLicenseTableComponent implements OnInit { .subscribe((editResponse: RemoteData) => { // check payload and show error or successful this.notifyOperationStatus(editResponse, successfulMessageContentDef, errorMessageContentDef); - this.loadAllLicenses(); + this.loadAllLicenses({ forceUsageReload: true }); }); } @@ -198,7 +272,7 @@ export class ClarinLicenseTableComponent implements OnInit { * Pop up License Label modal where the user fill in the License Label data. */ openDefineLicenseLabelForm() { - const defineLicenseLabelModalRef = this.modalService.open(DefineLicenseLabelFormComponent); + const defineLicenseLabelModalRef = this.modalService.open(DefineLicenseLabelFormComponent, { centered: true }); defineLicenseLabelModalRef.result.then((result: ClarinLicenseLabel) => { this.defineLicenseLabel(result); @@ -212,10 +286,11 @@ export class ClarinLicenseTableComponent implements OnInit { * @param clarinLicenseLabel object from the License Label modal. */ defineLicenseLabel(clarinLicenseLabel: ClarinLicenseLabel) { - const successfulMessageContentDef = 'clarin-license-label.define-license-label.notification.successful-content'; - const errorMessageContentDef = 'clarin-license-label.define-license-label.notification.error-content'; + const successfulMessageContentDef = 'clarin.license.label.create.success'; + const errorMessageContentDef = 'clarin.license.label.create.error'; if (isNull(clarinLicenseLabel)) { this.notifyOperationStatus(clarinLicenseLabel, successfulMessageContentDef, errorMessageContentDef); + return; } // convert file to the byte array @@ -266,6 +341,13 @@ export class ClarinLicenseTableComponent implements OnInit { // check payload and show error or successful this.notifyOperationStatus(defineLicenseLabelResponse, successfulMessageContentDef, errorMessageContentDef); this.loadAllLicenses(); + if (defineLicenseLabelResponse?.hasSucceeded) { + // The backend returns labels in ascending insertion order (it ignores the sort param), + // so a new label lands on the last page. Jump there so the admin sees it without paging. + this.goToLastLabelsPage(); + } else { + this.refreshLabels(); + } }); } @@ -274,7 +356,7 @@ export class ClarinLicenseTableComponent implements OnInit { * Delete selected license. If none license is selected do nothing. */ deleteLicense() { - if (isNull(this.selectedLicense?.id)) { + if (hasNoValue(this.selectedLicense?.id) || this.isSelectedLicenseInUse()) { return; } this.clarinLicenseService.delete(String(this.selectedLicense.id)) @@ -283,10 +365,161 @@ export class ClarinLicenseTableComponent implements OnInit { const successfulMessageContentDef = 'clarin-license.delete-license.notification.successful-content'; const errorMessageContentDef = 'clarin-license.delete-license.notification.error-content'; this.notifyOperationStatus(deleteLicenseResponse, successfulMessageContentDef, errorMessageContentDef); - this.loadAllLicenses(); + this.loadAllLicenses({ forceUsageReload: true }); + }); + } + + /** + * Returns whether selected license has attached bitstreams. + */ + isSelectedLicenseInUse(): boolean { + return this.selectedLicense?.bitstreams > 0; + } + + /** + * Open the edit modal for the selected license label, pre-filling its current values. + * On confirm, calls the PUT service and refreshes the label list. + */ + editLabel(label: ClarinLicenseLabel) { + if (isNull(label)) { + return; + } + + const editLabelModalRef = this.modalService.open(DefineLicenseLabelFormComponent, { centered: true }); + editLabelModalRef.componentInstance.clarinLicenseLabel = label; + + editLabelModalRef.result.then((result) => { + this.editLicenseLabel(result, label); + }).catch(() => { /* dismissed */ }); + } + + /** + * Send a PUT request to update the selected label with the new form values. + * Handles success/error notifications and refreshes the label list. + * @param formValues The updated form values returned from the edit modal. + * @param selectedLabel The selected label row to update. + */ + editLicenseLabel(formValues: any, selectedLabel: ClarinLicenseLabel) { + const successMsg = 'clarin.license.label.edit.success'; + const errorMsg = 'clarin.license.label.edit.error'; + if (isNull(formValues) || isNull(selectedLabel)) { + this.notifyOperationStatus(null, successMsg, errorMsg); + return; + } + + const updatedLabel = new ClarinLicenseLabel(); + updatedLabel.id = selectedLabel.id; + updatedLabel._links = selectedLabel._links; + updatedLabel.type = selectedLabel.type; + updatedLabel.label = formValues.label; + updatedLabel.title = formValues.title; + updatedLabel.extended = !!formValues.extended; + + // file input: convert if a new file was selected, otherwise keep existing icon + const reader = new FileReader(); + try { + reader.readAsArrayBuffer(formValues.icon?.[0]); + reader.onerror = () => { + this.notifyOperationStatus(null, successMsg, errorMsg); + }; + reader.onloadend = (evt) => { + if (evt.target.readyState === FileReader.DONE) { + const buf = evt.target.result; + const bytes: number[] = []; + if (buf instanceof ArrayBuffer) { + const arr = new Uint8Array(buf); + for (const b of arr) { bytes.push(b); } + } + updatedLabel.icon = bytes; + this.doUpdateLabel(updatedLabel, successMsg, errorMsg); + } + }; + } catch { + // no new file selected – clear the icon when requested, otherwise keep the existing one + updatedLabel.icon = formValues.clearIcon ? [] : undefined; + this.doUpdateLabel(updatedLabel, successMsg, errorMsg); + } + } + + /** + * Execute the actual PUT request for a label and handle notifications + dependent list refreshes. + */ + private doUpdateLabel(label: ClarinLicenseLabel, successMsg: string, errorMsg: string) { + this.clarinLicenseLabelService.put(label) + .pipe(getFirstCompletedRemoteData(), takeUntil(this.ngUnsubscribe)) + .subscribe((res: RemoteData) => { + this.notifyOperationStatus(res, successMsg, errorMsg); + if (res?.hasSucceeded) { + this.refreshLabels(); + this.loadAllLicenses(); + } }); } + /** + * Ask for confirmation and delete the selected license label. + */ + confirmDeleteLabel(labelToDelete: ClarinLicenseLabel) { + if (isNull(labelToDelete?.id)) { + return; + } + + const labelDeleteDSO = new DSpaceObject(); + labelDeleteDSO.name = labelToDelete.label; + + const modalRef = this.modalService.open(ConfirmationModalComponent, { centered: true }); + modalRef.componentInstance.dso = labelDeleteDSO; + modalRef.componentInstance.headerLabel = 'clarin.license.label.delete.confirm.title'; + modalRef.componentInstance.infoLabel = 'clarin.license.label.delete.confirm.message'; + modalRef.componentInstance.cancelLabel = 'clarin.license.label.delete.cancel.button'; + modalRef.componentInstance.confirmLabel = 'clarin.license.label.delete.confirm.button'; + modalRef.componentInstance.brandColor = 'danger'; + modalRef.componentInstance.confirmIcon = 'fas fa-trash'; + + modalRef.componentInstance.response + .pipe(take(1), takeUntil(this.ngUnsubscribe)) + .subscribe((confirm: boolean) => { + if (!confirm) { + return; + } + + this.clarinLicenseLabelService.delete(String(labelToDelete.id)) + .pipe(getFirstCompletedRemoteData(), takeUntil(this.ngUnsubscribe)) + .subscribe((deleteLabelResponse) => { + if (deleteLabelResponse?.hasSucceeded) { + this.notificationService.success('', this.translateService.get('clarin.license.label.delete.success')); + this.refreshLabels(); + this.loadAllLicenses(); + } else { + this.notificationService.error('', this.translateService.get('clarin.license.label.delete.error')); + } + }, () => { + this.notificationService.error('', this.translateService.get('clarin.license.label.delete.error')); + }); + }); + } + + /** + * Reload labels table using current pagination options. + */ + refreshLabels() { + this.labelsRefresh$.next(undefined); + } + + /** + * Navigate the labels table to the page that contains the most recently created label. + * Exactly one label was just added, so the new total is the current total plus one; the new + * label is on the last page because the backend lists labels in ascending insertion order. + */ + private goToLastLabelsPage() { + const pageSize = this.labelPaginationOptions.pageSize; + const currentTotal = this.labelsRD$.value?.payload?.totalElements ?? 0; + const lastPage = Math.max(1, Math.ceil((currentTotal + 1) / pageSize)); + this.paginationService.updateRoute(this.labelPaginationOptions.id, { page: lastPage }); + // Force a reload as well so the table refreshes even when already on the target page. + this.refreshLabels(); + } + /** * Pop up the notification about the request success. Messages are loaded from the `en.json5`. * @param operationResponse current response @@ -315,33 +548,41 @@ export class ClarinLicenseTableComponent implements OnInit { this.loadAllLicenses(); } + /** + * Run a search and reset the route-backed pagination when the search term changes. + */ + searchLicenses() { + const hasSearchTermChanged = this.searchingLicenseName !== this.previousSearchTerm; + + if (hasSearchTermChanged) { + this.paginationService.resetPage(this.options.id); + } + + this.loadAllLicenses({ pageOverride: hasSearchTermChanged ? 1 : undefined }); + this.previousSearchTerm = this.searchingLicenseName; + } + /** * Fetch all licenses from the API. */ - loadAllLicenses() { + loadAllLicenses(options: { pageOverride?: number; forceUsageReload?: boolean } = {}) { + const { pageOverride, forceUsageReload = false } = options; this.selectedLicense = null; this.licensesRD$ = new BehaviorSubject>>(null); this.isLoading = true; + this.ensureLicenseUsageLoaded(forceUsageReload); // load the current pagination and sorting options const currentPagination$ = this.getCurrentPagination(); const currentSort$ = this.getCurrentSort(); - const searchTerm$ = new BehaviorSubject(this.searchingLicenseName); - observableCombineLatest([currentPagination$, currentSort$, searchTerm$]).pipe( - scan((prevState, [currentPagination, currentSort, searchTerm]) => { - // If search term has changed, reset to page 1; otherwise, keep current page - const currentPage = prevState.searchTerm !== searchTerm ? 1 : currentPagination.currentPage; - return { currentPage, currentPagination, currentSort, searchTerm }; - }, { searchTerm: '', currentPage: 1, currentPagination: this.getCurrentPagination(), - currentSort: this.getCurrentSort() }), - - switchMap(({ currentPage, currentPagination, currentSort, searchTerm }) => { + observableCombineLatest([currentPagination$, currentSort$]).pipe( + switchMap(([currentPagination, currentSort]) => { return this.clarinLicenseService.searchBy('byNameLike', { - currentPage: currentPage, // Properly reset page only when needed + currentPage: pageOverride ?? currentPagination.currentPage, elementsPerPage: currentPagination.pageSize, sort: { field: currentSort.field, direction: currentSort.direction }, - searchParams: [new RequestParam('name', searchTerm)] + searchParams: [new RequestParam('name', this.searchingLicenseName)] }, false ); }), @@ -352,6 +593,120 @@ export class ClarinLicenseTableComponent implements OnInit { }); } + /** + * Ensure the expensive full usage crawl runs only when needed. + * @param forceReload When true, invalidate existing usage cache and reload. + */ + private ensureLicenseUsageLoaded(forceReload = false) { + if (forceReload) { + this.labelUsageReady$.next(false); + } + + if (this.labelUsageReady$.value || this.licenseUsageLoading) { + return; + } + + this.licenseUsageLoading = true; + this.labelUsageReady$.next(false); + this.loadAllLicensesForUsage(); + } + + /** + * Returns whether a license label is used by at least one license (primary or extended labels). + * @param label License label row object. + */ + isLabelInUse(label: ClarinLicenseLabel): boolean { + if (isNull(label?.id)) { + return false; + } + return this.inUseLabelIds.has(String(label.id)); + } + + /** + * Load all licenses page-by-page and rebuild label usage set. + */ + private loadAllLicensesForUsage() { + this.fetchAllLicensePages(0, []) + .pipe(takeUntil(this.ngUnsubscribe)) + .subscribe(({ response, licenses }) => { + this.licenseUsageLoading = false; + this.allLicensesRD$.next(response); + if (response?.hasSucceeded) { + this.rebuildLabelUsageSet(licenses); + this.labelUsageReady$.next(true); + } else { + this.inUseLabelIds.clear(); + this.labelUsageReady$.next(false); + } + }, () => { + this.licenseUsageLoading = false; + this.inUseLabelIds.clear(); + this.labelUsageReady$.next(false); + }); + } + + /** + * Recursively fetch all pages from the license search endpoint. + * @param currentPage Zero-based page index. + * @param accumulatedLicenses Already collected licenses. + */ + private fetchAllLicensePages( + currentPage: number, + accumulatedLicenses: ClarinLicense[] + ): Observable<{ response: RemoteData>, licenses: ClarinLicense[] }> { + return this.clarinLicenseService.searchBy('byNameLike', { + currentPage: currentPage + 1, + elementsPerPage: this.allLicensesPageSize, + sort: { field: defaultSortConfiguration.field, direction: defaultSortConfiguration.direction }, + searchParams: [new RequestParam('name', '')] + }, false).pipe( + getFirstCompletedRemoteData(), + switchMap((response: RemoteData>) => { + const pageLicenses = response?.payload?.page ?? []; + const nextAccumulated = [...accumulatedLicenses, ...pageLicenses]; + + if (!response?.hasSucceeded) { + return of({ response, licenses: nextAccumulated }); + } + + const totalPages = response?.payload?.totalPages ?? 1; + const payloadCurrentPage = response?.payload?.currentPage; + const resolvedCurrentPage = isNull(payloadCurrentPage) ? currentPage : payloadCurrentPage; + const nextPage = resolvedCurrentPage + 1; + const hasNextPage = nextPage < totalPages; + + if (!hasNextPage) { + return of({ response, licenses: nextAccumulated }); + } + + return this.fetchAllLicensePages(nextPage, nextAccumulated); + }) + ); + } + + /** + * Build fast lookup of label ids referenced by any loaded license. + * @param licenses Aggregated list of all licenses. + */ + private rebuildLabelUsageSet(licenses: ClarinLicense[]) { + const usageSet = new Set(); + + (licenses || []).forEach((license: ClarinLicense) => { + const mainLabelId = license?.clarinLicenseLabel?.id; + if (!isNull(mainLabelId)) { + usageSet.add(String(mainLabelId)); + } + + (license?.extendedClarinLicenseLabels || []).forEach((extendedLabel: ClarinLicenseLabel) => { + if (!isNull(extendedLabel?.id)) { + usageSet.add(String(extendedLabel.id)); + } + }); + }); + + this.inUseLabelIds = usageSet; + } + /** * Mark the license as selected or unselect if it is already clicked. * @param clarinLicense @@ -388,4 +743,40 @@ export class ClarinLicenseTableComponent implements OnInit { private getCurrentSort() { return this.paginationService.getCurrentSort(this.options.id, defaultSortConfiguration); } + + /** + * Initialize labels data stream so pagination query-param changes trigger fetches reactively. + */ + private initializeLabelsPaginationStream() { + const labelsLoadErrorKey = 'clarin.license.label.load.error'; + const currentLabelPagination$ = this.paginationService + .getCurrentPagination(this.labelPaginationOptions.id, this.labelPaginationOptions); + + observableCombineLatest([currentLabelPagination$, this.labelsRefresh$]) + .pipe( + switchMap(([currentPagination]) => { + this.labelsRD$.next(null); + this.loading$.next(true); + return this.clarinLicenseLabelService.findAll({ + currentPage: currentPagination.currentPage, + elementsPerPage: currentPagination.pageSize + }, false).pipe( + getFirstCompletedRemoteData() + ); + }), + takeUntil(this.ngUnsubscribe) + ) + .subscribe((labelsResponse: RemoteData>) => { + this.labelsRD$.next(labelsResponse); + if (!labelsResponse?.hasSucceeded) { + this.notificationService.error('', this.translateService.get(labelsLoadErrorKey)); + } + this.loading$.next(false); + }, () => { + this.labelsRD$.next(null); + this.notificationService.error('', this.translateService.get(labelsLoadErrorKey)); + this.loading$.next(false); + } + ); + } } diff --git a/src/app/clarin-licenses/clarin-license-table/modal/define-license-form/define-license-form.component.html b/src/app/clarin-licenses/clarin-license-table/modal/define-license-form/define-license-form.component.html index 2212f994e1d..2454283ae04 100644 --- a/src/app/clarin-licenses/clarin-license-table/modal/define-license-form/define-license-form.component.html +++ b/src/app/clarin-licenses/clarin-license-table/modal/define-license-form/define-license-form.component.html @@ -1,63 +1,57 @@ -