PR Feedback => tickets

[ShanaLMoore]

ref:

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

• [X] 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?

• [x] 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

• [X] 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.

• [x] 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

• [X] 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

• [x] 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 "

• [x] lib/tasks/sidekiq.rake

• [x] spec/controllers/hyrax/homepage_controller_spec.rb

• [x] spec/features/create_work_spec.rb