Closed rmjaffe closed 1 year ago
There's a couple things happening here:
Re: #2 At present and by happenstance, most of the test objects I've been playing with are components of complex objects. But in a closer to real world use case, it's unlikely that we would want to remove an inherited value from a child work using the edit form in the DAMS. If there was a situation in which we would want to remove inherited values from a child work, it's also more than likely that the child work should be estranged from that parent. Right now I'm thinking we don't really need a fix or a workaround strategy -- it's enough for us to be aware that values re/inherit each time a work is saved. Does this sound reasonable?
That sounds perfectly reasonable to me. I'll note this exception in the acceptance criteria above.
The source of this problem is code in work_actor.rb, introduced to fix the issues in this ticket. The problem there was that removing controlled vocabulary terms with Bulkrax round-trips didn't work when the spreadsheet used enumerated columns. The fix for that problem caused the problems in this ticket.
Later in the project, we decided to change Bulkrax to export using a single column for multi-valued fields, instead of enumerated columns. This means that attempting to round-trip with enumerated columns involves guesswork, insofar as we don't know precisely which value corresponds to which number in a multi-valued field. Talking this over with Rachel, it was agreed that we are not likely to need to do round-trips with enumerated columns.
So the forthcoming PR that resolves this ticket does so by commenting out the code that fixed the previous issue, and any future attempts to update controlled vocabulary terms with enumerated columns in a Bulkrax round-trip will likely fail. When updating controlled vocabulary terms with a Bulkrax round-trip, the preferred method is to use a single column with delimiters, which will replace all the existing values (aka keep values in the sheet if you want them retained on the work).
Edited the above comment, as the code to remove controlled terms using enumerated columns was able to be preserved. Additional logic was added to this function to fix the problem in this issue. All the technical details can be found in the PR linked above.
@rmjaffe this one is ready for QA. Note that inheritance rules can make this appear to not work. I just went through testing a case where a subject term was on the collection and kept re-inheriting on the work, making it appear as if removing it was not working. Removing it first from the collection fixed the issue. This can lead to additional issues if there are other controlled terms on the collection, which then get saved incorrectly due to #524.
I just tested this with a parent-child work image set and found the same: I can now remove all the terms from a given property if those terms are unique to the work, but if they were inherited from the parent the will just keep reinheriting unless removed from the parent record. And in may cases removing them from the parent record is of course not optimal from an access/description POV.
Just to see what happened, I removed some terms from the parent record and then experimented with editing the child records. When deleting those same terms off the child, the terms disappeared from view (expected), but when editing another metadata property and saving, those terms that had been removed from the parent did not disappear from the child (expected). So terms removed from the parent would not automatically disappear of child works; they'd need to be manually removed.
Wondering if we should have a conversation about the sustainability/practicality of the inheritance model. It seems to be pain in the butt to track and manage the impacts of on-off edits to records and now to collections (i.e. the modification of the default inheritance rules for the aerials). The purpose was to save time on the metadata creation end, but instead of saving time, it just pushed it onto the metadata/data model management end.
Agreed. I will start a document that outlines the current state and some of the potential issues I see with it.
@rschwab Are the values that we remove from the work-edit page removed and re-indexed on solr or we remove it from fedora all-together? My guess is...inheritance updates the fedora repo...it may be re-appearing because of that.
Yes the inheritance rules happen when the work is saved, ie into the repository. There used to be inheritance rules that happened in the solr indexer but we commented those out.
@rschwab I understand. But how about the works being updated after we remove the controlled field values? Are they updated on fedora as well?
Yeah, that is my understanding. So the controlled term is removed, and the save method is called. It removes the terms, then calls inheritance, grabs the term from the parent, and saves it back to the work in fedora.
Tested this out adding and removing values to the 538 test work in Production; was successfully able to remove all values from non-required metadata properties.
Descriptive summary
When editing metadata using the editing form in the DAMS, there are occasions where we will need to remove information from metadata records, particularly in response to patron feedback (example: the circumstance outlined in #512). This removal might include removing all the text from a metadata property (e.g. description) or all the controlled values from subjectName, subjectPlace, etc.
We want to be able to remove all values from any of the non-required metadata properties (as in, from every property except title).
Background
The original issue, that Hyrax would permit removal of all but one of a list of controlled terms, was observes during QA of #512.
Acceptance criteria
Done means:
Related work
512