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

Checksum truncation, click to copy + file metadata fragment code consolidation #7312

Merged
merged 41 commits into from Feb 3, 2021

Conversation

mheppler
Copy link
Contributor

@mheppler mheppler commented Oct 7, 2020

What this PR does / why we need it:

Long checksums (see SHA512) are too long to display in the file table, so they are truncated to only display first 3 and last 3 characters, with a hover tooltip to display the entire checksum/UNF string and provide "click to copy" directions for the user. Added word-break CSS to wrap long checksums to another line within container widths across dataset, file and upload/edit pg metadata views.

Overwrite css property from PrimeFaces component.css that was applying the hidden value to the overflow property of table cells in the dataTable component. Applied a new style attribute to the file dataTable on the dataset pg, so that it no longer cuts off long strings when the overflow is hidden by the width of the containing table cell.

Consolidated file metadata on dataset pg by using file-info-fragment from file replace pg. For the files table on the dataset pg, filesFragment.xhtml uses 87 lines code that can also be found on files-info-fragment.xhtml, which was created as part of file replace. This is an attempt to clean up and consolidate that code, making it easier to manage going forward. Already there was not-all-that-recently added file metadata displayed on the dataset pg that was not added to the file replace pg (e.g. file path/hierarchy).

There were also other responsive layout improvements to the file table, as part of the constant effort to improve the UI and UX for all users, on all devices.

Which issue(s) this PR closes:

Closes #5210 Long (e.g. SHA-512) fixity checksums are too long for display (NOTE, this issue was already closed and consolidated into #6685, but this PR does not address all issues outlined in that effort)

Closes #5215 Search Result Cards - Missing UNF + Variables, Observations for tabular files

Special notes for your reviewer:

This will effect the file metadata, thumbnails, tags, unfs, downloads on the dataset pg and file replace pg and S3-download dataverse package popup.

All checksums/UNF's in the file table on the dataset pg and file cards on the dataverse pg, will now be truncated, displaying just the type label (e.g. MD5, SHA512, UNF:6:, et al), then the first three unique characters, then "..." in place of the truncated middle portion of the string, and then the last three unique characters (NOTE: UNF strings end with either a "=" or "==" suffix, so three characters will be displayed prior to those characters).

Suggestions on how to test this:

Hover over the truncated checksum in the file table on the dataset pg, as well as file result cards on the dataverse pg, to see the full checksum. Click to copy to clipboard. You can also see the checksum info logged in the browser console.

Check out the dataset version differences popup and make sure all long checksums are displayed in full.

Make sure the file metadata, thumbnails, tags, unfs, downloads on the dataset pg and file replace pg and download dataverse package on S3 popup are all kosher still.

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

No mockups. Checksum is truncated to only show first 3 and last 3 digits. Hover to see full checksum displayed in a browser title tooltip.

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

N/A

Additional documentation:

N/A

@mheppler
Copy link
Contributor Author

mheppler commented Oct 7, 2020

TO-DO

  • restore file download count, commented out due to following null pointer

See line 55 of file-info-fragment.xhtml, for an outstanding to-do, as I had to comment out the file metadata download count because it resulted in a nullPointer on the dataset pg.

  Error Rendering View[/dataset.xhtml]
javax.el.ELException: /file-info-fragment.xhtml @56,98 value="#{DatasetPage.getGuestbookResponseCount(fileMetadata)}": java.lang.NullPointerException
	at com.sun.faces.facelets.el.TagValueExpression.getValue(TagValueExpression.java:77)
        ...

@coveralls
Copy link

coveralls commented Oct 7, 2020

Coverage Status

Coverage increased (+0.004%) to 19.628% when pulling dfa367e on 7081-file-metadata-include into bd91444 on develop.

@mheppler mheppler marked this pull request as draft October 8, 2020 14:01
@mheppler
Copy link
Contributor Author

mheppler commented Oct 8, 2020

Resolved nullPointer resulting from getGuestbookResponseCount in the file-info-fragment with the supervision of @pdurbin. Pointed the include to the guestbookResponseServiceBean instead. Removed no longer necessary backing bean code from DatasetPage.

