-
Notifications
You must be signed in to change notification settings - Fork 2
Force use of the Imperial community #340
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
|
Thanks for this @jamesturner246. Well done, it looks like a good approach, more elegant than the url hackery that was going on before. There is one extremely niche corner case but I don't think it's a concern. If a user is able to create metadata only records (currently only system admins though there is a slim chance this may change in the future) and they complete the form without first creating a draft (or clicking "get a doi" which implicitly creates a draft) they will still have the publish button shown and when they click it they get the message "You cannot publish a draft with an open review request. Please cancel the review request first.". This can be easily gotten past by refreshing the page or clicking "save draft" which causes the "submit for review" button to be displayed. As I say I don't think it's a concern as it's a very niche and it doesn't allow circumventing the review mechanism it just means an unclear error message is provided. |
|
Should be good to go now. The tests failed because the community now needs to be created before draft creation, since we now auto-create the review request when we create the draft. The publish issue is unresolved -- I am making an issue. #342 |
cc-a
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.
Sorry @jamesturner246 just spotted an issue whilst working on something else.
These changes seem to break the record preview functionality. To reproduce:
- create a new upload.
- enter something in the Title field.
- click
Preview
expected behavior: form validation should be carried out and errors displayed on the form
actual behavior: Page not found
|
Confirmed. Some notes:
|
|
Okay, so this looks like a bug in the preview mechanism in Invenio itself where preview looks for a 'registered', CF created, uid. The logic is a bit spaghetti, since it looks like it knows to look for the draft uid if the draft is created. If you look on Two solutions:
|
Resolves #329.
Just putting this up here to check you're happy with this, as, to nobody's surprise, it breaks the API tests. Before I get lost in that rabbit hole, could you please confirm you are happy with this solution in principle?
Before we were trying to hardcode the community through url args. Problem was this is one shot, and, as you saw, was ignored once the draft was actually created (url changes to an actual draft instance, with its own id, and url arg is no longer interpreted).
Fix is simple, if you know how... Added a new
ServiceComponentwhich creates an ICL review request through the service layer the moment the draft iscreated. The tied review request is sufficient to make Invenio use the review machinery instead of the publish one. Because the review request instance's status is not open until the user clicks 'submit for review', there won't be loads of review noise from trash or incomplete drafts, only submitted ones. Also no URL hacks are required for this, so they were removed.Enter a brief description of the PR contents here
Developer Checklist
Developers should review and confirm each of these items before requesting review
Reviewer Checklist
Reviewers should review and confirm each of these items before approval
If there are multiple reviewers, this section can be duplicated for each reviewer
Testing
List user test scripts that need to be run
List any non-unit test scripts that need to be run