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

6587 deleted files shown on grant permission #6659

Merged
merged 27 commits into from
Feb 21, 2020

Conversation

sekmiller
Copy link
Contributor

What this PR does / why we need it:
This PR allows the admin to deal with file permissions without having to wade through files that have been deleted from previous dataset versions. through the use of a check box she may include deleted files in case there are residual permissions on the deleted files.

Which issue(s) this PR closes:

Closes #6587 Deleted files showing up as duplicates in Grant File Permissions

Special notes for your reviewer:
Added "Transient" fields to DataFile and Role Assignment Row for ease of use

Suggestions on how to test this:
Add permissions or request access on files, then delete them.

Does this PR introduce a user interface change?:
Minor UI change, reviewed with Sonia.

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

Additional documentation:
None

@coveralls
Copy link

coveralls commented Feb 18, 2020

Coverage Status

Coverage decreased (-0.006%) to 19.465% when pulling e6276d7 on 6587-deleted-files-shown-on-grant-permission into 18e2813 on develop.

Copy link
Contributor

@mheppler mheppler left a comment

Choose a reason for hiding this comment

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

Added a few inline comments. Screenshots of the UI included in the issue or PR would help when reviewing. Didn't look at the java changes, if someone else wants to take a peek at those.

src/main/webapp/permissions-manage-files.xhtml Outdated Show resolved Hide resolved
src/main/webapp/permissions-manage-files.xhtml Outdated Show resolved Hide resolved
@mheppler mheppler assigned sekmiller and unassigned mheppler Feb 18, 2020
@djbrooke djbrooke moved this from Code Review 🦁 to IQSS Team Dev 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Feb 19, 2020
@djbrooke djbrooke moved this from IQSS Team Dev 💻 to Code Review 🦁 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Feb 19, 2020
@djbrooke djbrooke assigned mheppler and unassigned sekmiller Feb 19, 2020
@mheppler mheppler assigned sekmiller and unassigned mheppler Feb 19, 2020
@sekmiller
Copy link
Contributor Author

@mheppler as discussed I added the "include deleted files" check box to both panels. (Please see lines 53 and 165 - for additional formatting improvements.) Also, as discussed, I'll leave it to you to add the file path and re-style the name/path.

Thanks!

@mheppler mheppler self-assigned this Feb 19, 2020
@mheppler
Copy link
Contributor

Talked to @sekmiller, talked to @sbarbosadataverse, going to try another approach. Rather than continuing with my effort to add more "Include Deleted Files" checkbox toggles to each of the tables in the panels and popups on the Restricted File Permissions pg, I am going to try to have only one checkbox toggle above the Restricted Files panel datatable, and remove the deleted files from ever displaying in the Grant File Access popup datatable.

As Sonia pointed out, "there is no need to grant access to a deleted file", so there is no need to display deleted files in the Grant File Access popup and no need for a checkbox toggle.

Here is what the Grant File Access popup looks like with the changes as of now, which include my attempt to add a checkbox toggle to the table.

Screen Shot 2020-02-19 at 4 26 19 PM

@djbrooke djbrooke moved this from Code Review 🦁 to IQSS Team Dev 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Feb 20, 2020
@mheppler mheppler assigned sekmiller and unassigned mheppler Feb 20, 2020
Copy link
Contributor

@landreev landreev left a comment

Choose a reason for hiding this comment

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

Short of testing it myself, everything looks good.

@landreev
Copy link
Contributor

@sekmiller I moved it into qa; one nitpicking thing though: there seems to be an extra semicolon typo-ed in line 153 of ManageFilePermissionsPage.java

@mheppler
Copy link
Contributor

Latest and greatest UI.

Screen Shot 2020-02-21 at 8 48 12 AM

Copy link
Contributor

@mheppler mheppler 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.

@kcondon kcondon self-assigned this Feb 21, 2020
@kcondon kcondon merged commit 7227c38 into develop Feb 21, 2020
@kcondon kcondon deleted the 6587-deleted-files-shown-on-grant-permission branch February 21, 2020 19:43
@djbrooke djbrooke added this to the 4.20 milestone Feb 24, 2020
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.

Deleted files showing up as duplicates when granting permissions, Add path if it exists
6 participants