Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migration to standalone components #2750

Merged
merged 333 commits into from
Mar 20, 2024
Merged

Migration to standalone components #2750

merged 333 commits into from
Mar 20, 2024

Conversation

atarix83
Copy link
Contributor

@atarix83 atarix83 commented Jan 18, 2024

References

Description

The goal of this document is to migrate the DSpace Angular application to a standalone components approach.

Instructions for Reviewers

From a code review perspective the best approach to review this PR is to refer to the dedicated wiki page mentioned below.

From a functional perspective the best approach it to review this PR by running the application (against sandbox rest server) to verify everything works properly as the current sandbox environment.

List of changes in this PR:
See wiki page https://wiki.lyrasis.org/display/DSPACE/Migration+to+Standalone+Components

Checklist

  • 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 yarn lint
  • My PR doesn't introduce circular dependencies (verified via yarn 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.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@atarix83 and @AndreaBarbasso : Overall this looks good and mostly works. However, I've found a number of pages which are slightly broken while testing this. All my tests are done in production mode (yarn build:prod; yarn serve:ssr)

  1. Request a withdrawal feature no longer works.
    • Enable it via qaevents.enabled = true and qaevents.withdraw-reinstate.group = Anonymous
    • Login, and attempt to request a withdrawal on any Item
    • The popup modal will not appear and you will not be able to complete the request.
  2. Select Collection popup broken in batch import
    • Login as Admin
    • Go to Import > Batch Import (Zip)
    • Click "Select Collection". Popup is empty
  3. Publication Claim feature no longer loads from Admin Menu
    • Ensure this feature is enabled, and at least one publication claim exists
    • Login as an Admin and go to "Notifications" > "Publication Claim"
    • Page will not load and you'll see errors in your Browser DevTools
  4. Admin Workflow page no longer loads
    • Login as an Admin and go to "Administer Workflow"
    • Results will show but titles/authors are missing. You can only see the buttons
    • You'll also see errors in your Browser DevTools
  5. LDN Services page no longer loads
    • Login as an Admin and go to "COAR Notify -> LDN Services"
    • No services will load. Errors will apepar in your Browser DevTools
  6. Item Preview after import no longer loads properly
    • Login as someone with submit privileges
    • Go to MyDSpace, click the Import from external sources button
    • Select an external source and perform a search.
    • Click on import next to a result. The "Item Preview" popup will appear but all information will be missing from this popup.
    • (NOTE: it's still possible to complete the import. The only bug is that the Item Preview page doesn't show its preview)

Overall, this is looking great and most features appear to be working (at least everything else I've tried). These are the only ones where I've noticed larger bugs/failures.

@atarix83
Copy link
Contributor Author

thanks @tdonohue for your feedback we'll take a look

@atarix83
Copy link
Contributor Author

@tdonohue

point 1, 2, 4 and 6 should be fixed, still working on point 3 and 5

Copy link
Member

@artlowel artlowel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @atarix83!

Everything mostly works, but I noticed a few issues, on top of the ones @tdonohue already mentioned:

@atarix83
Copy link
Contributor Author

@tdonohue @artlowel

all issues should be fixed now.

@artlowel

In search results and list items, the "metadata only" and "open access" badges are no longer shown (e.g. see the publications section on https://sandbox.dspace.org/entities/project/3329a278-2719-4bae-a5d5-d2039afc2fe2, or the results at: https://sandbox.dspace.org/search?query=test)

regarding this point, by default the badges are not show https://github.com/DSpace/dspace-angular/blob/main/src/config/default-app-config.ts#L324 if you enable that option it works properly

@atarix83 atarix83 requested review from tdonohue and artlowel March 20, 2024 11:00
Copy link
Member

@artlowel artlowel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @atarix83!

regarding this point, by default the badges are not show https://github.com/DSpace/dspace-angular/blob/main/src/config/default-app-config.ts#L324 if you enable that option it works properly

You're right, my mistake

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thank you @atarix83 ! I've verified that all the prior issues are fixed. I've not noticed any new issues. This looks good to me now. Merging immediately to free up work on #2752

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

feat: migrate to standalone components, directives, pipes
7 participants