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.

Sign up for GitHub

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

Jump to bottom

#313: Adds delete button to PubMed articles in GDMs with no evidence #2092

Open
wants to merge 41 commits into
base: dev
Choose a base branch
from
Open

#313: Adds delete button to PubMed articles in GDMs with no evidence #2092

wants to merge 41 commits into from

Conversation

h-tong
Copy link
Contributor

@h-tong h-tong commented Dec 9, 2019

Steps to Test #313

  1. Begin curation of a GDM
  2. Add a new PMID with the "Add New PMID" button on the left panel of the GDM
  3. Expect to see a delete button appear in the center PMID panel next to the "PubMed" button
  4. Click the delete button and expect the PMID to be removed and to be navigated to the first article in the GDM or none if there are no other articles in the GDM
  5. Try adding evidence or article notes and expect the delete button to be hidden.
  6. Delete all evidence and article notes and expect the delete button to be shown again.

Note: This branch is branched from #2025

Howard Tong and others added 29 commits August 19, 2019 10:19
#1486: Adds notes field for PubMed articles
d799a7a
Redo data dump
2ba1225
Adds data dump again
b899037
Merge branch 'dev' of https://github.com/ClinGen/clincoded into 1486_…
5d6aac1 
…ht_pubmed_notes
Add notes to history, text box for each tick box
8088831
Fixes bug with text area for notes
4139625
Refactors handle functions, fixes comment
a65851e
Modifies comments
6e11eb5
Merge branch 'dev' of https://github.com/ClinGen/clincoded into 1486_…
1c0518f 
…ht_pubmed_notes
Fixes jsx style
e7ef9ac
#313: Adds delete pmid button
f3ba7b6
Edits text for no non-scorable evidence
ec0ff02
Merge branch '1486_ht_pubmed_notes' of https://github.com/ClinGen/cli…
eac3e63 
…ncoded into 313_ht_pmid_delete
Adds delete to history, but leaves hidden for now
92ea202
Pull dev, ignore dump conflicts
b6a84c3
Fixes data dump from merge
72b07a5
Pulls dev, ignores data dump conflicts
673ef9f
Fixes data dump conflict from dev merge
30c344a
Fix desc and title in JSONs, add undefined checks
3fd9a73
Pulls #1486, ignore data dump
0c91729
Fixes data dump from merge conflict
8d379e4
Pull dev, fix form, ignore data dump conflicts
21a912e
Fixes data dump conflicts
6985289
Pulls dev, ignores data dump conflicts
cdcfb5a
Fixes data dump conflicts
3b004e2
Merge branch 'dev' of https://github.com/ClinGen/clincoded into 1486_…
6a05f2a 
…ht_pubmed_notes
Sets status of deleted annotation to deleted
eb11002
Modifies delete pmid btn logic, adds error catches
a65396a
Removes commented code
e531033
@markmandell markmandell self-requested a review January 10, 2020 19:15
markmandell
markmandell reviewed Jan 14, 2020
View reviewed changes
src/clincoded/schemas/annotation.json
@@ -91,6 +91,47 @@
"title": "Affiliation",
"description": "The affiliation this annotation is associated with.",
"type": "string"
},
"articleNotes": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have to update schema version here? Saw that this schema is liked to curatorHistory, not sure if we forgot or if it's not necessary due to the link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you're right, the version should be updated. I feel like I purposefully left it unchanged for a reason, but I may have just forgotten. I'll add it in.

markmandell
markmandell reviewed Jan 14, 2020
View reviewed changes
src/clincoded/schemas/curatorHistory.json
@@ -14,7 +14,7 @@
],
"properties": {
"schema_version": {
"default": "3"
"default": "5"
Copy link
Contributor

@markmandell markmandell Jan 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Should schema version be "4" here? If not, let me know!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is 5 because this is branched off of the pubmed notes branch. The other branch upgrades to 4 and this branch upgrades to 5.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sweet, thanks for clarifying!

markmandell
markmandell reviewed Jan 14, 2020
View reviewed changes
Copy link
Contributor

@markmandell markmandell 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 to me! Will wait for that small schema change before approving 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markmandell markmandell markmandell approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@h-tong @markmandell