This actually is a follow-up of bug 29146. Currently, an import to a new record gets the defaultvalues. But when you overwrite an existing record (so you have a biblionumber), you wont get them. This is confusing and inconsistent. Since we import external data, we should imo not add our framework defaults. But this feels like it needs a bit more discussion or consensus. What do you think?
Created attachment 128082 [details] [review] Bug 29592: Do not apply defaultvalue to imported record The condition breedingid excludes records from Z3950. Test plan: Import to new or existing record. Verify that defaults are not applied. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 128083 [details] [review] Bug 29592: Counterpart for authorities Similar adjustment. Check the breedingid again. Test plan: Same as former patch for authorities editor. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Martin, Nick: What do you think?
Well, any feedback is surely welcome. We could argue that this approach is the most logical one: We want to see the external record, not our defaults for a new record. And if needed, we could change external records via the XSLT step on the Z3950 target.
Sorry, been a busy time as usual. I think you are right that it's a bit confusing/inconsistent at the moment but as I'm not all that close to cataloguing personally I need to ask colleagues for an opinion to really know what would be best.
Created attachment 131149 [details] [review] Bug 29592: Do not apply defaultvalue to imported record The condition breedingid excludes records from Z3950. Test plan: Import to new or existing record. Verify that defaults are not applied. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 131150 [details] [review] Bug 29592: Counterpart for authorities Similar adjustment. Check the breedingid again. Test plan: Same as former patch for authorities editor. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
(In reply to Katrin Fischer from comment #6) > Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Great. Thanks.
Seems correct to me but it would be great to have Nick's opinion on this one.
Noting that I've written a patch for Bug 30250, also a follow-up of Bug 29146, which adds a system preference ApplyFrameworkDefaults to configure when to apply framework defaults - when cataloguing a new record, when editing a record as new (duplicating), or when changing the framework while editing an existing record. I could add a follow-up to my patch on Bug 30250 to also consider the instance of importing a record, whether we should apply framework defaults or not. I don't think we should hardcode a decision like that as libraries will have different ways of cataloguing.
(In reply to Aleisha Amohia from comment #10) > Noting that I've written a patch for Bug 30250, also a follow-up of Bug > 29146, which adds a system preference ApplyFrameworkDefaults to configure > when to apply framework defaults - when cataloguing a new record, when > editing a record as new (duplicating), or when changing the framework while > editing an existing record. > > I could add a follow-up to my patch on Bug 30250 to also consider the > instance of importing a record, whether we should apply framework defaults > or not. > > I don't think we should hardcode a decision like that as libraries will have > different ways of cataloguing. This sounds like the best option - allow users to configure, always difficult when we change behavior that is "wrong"
(In reply to Aleisha Amohia from comment #10) > I could add a follow-up to my patch on Bug 30250 to also consider the > instance of importing a record, whether we should apply framework defaults > or not. Okay. Please do so. I will move this report to BLOCKED for the time being.
Not sure, should we work on this together with bug 30250 or abandon this one in favor of the other bug?