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

🚧 i205 - add ability to upload collection thumbnails #2206

Merged
merged 9 commits into from
May 21, 2024

Conversation

ShanaLMoore
Copy link
Collaborator

@ShanaLMoore ShanaLMoore commented May 6, 2024

Issue:

The ability for collections to have thumbnails will be deprecated in Hyrax v5.0.0.

This PR implements the ability to upload collection thumbnails by following the precedence set by the collection's branding's banner and logo uploads.

Ideally we'd contribute this back if the community supports it. However, for the sake of time we are overriding Hyrax instead.

TODO:

  • FRONTEND: render image for collection thumbnails (ie: catalog or collection index page should render the uploaded images)
  • FRONTEND: thumbnails should link to the associated collection from the catalog page
  • code clean up: there's a lot of similar code since it was basically copied from existing functionality. It'd also be nice to make partials out of the form_branding.html.erb
  • thumbnail alt text not saving. the img must be completely removed first.
  • add and update specs
  • write a rake task to migrate existing collection thumbnails to valkyrie collections
# altext should've been "test" 
# Steps to reproduce: create a collection. upload a thumbnail and save. enter alt text and save. 
# Noticed that you must upload the thumbnail AND enter the alt text before saving for it to persist. 

CollectionBrandingInfo.where(role: thumbnail) => 
[#<CollectionBrandingInfo:0x0000ffff7e0440d8     
  id: 5,                                         
  collection_id:                                 
   "69afdb60-3a3d-41b1-9ecd-c8a5959516ed",       
  role: "thumbnail",                             
  local_path:                                    
   "/app/samvera/hyrax-webapp/public/branding/69afdb60-3a3d-41b1-9ecd-c8a5959516ed/thumbnail/diego_1_.jpg",                                        
  alt_text: "",
  target_url: "TODO: link to the collection",
  height: nil,
  width: nil,
  created_at:
   Mon, 06 May 2024 23:03:29.140187000 UTC +00:00,
  updated_at:
   Mon, 06 May 2024 23:03:29.140187000 UTC +00:00>]

image

image

image

image

@ShanaLMoore ShanaLMoore added the patch-ver for release notes label May 6, 2024
@ShanaLMoore ShanaLMoore changed the title 🚧 i205 - add ability to upload thumbnails 🚧 i205 - add ability to upload collection thumbnails May 6, 2024
@ShanaLMoore ShanaLMoore force-pushed the i205-upload-collection-thumbnails branch from af4df7c to 609d333 Compare May 6, 2024 23:11
This commit is implemented following the precendence set by collection branding's banner and logo uploads.

TODO:
- add and update specs
- attach image to collection thumbnails
- thumbnails should link to the associated collection
@ShanaLMoore ShanaLMoore force-pushed the i205-upload-collection-thumbnails branch from 609d333 to 750b4ce Compare May 6, 2024 23:16
ShanaLMoore and others added 2 commits May 6, 2024 18:06
This commit will make the collection thumbnails show up in the views
such as in the dashboard list of collections and also in the catalog
search results.  The gitignore was updated to ignore the path where the
thumbnails are stored.
app/assets/stylesheets/hyku.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/hyku.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/hyku.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/hyku.scss Outdated Show resolved Hide resolved
@kirkkwang kirkkwang force-pushed the i205-upload-collection-thumbnails branch 3 times, most recently from 0113466 to 0c9a10b Compare May 8, 2024 16:24
app/assets/stylesheets/hyku.scss Outdated Show resolved Hide resolved
app/assets/stylesheets/hyku.scss Outdated Show resolved Hide resolved
This commit will fix a bug where if the user sets the thumbnail first
and then later sets the alt text, they would have to have to remove the
thumbnail and re-add it and set the alt text at the same time to get the
alt text to be set.
@kirkkwang kirkkwang force-pushed the i205-upload-collection-thumbnails branch from 0c9a10b to 56b5788 Compare May 8, 2024 17:05
This commit will add a rake task to migrate the Hyku Commons collection
thumbnails which uses the /uploads/uploaded_collection_thumbnails/ path
as opposed to a downloads controller endpoint.
@ShanaLMoore ShanaLMoore marked this pull request as ready for review May 9, 2024 15:08
@ShanaLMoore ShanaLMoore requested review from laritakr and removed request for laritakr May 9, 2024 15:08
@ShanaLMoore ShanaLMoore marked this pull request as draft May 9, 2024 15:13
@ShanaLMoore
Copy link
Collaborator Author

ShanaLMoore commented May 9, 2024

Moving back to draft to address spec failures. cc @kirkkwang

Update: Specs were flaky. They passed after re running the pipeline. ✅

@ShanaLMoore ShanaLMoore requested a review from laritakr May 9, 2024 16:57
@ShanaLMoore ShanaLMoore marked this pull request as ready for review May 9, 2024 16:57
This commit will address the code review comments with the following:

- remove the /uploads gitignore, turns out there was a deeper issue with
  where collection branding files were being saved
- remove a stray comment
- remove an unnecessary TODO
- remove an unnecessary method definition in the new rake task
@kirkkwang kirkkwang requested a review from orangewolf May 14, 2024 12:45
@ShanaLMoore ShanaLMoore merged commit 7b10d06 into main May 21, 2024
4 checks passed
@ShanaLMoore ShanaLMoore deleted the i205-upload-collection-thumbnails branch May 21, 2024 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch-ver for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants