Skip to content

Conversation

@JorgeEscire
Copy link

@JorgeEscire JorgeEscire commented Feb 10, 2026

References

Fixes #5085

Description

This PR fixes two critical bugs in the end-user agreement flow:

Part 1 - Checkbox reset preventing form submission: When new users accessed the end-user agreement page, checking the accept checkbox would sometimes get reset automatically, making the Save button appear non-functional until page reload.

Part 2 - Navigation hanging after successful submission: After successfully accepting the agreement (PATCH request succeeded), the page would stall indefinitely without redirecting to the intended destination.

Root Causes
Both issues were caused by improper handling of ngrx store observables that never complete:

  1. Checkbox reset: The initAccepted() subscription to hasCurrentUserOrCookieAcceptedAgreement() lacked take(1), allowing store re-emissions to override user interactions by resetting this.accepted to false.

  2. Navigation hang: After the PATCH operation, the endUserAgreementCurrentUserGuard would check user acceptance status using store selectors that could return stale data or never complete, causing the Angular router to hang.

Solution
Applied established DSpace patterns for handling store-backed observables:

Added take(1) to all store selector subscriptions to ensure they complete after first emission
Implemented cookie fallback after successful PATCH operations for synchronous state verification
Updated the guard to use hasCurrentUserOrCookieAcceptedAgreement() which checks the cookie first before querying potentially stale store data
Instructions for Reviewers
This PR modifies the end-user agreement component, service, and guard to properly handle RxJS observables from the ngrx store.

List of changes in this PR:

  1. end-user-agreement.component.ts: Added take(1) to initAccepted() subscription to prevent store re-emissions from overriding user checkbox interactions

  2. end-user-agreement.service.ts:

    • Added take(1) to hasCurrentUserAcceptedAgreement() when calling getAuthenticatedUserFromStore()
    • Modified setUserAcceptedAgreement() to set cookie immediately after successful PATCH as a synchronous fallback
  3. end-user-agreement-current-user.guard.ts: Updated to use hasCurrentUserOrCookieAcceptedAgreement() with take(1), leveraging cookie-based checks to prevent guard hangs

How to test / review:
Setup:

  1. Ensure info.enableEndUserAgreement is set to true in your config
  2. Clear browser cookies (especially hasAgreedEndUser)
  3. Create a new test user account OR remove the dspace.agreements.end-user metadata from an existing user

Test Part 1 - Checkbox functionality:

  1. Log in as a user without end-user agreement metadata
  2. You should be redirected to /info/end-user-agreement
  3. Check the "I accept" checkbox
  4. Verify the checkbox stays checked and the Save button becomes enabled
  5. Expected: Checkbox should not reset itself (bug fixed)

Test Part 2 - Navigation after submission:

  1. While on the end-user agreement page, click "Save"
  2. Wait for the success notification
  3. Expected: Page should automatically redirect to /home (or the original destination)
  4. Expected: No indefinite stalling or hanging (bug fixed)

Verify no regressions:

  • Test with users who have already accepted the agreement - they should not see the page
  • Test logout and login flow multiple times
  • Verify anonymous users are not affected
  • Check that cookie is properly set after acceptance

Verify the fix persists:

  1. After accepting, log out and log back in - should not see agreement page again
  2. Clear cookies but keep user metadata - should still not see agreement page (metadata check works)
  3. Clear metadata but keep cookie - should still not see agreement page (cookie fallback works)

Checklist

  • My PR is created against the main branch of code (unless it is a backport or is fixing an issue specific to an older branch).
  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using npm run lint
  • My PR doesn't introduce circular dependencies (verified via npm run check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • My PR includes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
  • If my PR fixes an issue ticket, I've linked them together.

@tdonohue tdonohue added bug usability port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release port to dspace-9_x This PR needs to be ported to `dspace-9_x` branch for next bug-fix release 1 APPROVAL pull request only requires a single approval to merge labels Feb 10, 2026
@tdonohue tdonohue moved this to 🙋 Needs Reviewers Assigned in DSpace 10.0 Release Feb 10, 2026
@tdonohue
Copy link
Member

@lgeggleston : I'm going to add you as a "reviewer" on this PR since you reported the initial issue. If you can find time to test this in the next week or so, that'd help us to get this fixed quickly. Thanks!

@tdonohue tdonohue requested a review from lgeggleston February 10, 2026 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 APPROVAL pull request only requires a single approval to merge bug port to dspace-8_x This PR needs to be ported to `dspace-8_x` branch for next bug-fix release port to dspace-9_x This PR needs to be ported to `dspace-9_x` branch for next bug-fix release usability

Projects

Status: 🙋 Needs Reviewers Assigned

Development

Successfully merging this pull request may close these issues.

End user agreement stalls on submit in two ways

3 participants