Skip to content

Commit

Permalink
Compacting array building logic
Browse files Browse the repository at this point in the history
This commit serves two purposes:

1) First, and most important, it fixes #4833
2) Second, it unifies the logic for determining the ids

More on 4833 (in case you don't have access to Github)

The observed error is as follows:

> Depositing a work with past embargo gives no id error
>
> Depositing a work with an embargo date in the past gives an "No ID
> provided for ActiveFedora::Base" error.
>
> Expected Behavior: Depositing a work with a past embargo should fail
> to be created and entered information should be preserved

In debugging the application, the `@object.member_of_collection_ids` was
the following: `["", "admin_set/default"]`

That appears to come from the form submitting a select box that has no
selected value (e.g., the `""` value).  I verified this logic change by
UI interaction.
  • Loading branch information
jeremyf committed Apr 26, 2021
1 parent 42c51a5 commit fa9c404
Showing 1 changed file with 1 addition and 7 deletions.
8 changes: 1 addition & 7 deletions app/services/hyrax/edit_permissions_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,7 @@ def object_unauthorized_collection_ids
# find all of the collection ids an object is a member of
# @return [Array] array of collection ids
def object_member_of_ids
@object_member_of_ids ||= begin
belongs_to = []
# get all of work's collection ids from the form
belongs_to += @object.member_of_collection_ids
belongs_to << @object.admin_set_id if @object.admin_set_id.present?
belongs_to
end
@object_member_of_ids ||= (@object.member_of_collection_ids + [@object.admin_set_id]).select(&:present?)
end

# The list of all collections this user has manage rights on
Expand Down

0 comments on commit fa9c404

Please sign in to comment.