Bug 29592 - Imported records should not be completed with defaultvalues
Summary: Imported records should not be completed with defaultvalues
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Nick Clemens
URL:
Keywords:
Depends on: 29146
Blocks:
  Show dependency treegraph
 
Reported: 2021-11-29 14:54 UTC by Marcel de Rooy
Modified: 2023-06-08 22:26 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 29592: Do not apply defaultvalue to imported record (1.35 KB, patch)
2021-11-29 15:06 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29592: Counterpart for authorities (1.16 KB, patch)
2021-11-29 15:13 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29592: Do not apply defaultvalue to imported record (1.37 KB, patch)
2022-02-26 22:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 29592: Counterpart for authorities (1.18 KB, patch)
2022-02-26 22:36 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2021-11-29 14:54:01 UTC
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?
Comment 1 Marcel de Rooy 2021-11-29 15:06:47 UTC
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>
Comment 2 Marcel de Rooy 2021-11-29 15:13:38 UTC
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>
Comment 3 Marcel de Rooy 2021-11-30 12:21:47 UTC
Martin, Nick: What do you think?
Comment 4 Marcel de Rooy 2021-12-02 10:24:38 UTC
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.
Comment 5 Martin Renvoize 2021-12-06 09:05:24 UTC
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.
Comment 6 Katrin Fischer 2022-02-26 22:36:15 UTC
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>
Comment 7 Katrin Fischer 2022-02-26 22:36:19 UTC
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>
Comment 8 Marcel de Rooy 2022-02-28 07:11:23 UTC
(In reply to Katrin Fischer from comment #6)
> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Great. Thanks.
Comment 9 Jonathan Druart 2022-03-21 12:56:13 UTC
Seems correct to me but it would be great to have Nick's opinion on this one.
Comment 10 Aleisha Amohia 2022-03-21 20:00:09 UTC
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.
Comment 11 Nick Clemens 2022-03-22 11:01:40 UTC
(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"
Comment 12 Marcel de Rooy 2022-03-22 12:07:21 UTC
(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.
Comment 13 Katrin Fischer 2022-08-07 12:06:41 UTC
Not sure, should we work on this together with bug 30250 or abandon this one in favor of the other bug?