-
Notifications
You must be signed in to change notification settings - Fork 7
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
Allow depositors to request accessibility remediation and save as draft #1610
Conversation
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 looking good. I'll wait to approve when the description wording is added. We should consider what will happen when someone submitting data & code wants curation and remediation. If accessibility remediation is requested, the work will still be synced to airtable during our daily syncs, right?
elsif request_curation? && !@resource.draft_curation_requested | ||
@resource.update_column(:draft_curation_requested, true) | ||
elsif curator_action_requested? | ||
column = request_curation? ? :draft_curation_requested : :accessibility_remediation_requested | ||
|
||
@resource.update_column(column, true) |
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 think these changes are fine. If we want this to be a bit cleaner or easier to read, we could pull all of this curation logic out into it's own service class.
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.
My understanding is that curation will include accessibility review (it's currently listed as one of the services in the curation request description), but I agree we need to do something to make that more clear. Might be as simple as updating the description with language emphasizing that curation includes remediation so if you want both, select curation.
I'll play around with moving it to a service class. I tried just pulling it into a private method, but there were some issues with error raising/number of redirects, but I might be able to resolve those if it's a separate service class.
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.
Ah, I see that now in the description. Yeah, I think it would be good to emphasize the accessibility part in the curation description a bit more.
Ah nevermind. That's only for published works. |
@resource = resource | ||
end | ||
|
||
def request_action(curation_requested) |
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.
Method request_action
has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.
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.
LGTM!
Fixes #1603
Currently, this pr allows accessibility remediation to be requested for any work type as long as it has not already been requested & draft curation has not been requested. Additional logic will need to be added so that the button does not show up if all files have passed the automatic checker, but that is on hold while the checker is being developed. Depending on timing, that addition will go in another ticket.
I updated the language for both request buttons to drop the '& Save as Draft' part. With the current number of buttons, it took up too much space & buttons were starting to overlap. Instead, I added that language to the help text so users still have an indicator that the work is being saved as a draft.