Skip to content

3758 file page preview #6325

Merged
kcondon merged 53 commits intodevelopfrom
3758-file-pg-preview
Oct 31, 2019
Merged

3758 file page preview #6325
kcondon merged 53 commits intodevelopfrom
3758-file-pg-preview

Conversation

@sekmiller
Copy link
Contributor

New Contributors

Welcome! New contributors should at least glance at CONTRIBUTING.md, especially the section on pull requests where we encourage you to reach out to other developers before you start coding. Also, please note that we measure code coverage and prefer you write unit tests. Pull requests can still be reviewed without tests or completion of the checklist outlined below. Note that we use the "closes" syntax below to trigger Github's automation to close the corresponding issue once the pull request is merged.

Thanks for your contribution to Dataverse!

Related Issues

Pull Request Checklist

  • Unit [tests][x] NA
  • Integration [tests][x]: None
  • Deployment requirements, [SQL updates][x]
  • Documentation completed
  • Merged latest from "develop" branch and resolved conflicts

mheppler and others added 30 commits August 9, 2019 16:13
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.

The code looks ready for QA, I believe but the docs seem to make assertions that aren't entirely accurate and I have a couple other comments as well.

@djbrooke
Copy link
Contributor

@pdurbin @sekmiller thanks for the feedback, I attempted to address it in a992e0e. If there are still questions let's talk in the morning, but if it looks all good feel free to send it on to QA.

@djbrooke djbrooke requested a review from pdurbin October 31, 2019 03:20
@djbrooke djbrooke assigned sekmiller and unassigned sekmiller and djbrooke Oct 31, 2019
@djbrooke
Copy link
Contributor

Moving over to QA, we talked about the docs in standup and everything is good there.

@kcondon kcondon self-assigned this Oct 31, 2019
@kcondon
Copy link
Contributor

kcondon commented Oct 31, 2019

So, found a couple issues that need decisions on:

  1. restricted file in v2 hides preview but in v1 shows preview and error on getting file, same as download and explore behavior, related to no distinct file versions.
  2. tou in v2 hides preview but in v1 shows preview and not prompted for tou.
  3. gb in v2 hides preview and in v1 hides preview.

@scolapasta
Copy link
Contributor

One possible solution to keep this moving is to not show previews on older versions for now. (i.e. we should treat this in the context if the already known issue that older versions show download buttons when later version is restricted).

@djbrooke
Copy link
Contributor

I just talked briefly with @sekmiller and @scolapasta. I'm OK not showing previews on previous versions in order to deal with this issue for now as I'd like to get this out because it provides immense value. @sekmiller is going to make the change.

@sekmiller
Copy link
Contributor Author

Fix for only showing File Preview tab when you're on the latest version of the file is checked in and available for verification.

@kcondon
Copy link
Contributor

kcondon commented Oct 31, 2019

current version change looks good but found one additional issue, that is I had seen it but just realized it was an issue:

  1. if a file is restricted but you have access, then there should be a preview but there is not.

@sekmiller
Copy link
Contributor Author

OK. Actually, since we're using the same logic as the display of the public download URL, if the file is restricted you cannot see the preview even if you have access.

@djbrooke
Copy link
Contributor

@sekmiller @kcondon Thanks, this is how I expected it to work before editing the docs yesterday and it's acceptable to me. :) I'll revert those changes.

@djbrooke
Copy link
Contributor

Reverted in 29a7edc

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.

File preview on the file page only

7 participants