PR Feedback => tickets

[ShanaLMoore]

ref:

• https://github.com/samvera/hyku/pull/2178/files

• app/controllers/concerns/hyrax/collections_controller_behavior_decorator.rb

I'll create a separate ticket for this to open for discussion. This was contributed back from pals to sort sub collections by title on default. What should the global default behavior be, if otherwise?

• app/controllers/hyrax/dashboard/profiles_controller_decorator.rb

@orangewolf this comes from pals and it got implemented along with the use roles work. I'm going to leave it for now.

ref:

• https://gitlab.com/notch8/palni-palci/-/issues/201

• https://gitlab.com/notch8/palni-palci/-/issues/313

• https://gitlab.com/notch8/palni-palci/-/merge_requests/268

• app/models/concerns/bulkrax/has_matchers_decorator.rb

@orangewolf Jeremy left this comment:

    # @note We likely want to extract something like this to Bulkrax.
    #       However, we probably cannot assume that we have a field named
    #       `based_near` that is in fact the Location field.
    #
    #       On possibility is to have a Hash that has key of :name and value
    #       of a lambda; that Lambdawould receive the given :result and return
    #       the correct value.  As we can see in the implementation below, we
    #       need to consider the `Site.instance` which is a Hyku specific
    #       consideration.

so I think he would agree with you but didn't have the time to think about it and follow through. I can make a ticket for us to handle this later.

• app/models/file_download_stat_decorator.rb

re: @orangewolf

Here is the commit from pals, and here is the original ticket.

also ref ticket

It uses Legato for file analystics reports

It looks like part of this work (app/models/hyrax/statistic_decorator.rb) already got contributed back to hyrax.

file set decorator exists because '# OVERRIDE Hyrax hyrax-v3.5.0 to require Hyrax::Download so the method below doesn't fail'. The only diff between this and the one in hyrax is calling Hyrax::Download

• app/views/catalog/_index_header_list_default.html.erb

re: @orangewolf I think this is ok.

It was taken from pals which duplicates the same exact code in both conditionals; basically it should always display. So it looks like Jeremy dried that bit up and left the remaining conditional to handle collections. Logically I think it looks equivalent.

Moreover, I tested the search and here are my results. We'll have to look into why thumbnails don't render.

https://github.com/scientist-softserv/palni-palci/blob/main/app/views/catalog/_index_header_list_default.html.erb

• app/views/hyrax/base/_related_items.html.erb

@orangewolf this is the entire commit. Looks to be intentional. The commit message says: " Add render of empty related items partial "

• lib/tasks/sidekiq.rake

• spec/controllers/hyrax/homepage_controller_spec.rb

• spec/features/create_work_spec.rb

[ShanaLMoore]

OTHER NOTICED ISSUES

• thumbnails missing from search index pg
  ticket: 🐛 thumbnails missing from search index pg scientist-softserv/hykuup_knapsack#212
  https://github.com/scientist-softserv/palni-palci/blob/main/app/views/catalog/_index_header_list_default.html.erb

• footer/login missing from base domain?
  ticket: 🐛 footer/login missing from base domain? scientist-softserv/hykuup_knapsack#211

• collections can no longer upload a thumbnail. After creating a collection, a user used to be able to upload a thumbnail on edit.
  ticket: 🐛 Unable to import a bulkrax record scientist-softserv/hykuup_knapsack#204

It should look like:

• consider moving app/models/concerns/bulkrax/has_matchers_decorator.rb to bulkrax
  ticket: 🐛 consider moving app/models/concerns/bulkrax/has_matchers_decorator.rb to bulkrax scientist-softserv/hykuup_knapsack#209
  Jeremy's comment:

    # @note We likely want to extract something like this to Bulkrax.
    #       However, we probably cannot assume that we have a field named
    #       `based_near` that is in fact the Location field.
    #
    #       On possibility is to have a Hash that has key of :name and value
    #       of a lambda; that Lambdawould receive the given :result and return
    #       the correct value.  As we can see in the implementation below, we
    #       need to consider the `Site.instance` which is a Hyku specific
    #       consideration.

RELATED?:

• CSV imports with based_near fail. update: actually, all csv imports do (even without based_near). This error is seen in sidekiq from ImportWorkJob.
  ticket: 🐛 Unable to import a bulkrax record scientist-softserv/hykuup_knapsack#204

2024-04-24T21:51:22.442Z pid=547 tid=b8n7 WARN: RSolr::Error::Http: RSolr::Error::Http - 400 Bad Request
Error: Unable to index docs with children: the schema must include definitions for both a uniqueKey field and the '_root_' field, using the exact same fieldType

generic_work_locations_test.csv

• app/controllers/concerns/hyrax/collections_controller_behavior_decorator.rb
  ticket: 🐛 allow users to choose their default sort behavior scientist-softserv/hykuup_knapsack#210

What should the default be?

• confirm iiif print split works
  PR: Fixes PDF splitting for Valkyrie resources scientist-softserv/iiif_print#346
• Lazy migration of fileset derivative
  ticket: 🐛 Valkyrie: double_combo FileSet Derivatives don't get copied over scientist-softserv/hykuup_knapsack#206

[ShanaLMoore]

HYKU v6.0.0 is a show stopper until we fix:

• bulkrax
• Collection can support thumbnails: Valkyrie: Edit collection using PcdmCollection raises exception undefined thumbnail_id hyrax#5286
• File set derivatives don't get copied over. Only thumbnails do
• Iiif Print

@ShanaLMoore to create tickets