UoE/datashare: add "Keep embargo policies" option to the move-item screen by milanmajchrak · Pull Request #20 · dataquest-dev/uoe-dspace-datashare-angular

milanmajchrak · 2026-07-08T14:03:05Z

What

Adds a "Keep embargo policies" checkbox to the Move item screen (Edit item → Move) — the companion frontend for backend PR dataquest-dev/uoe-dspace-datashare-backend#27, which addresses dataquest-dev/dspace-customers#761 ("Embargo is lost when moving into another collection").

When moving an item and inheriting the destination collection's policies, the option lets the user choose whether to keep an existing embargo:

checked (default) → inherit everything except the embargo; embargoed files stay embargoed (keepEmbargoPolicies=true).
unchecked → plain DSpace behaviour: the inherited default read access replaces (and lifts) the embargo.

The checkbox is shown only when "Inherit policies" is ticked, since keeping an embargo is only meaningful when inheriting.

Changes

item-move.component.ts: new keepEmbargoPolicies model (default true), passed to moveToCollection (only meaningful when inheriting).
item-move.component.html: "Keep embargo policies" checkbox + description, shown via *ngIf="inheritPolicies".
item-data.service.ts: getMoveItemEndpoint / moveToCollection append &keepEmbargoPolicies=... to the owningCollection move request.
en.json5: item.edit.move.keepembargopolicies.{checkbox,description,tooltip}.
item-move.component.spec.ts: updated the existing expectation and added a case asserting keepEmbargoPolicies is passed through when inheriting.

Requires backend PR dataquest-dev/uoe-dspace-datashare-backend#27 (adds the keepEmbargoPolicies request param; defaults to true).

🤖 Generated with Claude Code

…reen Companion to backend PR dataquest-dev/uoe-dspace-datashare-backend#27 (dspace-customers#761 - embargo lost when moving into another collection). When moving an item and inheriting the destination collection's policies, let the user choose whether to keep an existing embargo: - checked (default): inherit everything except the embargo (keepEmbargoPolicies=true) - unchecked: plain DSpace behaviour - inherited default read lifts the embargo The checkbox is shown only when "Inherit policies" is ticked. - item-move.component: keepEmbargoPolicies model (default true), passed to moveToCollection only when inheriting. - item-move.component.html: checkbox + description gated on inheritPolicies. - item-data.service: send &keepEmbargoPolicies= on the owningCollection move. - en.json5: item.edit.move.keepembargopolicies.{checkbox,description,tooltip}. - item-move.component.spec: updated expectation + case for inheriting. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

coderabbitai · 2026-07-08T14:03:13Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 907a2197-62f4-4b89-8361-e46768993df2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

Pull request overview

Adds a “Keep embargo policies” option to the Item → Edit → Move flow, allowing users who choose Inherit policies to preserve an existing embargo during the move (via a new keepEmbargoPolicies request param).

Changes:

Adds a keepEmbargoPolicies UI model (default true) and conditionally displays a checkbox when inheritPolicies is enabled.
Extends the move request URL generation / API call to include keepEmbargoPolicies.
Adds English i18n strings and updates unit tests to assert the new parameter is passed through.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/assets/i18n/en.json5	Adds new i18n keys for the checkbox label/description/tooltip
src/app/item-page/edit-item-page/item-move/item-move.component.ts	Adds `keepEmbargoPolicies` state and passes it into the move call
src/app/item-page/edit-item-page/item-move/item-move.component.html	Adds checkbox + help text, shown only when inheriting policies
src/app/item-page/edit-item-page/item-move/item-move.component.spec.ts	Updates/extends tests to validate new moveToCollection args
src/app/core/data/item-data.service.ts	Appends `keepEmbargoPolicies` query param to the move endpoint and wires it through

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

milanmajchrak · 2026-07-08T14:12:08Z

+  "item.edit.move.keepembargopolicies.checkbox": "Keep embargo policies",
+
+  "item.edit.move.keepembargopolicies.description": "When inheriting policies, keep any existing embargo: only the non-embargo access is inherited from the destination collection.",
+
+  "item.edit.move.keepembargopolicies.tooltip": "When enabled, inheriting the destination collection's policies preserves an existing embargo, so embargoed files stay embargoed until their original lift date. When disabled, the inherited default read access replaces (and lifts) the embargo.",


These keys are added to en.json5 only, matching this repo's convention for new custom keys: the recent DataShare feature commits (embargo badge, hide download-all button, a11y fixes) each touch only en.json5. Other locales are propagated with the repo's sync-i18n script (which adds new keys under a '// TODO New key' marker) at merge/release time; running it per-feature here would also pull in every other not-yet-synced key and create a large unrelated diff. So I've kept this PR to en.json5 and left locale propagation to the existing sync workflow.

milanmajchrak · 2026-07-08T14:12:06Z

