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

IQSS/7687-Curate command updates #7688

Conversation

qqmyers
Copy link
Member

@qqmyers qqmyers commented Mar 15, 2021

What this PR does / why we need it: Fixes a bug where the Curate (update an existing published dataset) command, usable by superadmins, didn't correctly update the file access request allowed flag if it was changed in the update. The PR also include a couple other fixes updates - at QDR, file name updates were being dropped, noticed after the equivalent of #7337 was merged. Also changing the permission annotation (see issue discussion).

Which issue(s) this PR closes:

Closes #7687

Special notes for your reviewer: The file access request issue was just discovered and I've verified the fix at QDR. The change to add a merge of the datasetversion makes sense, but I'm just catching up from last year on that and my notes indicate that I thought some change related to #7337 could be involved, but it's possible that the problem affects develop.
W.r.t. addressing legacy db issues - I hope use is fairly rare and that if there were any name issues, those would have been caught/reported. The file access flag was more subtle, so easier to miss. As I noted in the issue, I'm not sure a flyway script makes sense but I could add a release note (that could either point to the query I listed that would help find instances and/or just point out that you can just do the update again to fix any affected datasets). Let me know what makes the most sense.

Suggestions on how to test this: Change the file request access flag in the Terms panel and update a file name, then use the Curate command and verify that Terms panel shows the flag update and/or that a restricted file acts as specified by the flag (requesting access is/isn't allowed), and make sure the file name changed.

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?: Some note along the lines of 'Bugs were found in the functionality allowing superadmins to update a published dataset version (without creating a new version) involving the Terms setting on whether to allow file access requests and with file name changes. If you have used this functionality, issue #7687 provides details and includes an sql query that will help in identifying any affected dataset.'

Additional documentation:

@qqmyers qqmyers changed the base branch from IQSS/7687-Curate_command_updates to develop March 15, 2021 19:01
Copy link
Contributor

@scolapasta scolapasta 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 (and good catch!). I'll leave it to @djbrooke to determine what we include in the release notes (my vote is a reference to the issue could be enough).

IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Review 🦁 to QA 🔎✅ Mar 15, 2021
@djbrooke djbrooke self-assigned this Mar 15, 2021
@djbrooke
Copy link
Contributor

(will review and add release note when I have a few)

@djbrooke djbrooke assigned scolapasta and unassigned djbrooke Mar 16, 2021
Copy link
Contributor

@scolapasta scolapasta left a comment

Choose a reason for hiding this comment

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

Re-approved for script and release notes.

IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Review 🦁 to QA 🔎✅ Mar 16, 2021
@kcondon kcondon self-assigned this Mar 22, 2021
@kcondon
Copy link
Contributor

kcondon commented Mar 23, 2021

@qqmyers The publish in progress banner and page update is missing. Otherwise works
This was due to having a postpublish log step enabled. when I removed it, banner returned.

@kcondon kcondon moved this from QA 🔎✅ to Community Dev 💻❤️ in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Mar 23, 2021
@kcondon kcondon assigned qqmyers and unassigned kcondon Mar 23, 2021
@kcondon kcondon moved this from Community Dev 💻❤️ to QA 🔎✅ in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Mar 23, 2021
@kcondon kcondon assigned kcondon and unassigned qqmyers Mar 23, 2021
@kcondon kcondon merged commit f041e5c into IQSS:develop Mar 23, 2021
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from QA 🔎✅ to Done 🚀 Mar 23, 2021
@djbrooke djbrooke added this to the 5.4 milestone Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

CuratePublishedDatasetVersionCommand doesn't update everything
4 participants