-
Notifications
You must be signed in to change notification settings - Fork 19
Allow publishers to upload content if they have two journal records in DOAJ with same ISSN: one withdrawn, one live #2515
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
base: develop
Are you sure you want to change the base?
Conversation
… with the same issns are present
…e/1891_two_journals_bug
richard-jones
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amdomanska can you add the functional tests/regression tests for this to the PR please.
richard-jones
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is missing one final piece, which is to update the journal_activate web route to check that a journal can be reinstated before submitting the job.
richard-jones
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small change to background job execution, plus we still need to see the relevant tests for this to complete review
| job.add_audit_message( | ||
| Messages.CANNOT_CHANGE_THE_STATUS__OTHER_JOURNAL_IN_DOAJ_EXISTS) | ||
| job.outcome_fail() | ||
| job.fail() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the job has failed, the job has completed with an outcome that is a failure for the user, but not for the code. You can just finish here with outcome_fail and the background job handler will sort the rest
Allow publishers to upload content if they have two journal records in DOAJ with same ISSN: one withdrawn, one live
This PR is implemented in line with: https://github.com/DOAJ/doajPM/issues/1891#issuecomment-3224552191
This PR...
Developer Checklist
Developers should review and confirm each of these items before requesting review
constantsormessagesfilesdates)url_fornot hard-codeddevelopReviewer Checklist
Reviewers should review and confirm each of these items before approval
If there are multiple reviewers, this section should be duplicated for each reviewer
constantsormessagesfilesdates)url_fornot hard-codeddevelopTesting
List user test scripts that need to be run
List any non-unit test scripts that need to be run by reviewers
Deployment
What deployment considerations are there? (delete any sections you don't need)
Configuration changes
What configuration changes are included in this PR, and do we need to set specific values for production
Scripts
What scripts need to be run from the PR (e.g. if this is a report generating feature), and when (once, regularly, etc).
Migrations
What migrations need to be run to deploy this
Monitoring
What additional monitoring is required of the application as a result of this feature
New Infrastructure
What new infrastructure does this PR require (e.g. new services that need to run on the back-end).
Continuous Integration
What CI changes are required for this