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

3758 file page preview #6325

Merged
merged 53 commits into from
Oct 31, 2019
Merged

3758 file page preview #6325

merged 53 commits into from
Oct 31, 2019

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
@pdurbin pdurbin removed their assignment Oct 30, 2019
@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
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