Also cleaned up the thumbnail code in the include, as it was pointing to EditDatafilesPage for render logic, so pointed it to dataFileServiceBean instead. Removed no longer necessary backing bean code, including !fileDownloadHelper.canDownloadFile(fileMetadata) logic, as I believe we do not need to check for download permissions on an edit files pg. Maybe @scolapasta can look into this in more detail and determine if this is in fact a kosher assumption, or if render logic needs to be returned to the xhtml thumbnail code or something.

@mheppler mheppler marked this pull request as ready for review October 8, 2020 15:36
@mheppler
Copy link
Contributor Author

mheppler commented Oct 15, 2020

Initial checksum truncation has also been lumped into this PR... WIP (WORK IN PROGRESS)... See commit 81384ba related to issues #6685 #5210... Easy enough to comment out the changes on dv_rebind_bootstrap_ui.js and file-info-fragment.xhtml if need be...

Screen Shot 2020-10-15 at 1 04 03 PM

@djbrooke
Copy link
Contributor

We can put these changes up against the develop branch as independent code cleanup after Kebabs are merged

Base automatically changed from 7081-kebab-file-options-icon-btn to develop November 6, 2020 14:33
@mheppler
Copy link
Contributor Author

Commented out the checksum click to copy code, as it was not yet complete. This gets the scope of this PR a little smaller, and easier to move forward.

Screen Shot 2020-12-17 at 11 39 07 AM

@mheppler
Copy link
Contributor Author

mheppler commented Jan 5, 2021

This PR has been updated to include truncating of the checksum and click to copy in the file table on the dataset pg and in the file result card on the dataverse pg. There is considerable code clean up and responsive improvements included, but more work to do on both of those front, so this PR will only be linked to the already consolidated and closed issue Long (e.g. SHA-512) fixity checksums are too long for display #5210.

Screen Shot 2021-01-05 at 3 11 55 PM

@mheppler mheppler removed the request for review from scolapasta January 5, 2021 20:31
@mheppler mheppler changed the title Consolidated file metadata on dataset pg by using file-info-fragment from file replace pg Checksum truncation, click to copy + file metadata fragment code consolidation Jan 6, 2021
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Review 🦁 to QA 🔎✅ Jan 26, 2021
@mheppler mheppler removed their assignment Jan 27, 2021
@kcondon kcondon self-assigned this Jan 27, 2021
@kcondon
Copy link
Contributor

kcondon commented Jan 28, 2021

Things noticed, not necessarily bugs:

  1. selecting copy md5 in search card gives no feedback that it did anything, unlike a button, but works.
  2. selecting copy md5 in dataset file table selects file and "flashes" full md5 view for all files temporarily. More noticeable with more files.
  3. 0 downloads still appears on dataset and file pages below fancy action stack in draft both mdc and legacy.
  4. downloading all or batch expands md5s and leaves them expanded.

@kcondon kcondon removed their assignment Jan 29, 2021
@kcondon kcondon moved this from QA 🔎✅ to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Jan 29, 2021
@mheppler
Copy link
Contributor Author

mheppler commented Feb 3, 2021

Based on the demo feedback, I've cleaned up the follow...

  • file dataTable on dataset pg no longer selects file row when you click in the row, you have to click the checkbox
  • copy checksum/UNF on dataset pg no longer selects file row in dataTable
  • copy checksum/UNF now swaps the clipboard icon to a green "success" OK icon when successfully copied
  • added a bunch of oncomplete="bind_bsui_components();" attributes to buttons and links that were missing them to fix breaking the copy to clipboard and truncation functions when ajax updates the page

Sending back to QA.

@mheppler mheppler moved this from IQSS Team - In Progress 💻 to QA 🔎✅ in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Feb 3, 2021
@mheppler mheppler removed their assignment Feb 3, 2021
@kcondon kcondon self-assigned this Feb 3, 2021
@kcondon kcondon merged commit 78a1044 into develop Feb 3, 2021
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from QA 🔎✅ to Done 🚀 Feb 3, 2021
@kcondon kcondon deleted the 7081-file-metadata-include branch February 3, 2021 23:23
@kcondon
Copy link
Contributor

kcondon commented Feb 3, 2021

Works great @mheppler

@djbrooke djbrooke added this to the 5.4 milestone Feb 3, 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
7 participants