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

Refactor SubmissionConfigReader to use a map for the item process configurations based on entityType #9478

Merged
merged 7 commits into from May 2, 2024

Conversation

toniprieto
Copy link
Contributor

@toniprieto toniprieto commented Apr 15, 2024

References

Description

This PR implements the changes described in this comment to utilize a map for returning the item process submission form configured for collections with a specific entityType. The implementation follows a similar approach to #9259

Instructions for Reviewers

List of changes in this PR:

  • Refactor SubmissionConfigReader to use a new map for entityType-configured submission forms
  • Add a new test to ensure that entityType-configured submission forms work correctly and take precedence over community-based configurations.
  • Remove from default configuration the submissionconfig consumer used to reload the submission configuration after creating/modifying a collection. With the proposed changes, this reload should not be necessary. This change could be undone if it is useful for other use cases.

Test that the entityType configurations are working properly after creating o editing the entityType of a collection and that the issue #9402 is also resolved.

Checklist

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & integration tests). Exceptions may be made if previously agreed upon.
  • My PR passes Checkstyle validation based on the Code Style Guide.
  • My PR includes Javadoc for all new (or modified) public methods and classes. It also includes Javadoc for large or complex private methods.
  • My PR passes all tests and includes new/updated Unit or Integration Tests based on the Code Testing Guide.
  • If my PR includes new libraries/dependencies (in any pom.xml), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR modifies REST API endpoints, I've opened a separate REST Contract PR related to this change.
  • If my PR includes new configurations, I've provided basic technical documentation in the PR itself.

@tdonohue tdonohue added 1 APPROVAL pull request only requires a single approval to merge. bug component: submission Related to configurable submission system labels Apr 15, 2024
@tdonohue tdonohue added the port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release label Apr 15, 2024
@tdonohue tdonohue added this to the 8.0 milestone Apr 15, 2024
@tdonohue tdonohue mentioned this pull request Apr 15, 2024
Copy link
Contributor

@paulo-graca paulo-graca 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 @toniprieto for this contribution! With this PR, we don't need to reload (can also be configured to do it so) submission configuration every time we change a collection. It's a much better implementation. It also ensures that if we specify an handle, it will be given priority to that configuration.
I just have one request, if you within this PR, could remove unused collectionService code

  • findAllCollectionsByEntityType.

@@ -238,6 +244,16 @@ public SubmissionConfig getSubmissionConfigByCollection(Collection col) {
return getSubmissionConfigByName(submitName);
}

// get the name of the submission process based on the entity type of this collections
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 this will ensure that if an handle is specified, then it will be firstly returned

@toniprieto
Copy link
Contributor Author

@paulo-graca @tdonohue Thank you for the reviews, the feedback has been addressed.

Two extra comments:

Copy link
Contributor

@paulo-graca paulo-graca 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 @toniprieto! All changes are looking perfect to me. It's a +1 on my behalf. I will try to test it in our on-premises setup.

@vins01-4science
Copy link
Contributor

Thank you @toniprieto,

@tdonohue: I checked that this PR is solving the issue I was figuring out by starting the backend with the server-boot configuration (#9505 (comment), #9476).
So if everything looks good for @tdonohue , I think that we can merge this PR in order to solve both issues 🚀 !

@tdonohue
Copy link
Member

tdonohue commented May 2, 2024

Thanks for the note @vins01-4science ! I'll test this today as well and, assuming it works for me, I'll get it merged immediately. I'm very glad to hear this refactor should help solve issues that we were having in other PRs!

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.

👍 Thanks @toniprieto ! This looks great. Verified this fixes #9402. I've also verified it fixes #9217 as well (which is likely why this PR helps fix issues with other PRs, as I think #9217 was the root cause in those scenarios).

Merging immediately as this looks great and provides major benefits the the startup process as well as fixes SubmissionConfigReader bugs.

@tdonohue tdonohue removed the port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release label May 2, 2024
@tdonohue tdonohue merged commit c15d939 into DSpace:main May 2, 2024
20 checks passed
@tdonohue
Copy link
Member

tdonohue commented May 2, 2024

I've also updated the Submission Docs to remove mentions of having to restart Tomcat whenever a new Entity Collection is created (as this fixes that bug): https://wiki.lyrasis.org/display/DSDOC8x/Submission+User+Interface#SubmissionUserInterface-AssigningacustomSubmissionProcesstoaCollection

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 component: submission Related to configurable submission system high priority
Projects
Status: ✅ Done
4 participants