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

DRYD-1575: Rename repatriationclaim to repatriationrequest #429

Merged
merged 2 commits into from
Nov 5, 2024

Conversation

mikejritter
Copy link
Contributor

What does this do?

  • Rename repatriationclaim to repatriationrequest
  • Add id generator for repatriation request

Why are we doing this? (with JIRA link)
Jira: https://collectionspace.atlassian.net/browse/DRYD-1575

The new repatriation procedure was incorrectly specced out to be repatriation claim and needs to be repatriation request. This updates the names of the classes, packages, and schemas used to be repatriation request. The claim id generator was also being used previously and a new id generator for the repatriation request is being added.

How should this be tested? Do these changes have associated tests?

  • Pull this and the related application PR
  • Build and deploy a clean CollectionSpace instance
  • Check that the repatriationrequests endpoint is still working
curl --user [email protected]:Administrator http://localhost:8180/cspace-services/repatriationrequests
  • Further testing is done via the UI
  • If desired, build with the necessary tenants and run the integration tests with mvn:test

Dependencies for merging? Releasing to production?
It's possible for the repatriationclaim to still exist if you didn't fully clean your environment.

Has the application documentation been updated for these changes?
No

Did someone actually run this code to verify it works?
@mikejritter tested against a local instance

@axb21
Copy link
Contributor

axb21 commented Nov 5, 2024

@mikejritter I looked through this PR and it looks good as far as I can see. I didn't spot any typos or obvious errors. I did not pull the code and run it, so I haven't done a check at that level of depth. I'm happy to do that too if that's part of the usual PR review process.

Right now github offers "squash and merge" as the default merge option. Is that what we normally do, out of curiosity?

Update; I forgot to mention that I also grepped through the files in the branch for various forms of "repatriation claim" in filenames or in text content, and didn't find any.

@mikejritter
Copy link
Contributor Author

@axb21 I think squash and merge is the only option for the CollectionSpace repositories which from my experience is what has been used in the past. Personally I sometimes do a rebase and merge depending on the PR. It might be worth discussing more as we could go over some of the workflows in general.

When adding new procedures pulling the code and running it would be ideal but obviously takes additional time (especially with how slow the startup is if all tenants are enabled for running the tests). It would be nice to have some CI that could show us the tests pass but with how CollectionSpace is deployed that isn't the easiest task at the moment.

I'll get this merged in.

@mikejritter mikejritter merged commit 8b5d919 into collectionspace:v8.1-branch Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants