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

append deaccessionDialog.reasons with periods for proper display #7996

Merged
merged 1 commit into from
Jul 12, 2021

Conversation

donsizemore
Copy link
Contributor

@donsizemore donsizemore commented Jul 9, 2021

What this PR does / why we need it: deaccessionDialog.reasons aren't terminated with punctuation but wind up concatenated with other text in run-on sentences

Which issue(s) this PR closes:

Closes #7936

Special notes for your reviewer: none

Suggestions on how to test this: deaccession a dataset, then attempt to view it

Does this PR introduce a user interface change? If mockups are available, please link/include them here: no

Is there a release notes update needed for this change?: no

Additional documentation: none

@pdurbin pdurbin self-assigned this Jul 9, 2021
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

Looks good.

One additional thing to test or at least think about is that you can see the deaccession details in the database with select versionnote,archivenote from datasetversion and the "versionnote" will now have a period after the reason and before the (optional) additional info (screenshot below). I think this is fine. We can't facet on (or GROUP BY) "versionnote" anyway since it's a combination of these two fields. I only mentioned "archivenote" above because that's where the URL goes, like this (from "develop", no period after "IRB request"):

dvndb=> select versionnote,archivenote from datasetversion;
    versionnote     |      archivenote       
--------------------+------------------------
                    | 
 IRB request foobar | https://www.google.com

Screen Shot 2021-07-09 at 2 52 09 PM

@pdurbin pdurbin removed their assignment Jul 9, 2021
@kcondon kcondon self-assigned this Jul 12, 2021
@kcondon kcondon merged commit 2cd09f8 into IQSS:develop Jul 12, 2021
@djbrooke djbrooke added this to the 5.6 milestone Jul 12, 2021
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.

Please add punctuation to standard deaccession reasons.
4 participants