-
Notifications
You must be signed in to change notification settings - Fork 492
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
Update the conditions to display image_url in Solr search results for file type #10875
Comments
@GPortas
Please comment if the following should be added/removed:
|
Hi @stevenwinship ! Answering your first list: 3) datafile is not restricted If a datafile is restricted but you have the right permissions, the image is displayed in the search result card, so this condition is not sufficient on its own. For example, I created a file from the admin user and restricted it. Logged in as the admin user, the file card appears like this: Therefore, if we do not return the image_url in restricted files in all cases, we would not be able to display this card. When I log in as a new user (without permissions to access the file), the card appears like this, so I believe the image_url should not be returned in this case: 4) datafile is not embargoed Currently in JSF, if a file is embargoed, all users, including the Guest user (unauthenticated), can view the thumbnail in the file card. Therefore, if we keep this condition, we would not be able to replicate the current behavior on the SPA. A separate issue is whether we want to change this behavior for embargoed files, but it would be a change in the SPA compared to JSF that we would need to discuss. Possibly @qqmyers, who I think has experience with the Embargo feature, can provide more information on this. 2) and 6) Right now, I’m not clear whether the file would be visible in these cases or not, or under which conditions. So far, I haven't found a way to test these scenarios in my local environment. Perhaps someone on the team with more context on these features can clarify this point for us directly; otherwise, we need to replicate these scenarios or analyze the JSF code to confirm this point. Answering your second list: 1) return the image url even if it was not published Should be added. This is necessary to display the image when the file is part of a DRAFT version, as we can see below. 2) one check that was removed and should be added back is user "hasDownloadFilePermission" It makes sense to include it, especially if this check was already there and was removed. 3) size limitations. We used to scale the image down when returning the base64 image. Now we return the url so the caller is responsible for scaling the image. Should we not return the url if the image is too large? If I’m not mistaken, we are using the query parameter imageThumb in the image_url property for files, right? ( |
re: 4) - I think files that are embargoed or have a retention period should follow the access rules for restricted when the embargo is active or the retention period has expired. Without digging, I'm not sure if showing the thumb when embargoed was just missed originally or changed with some later fix. In any case, I'd consider the current JSF behavior to be inconsistent with the design. |
RE: 3) datafile is not restricted This is where the hasDownloadFilePermission was added back to allow access to the restricted file |
I added the conditions to the release note in the PR |
Overview of the Feature Request
We need to update the added conditions in #10855 to determine whether to return image_url or not for a file type result. JSF displays images for a file type result even when it is not published, which does not happen with the newly added logic. We also need to review the other conditions (restricted, harvested...) and understand how it currently works in JSF to return or not return image_url for a file type result.
What kind of user is the feature intended for?
API User
What inspired the request?
What existing behavior do you want changed?
image_url display condition for file search result included in #10855
Any brand new behavior do you want to add to Dataverse?
None
Any open or closed issues related to this feature request?
Are you thinking about creating a pull request for this feature?
Yes
The text was updated successfully, but these errors were encountered: