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

Move file downloads into the files table #1831

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Move file downloads into the files table #1831

wants to merge 16 commits into from

Conversation

lfarrell
Copy link
Contributor

@lfarrell lfarrell commented Nov 5, 2024


showImageDownload(brief_object) {
return this.hasPermission(brief_object, 'viewReducedResImages') &&
brief_object.format.includes('Image') && this.getOriginalFile(brief_object) !== undefined
Copy link
Member

Choose a reason for hiding this comment

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

this should also take into account if there is a jp2

@@ -0,0 +1,286 @@
<template>
<div v-if="!isLoggedIn && restrictedFiles(recordData) && hasGroupRole(recordData, 'canViewOriginals', 'authenticated')" class="actionlink download">
<a @click.prevent="modal_open = true" class="download login-modal-link button action" href="#">Contact Wilson/Log in to access</a>
Copy link
Member

Choose a reason for hiding this comment

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

Seems like we should use a t entry for the text here


showNonImageDownload(brief_object) {
return this.hasPermission(brief_object, 'viewOriginal') &&
!brief_object.format.includes('Image') && this.getOriginalFile(brief_object) !== undefined;
Copy link
Member

Choose a reason for hiding this comment

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

We should take into account whether the record has a jp2 or not, since that is what is actually required for the image download options. That could potentially take the place of the format check (seems like we still need there to be an original file too for nonImage downloads)

@@ -1,20 +1,17 @@
<template>
Copy link
Member

Choose a reason for hiding this comment

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

it seems like this file isn't really "restrictedContent" anymore, its more like top level actions for the current record. Maybe we should rename it? I was a bit confused at first why the file still exists since downloadOptions was doing the rendering of the "contact wilson/login" button

expect(wrapper.find('.dropdown-menu').exists()).toBe(false);
});

it('a login button when user is not logged in, but authenticated users canViewOriginals and viewReducedResImages but image is smaller than min size', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
it('a login button when user is not logged in, but authenticated users canViewOriginals and viewReducedResImages but image is smaller than min size', async () => {
it('shows a login button when user is not logged in, but authenticated users canViewOriginals and viewReducedResImages but image is smaller than min size', async () => {

@lfarrell
Copy link
Contributor Author

Should also fix https://unclibrary.atlassian.net/browse/BXC-4778 now.

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.

2 participants