Add new Units model, controller, views #6616
Conversation
cjcolvar
left a comment
cjcolvar
left a comment
There was a problem hiding this comment.
Wow! Great work! You've filled out a lot of the functionality beyond just scaffolding!
I did a first pass and overall looks really good. I'll have to digest it a bit more to consider if we want to make any more fundamental changes to the approach taken in this PR. But I'm sort of inclined to merge now and make any changes in follow on PRs.
| temp_solr_parameters = {} | ||
| add_access_controls_to_solr_params_if_not_admin(temp_solr_parameters) | ||
|
|
||
| query = "{!join from=heldBy_ssim to=id}" |
There was a problem hiding this comment.
I think we may need to doubly join here to get from the unit down to the media object. I forget if it works to chain them:
{!join from=heldBy_ssim to=id}{!join from=isMemberOfCollection_ssim to=id}
There was a problem hiding this comment.
Do we need to drill down to the media object? In the current implementation, this is primarily (solely?) used by the Units index where my assumption was we would be only concerned that child collections were being returned.
This commit creates (hopefully) all the controllers, models, and views for the new Units structure. Initial code for the Units is copied from the Admin::Collection model and given some tweaking. Tests are not passing in the work for this commit. TODO: Properly map hierarchy of permissions for Unit. Do managers, editors, depositors have permissions to do anything to the Unit or are they only relevant for being assigned to child collections? If they do have permissions, how do they differ from the new unit administrator role?
This commit reworks collections to belong_to a Unit. Is this the approach that should be taken? This would require a large effort data remediation to create Unit entries for all existing units in a repository and making sure that all collections are assigned to the proper new Unit, while retaining all the current permissions structures. Is there a way to have both the modelled Units and the controlled vocab based units exist alongside each other to ease the remediation burden?
This commit effectively copies and tweaks the collections ability checks over to the Units. There will need to be a conversation on if these changes are all that is necessary.
…ns for updating pulling unit name from the RDF
This commit makes use of the auto-complete-element 'data-autocomplete-value' feature which allows a separate input value to be assigned to a given display value. The issue is that when a selection is made, the input is changed to the input value. So a user inputting 'Defaul' then selecting 'Default Unit' will see the input field change to the unit id. This could be confusing for users, but might be alright?
Co-authored-by: Chris Colvard <[email protected]>
Use autocomplete for unit selection on collection new/edit
Inherit roles from units
Related issue: #6054
By changing Units from a controlled vocab list to a model, this PR changes Collections to have a foreign key relationship to the unit. Is this how it should actually be implemented? There is definitely the
has_many -> belongs_todatabase relationship, but remediating the data on application update is likely significant. All existing Unit values would need to have a corresponding instance created and then each collection would need to be changed to have the appropriate foreign key. This is probably doable with a rake task, but we need to make sure it is ironclad.Edit: created potential rake task. It uses
update_allwhich is more efficient since it is a single database injection, but does skip validations and callbacks. Since this is a rake task that should only need to be used once, maybe it is better to cycle through and individually update each record so all the callbacks and everything are properly set off...TODO:
Unitstab like we have with collections? Or is the Unit meant as an administrative aspect that is only surfaced for organizing the collection index? If we need the tab, the views need to be added and controller/model code needs to be tweaked for functionality. If we are not adding the tab we can likely remove some of the files added in this PR.