curationexperts / epigaea

A MIRA replacement running on Hyrax 2.0
Other
1 stars 2 forks source link

COLLECTIONS: attaching a work to a collection from the work fails #833

Closed mark-dce closed 6 years ago

mark-dce commented 6 years ago

STEPS TO REPRODUCE

  1. Choose create a new work and fill out the "Descriptions" and "Files" tabs OR edit an existing work
  2. Click on the "Relationships" tab
  3. Under the "This work in collections" section, command-click on a collection that is not already selected to add it to the selection list. (i.e. highlight a new collection)
  4. Click on save

EXPECTED BEHAVIOR In the show view for the work, the new collections should be listed under the "Relationships" selection. Also, when I edit the work, the collection should be selected.

ACTUAL BEHAVIOR The show view does not show any changes to the "Relationships" and the edit view doesn't show the collection as selected.

NOTE: I tested the behavior on demo.curationexperts.com and it behaves as described in EXPECTED BEHAVIOR.

mark-dce commented 6 years ago

Hint: user.name and user.user_key were probably used pretty interchangeably since they were the same prior to us making this change: https://github.com/curationexperts/epigaea/blob/9e66a6aab6924cc8ef78f2abbf5e078d86f959f1/app/models/user.rb#L23-L32

We should use user_key in this situation.

bess commented 6 years ago

This is being caused because the values passed in via the :member_of_collection_ids are being stripped before they hit the actor stack, specifically here: https://github.com/samvera/hyrax/blob/master/app/forms/hyrax/forms/work_form.rb#L152

I think it's happening because somehow the member_of_collection_ids is not defined as multiple, so this build_permitted_params method in hydra-editor isn't behaving as expected. However, I cannot figure out where this field is defined. @no-reply Could we take a look together when you're back?

PR with failing test here: https://github.com/curationexperts/epigaea/compare/bugfix/adding_work_to_collection

I can make that test pass by forcing multiple?(:member_of_collection_ids) to return as expected, but I'd like help figuring out where in the code to make that change.

byebug example:

[75, 84] in /Users/bess/.rvm/gems/ruby-2.3.4@epigaea/gems/hydra-editor-3.3.2/app/forms/hydra_editor/form.rb
   75:
   76:       def build_permitted_params
   77:         permitted = []
   78:         terms.each do |term|
   79:           byebug if(term == :member_of_collection_ids)
=> 80:           if multiple?(term)
   81:             permitted << { term => [] }
   82:           else
   83:             permitted << term
   84:           end
(byebug) term
:member_of_collection_ids
(byebug) multiple?(term)
false
(byebug) self
Hyrax::ImageForm
(byebug) self.terms
[:title, :creator, :contributor, :description, :license, :rights_statement, :publisher, :date_created, :subject, :language, :identifier, :related_url, :representative_id, :thumbnail_id, :files, :visibility_during_embargo, :embargo_release_date, :visibility_after_embargo, :visibility_during_lease, :lease_expiration_date, :visibility_after_lease, :visibility, :ordered_member_ids, :source, :in_works_ids, :member_of_collection_ids, :admin_set_id, :title, :displays_in, :abstract, :accrual_policy, :admin_start_date, :alternative_title, :audience, :bibliographic_citation, :contributor, :corporate_name, :createdby, :creator, :creator_department, :date_accepted, :date_available, :date_copyrighted, :date_issued, :date_modified, :date_uploaded, :description, :embargo_note, :end_date, :extent, :format_label, :funder, :genre, :has_format, :has_part, :held_by, :identifier, :internal_note, :is_format_of, :is_replaced_by, :language, :legacy_pid, :personal_name, :primary_date, :provenance, :publisher, :qr_note, :qr_status, :rejection_reason, :replaces, :resource_type, :retention_period, :rights_holder, :rights_note, :geographic_name, :steward, :subject, :table_of_contents, :temporal, :is_part_of, :tufts_license]
(byebug) self.terms.include? :member_of_collection_ids
true
(byebug) term
:member_of_collection_ids
(byebug) multiple?(term)
false