+  public getMoveItemEndpoint(itemId: string, inheritPolicies: boolean, keepEmbargoPolicies = true): Observable<string> {
    return this.halService.getEndpoint(this.linkPath).pipe(
      map((endpoint: string) => this.getIDHref(endpoint, itemId)),
-      map((endpoint: string) => `${endpoint}/owningCollection?inheritPolicies=${inheritPolicies}`),
+      map((endpoint: string) =>
+        `${endpoint}/owningCollection?inheritPolicies=${inheritPolicies}&keepEmbargoPolicies=${keepEmbargoPolicies}`),
    );


Done in the latest commit — getMoveItemEndpoint now only appends keepEmbargoPolicies when inheritPolicies is true (it has no effect otherwise), keeping the URL minimal.

… when inheriting getMoveItemEndpoint now appends keepEmbargoPolicies to the move URL only when inheritPolicies is true (the flag has no effect otherwise), keeping the request URL minimal and the semantics clear. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

milanmajchrak · 2026-07-08T14:17:38Z

+    const move$ = this.itemDataService.moveToCollection(
+      this.item.id, this.selectedCollection, this.inheritPolicies,
+      this.inheritPolicies ? this.keepEmbargoPolicies : true,
+    ).pipe(getFirstCompletedRemoteData());


Done — the component now passes keepEmbargoPolicies straight through. getMoveItemEndpoint already omits it unless inheritPolicies is true, so the conditional was redundant.

milanmajchrak · 2026-07-08T14:17:39Z

+        // keepEmbargoPolicies is only meaningful when inheriting; not inheriting always sends true
+        expect(itemDataService.moveToCollection).toHaveBeenCalledWith('item-id', collection1, false, true);


Reworded — the comment now says keepEmbargoPolicies defaults to true and is only sent on the request URL when inheriting.

…ugh directly getMoveItemEndpoint already omits keepEmbargoPolicies unless inheriting, so the component no longer needs the 'inheritPolicies ? keepEmbargoPolicies : true' conditional - it passes keepEmbargoPolicies straight through. Also reword the spec comment (keepEmbargoPolicies defaults to true and is only sent when inheriting). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

milanmajchrak · 2026-07-09T08:30:05Z

+  public getMoveItemEndpoint(itemId: string, inheritPolicies: boolean, keepEmbargoPolicies = true): Observable<string> {
    return this.halService.getEndpoint(this.linkPath).pipe(
      map((endpoint: string) => this.getIDHref(endpoint, itemId)),
-      map((endpoint: string) => `${endpoint}/owningCollection?inheritPolicies=${inheritPolicies}`),
+      map((endpoint: string) => {
+        let href = `${endpoint}/owningCollection?inheritPolicies=${inheritPolicies}`;
+        // Only relevant when inheriting.
+        if (inheritPolicies) {
+          href += `&keepEmbargoPolicies=${keepEmbargoPolicies}`;
+        }
+        return href;
+      }),


Added getMoveItemEndpoint tests to item-data.service.spec.ts asserting keepEmbargoPolicies is appended only when inheritPolicies is true (and omitted otherwise).

Assert keepEmbargoPolicies is appended only when inheriting policies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

milanmajchrak requested a review from Copilot July 8, 2026 14:03

milanmajchrak mentioned this pull request Jul 8, 2026

UoE/datashare: keep embargo when moving an item with inherit policies (#761) dataquest-dev/uoe-dspace-datashare-backend#27

Open

Copilot started reviewing on behalf of milanmajchrak July 8, 2026 14:03 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

milanmajchrak requested a review from Copilot July 8, 2026 14:12

Copilot started reviewing on behalf of milanmajchrak July 8, 2026 14:12 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

milanmajchrak requested a review from Copilot July 8, 2026 14:17

Copilot started reviewing on behalf of milanmajchrak July 8, 2026 14:18 View session

Copilot AI reviewed Jul 8, 2026

View reviewed changes

UoE/datashare: trim verbose code comments

0918214

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

milanmajchrak requested a review from Copilot July 9, 2026 08:16

Copilot started reviewing on behalf of milanmajchrak July 9, 2026 08:17 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

UoE/datashare: test getMoveItemEndpoint keepEmbargoPolicies URL shaping

9a4fd2e

Assert keepEmbargoPolicies is appended only when inheriting policies. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

milanmajchrak requested a review from Copilot July 9, 2026 08:30

Copilot started reviewing on behalf of milanmajchrak July 9, 2026 08:36 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

UoE/datashare: add "Keep embargo policies" option to the move-item screen#20

UoE/datashare: add "Keep embargo policies" option to the move-item screen#20
milanmajchrak wants to merge 5 commits into
datashare-UoEMainLibrary-dspace-8_xfrom
uoe/move-keep-embargo-policies-option

milanmajchrak commented Jul 8, 2026

Uh oh!

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

Review skipped

Uh oh!

Copilot AI left a comment

Uh oh!

milanmajchrak Jul 8, 2026

Uh oh!

milanmajchrak Jul 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

milanmajchrak Jul 8, 2026

Uh oh!

milanmajchrak Jul 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

milanmajchrak Jul 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// keepEmbargoPolicies is only meaningful when inheriting; not inheriting always sends true
		expect(itemDataService.moveToCollection).toHaveBeenCalledWith('item-id', collection1, false, true);

Uh oh!

Conversation

milanmajchrak commented Jul 8, 2026

What

Changes

Uh oh!

coderabbitai Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

milanmajchrak Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

milanmajchrak Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

milanmajchrak Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

milanmajchrak Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

milanmajchrak Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading