Bug 29334

Summary: Do not apply framework defaultvalue to existing authority records
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: MARC Authority data supportAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: david, kyle, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29146
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This fixes an issue where the default value for a field in a framework was being applied when records were edited, rather than only when first created.
Version(s) released in:
21.11.00
Attachments: Bug 29334: Do not apply defaultvalue to existing auth record
Bug 29334: Do not apply defaultvalue to existing auth record
Bug 29334: Do not apply defaultvalue to existing auth record

Description Marcel de Rooy 2021-10-27 12:12:56 UTC
Counterpart of bug 29146
Comment 1 Marcel de Rooy 2021-10-28 09:46:37 UTC
Created attachment 127035 [details] [review]
Bug 29334: Do not apply defaultvalue to existing auth record

Note: The check in build_authorized_values_list with defaultvalue can be
simplified since $value may already contain the defaultvalue. It is filled
before the sub is called.
The test for inserting a blank option is generalized; and may be still
improved when using required attributes in the template? Note that we
might experience some browser issues for these placeholders.

Removed a few useless lines with respect to fields controlled
by itemtypes. The last assignment to $value was useless and
its probable intention unwanted (just use defaultvalue).

Test plan:
Put a defaultvalue in an authority framework.
Add a new authority. Check for that value.
Open existing authority. Clear field, save and reopen. Check if
field is still empty.
Bonus: test field controlled by authorised values.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 David Nind 2021-10-28 11:40:01 UTC
Created attachment 127044 [details] [review]
Bug 29334: Do not apply defaultvalue to existing auth record

Note: The check in build_authorized_values_list with defaultvalue can be
simplified since $value may already contain the defaultvalue. It is filled
before the sub is called.
The test for inserting a blank option is generalized; and may be still
improved when using required attributes in the template? Note that we
might experience some browser issues for these placeholders.

Removed a few useless lines with respect to fields controlled
by itemtypes. The last assignment to $value was useless and
its probable intention unwanted (just use defaultvalue).

Test plan:
Put a defaultvalue in an authority framework.
Add a new authority. Check for that value.
Open existing authority. Clear field, save and reopen. Check if
field is still empty.
Bonus: test field controlled by authorised values.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2021-10-28 11:41:28 UTC
Note: tested using authorised values and everything worked as expected (after the patch was applied)
Comment 4 Martin Renvoize 2021-11-12 11:23:00 UTC
Created attachment 127581 [details] [review]
Bug 29334: Do not apply defaultvalue to existing auth record

Note: The check in build_authorized_values_list with defaultvalue can be
simplified since $value may already contain the defaultvalue. It is filled
before the sub is called.
The test for inserting a blank option is generalized; and may be still
improved when using required attributes in the template? Note that we
might experience some browser issues for these placeholders.

Removed a few useless lines with respect to fields controlled
by itemtypes. The last assignment to $value was useless and
its probable intention unwanted (just use defaultvalue).

Test plan:
Put a defaultvalue in an authority framework.
Add a new authority. Check for that value.
Open existing authority. Clear field, save and reopen. Check if
field is still empty.
Bonus: test field controlled by authorised values.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2021-11-12 11:23:32 UTC
All working as expected, no regressions found and the QA scripts are happy.

Passing QA
Comment 6 Jonathan Druart 2021-11-15 11:40:16 UTC
Pushed to master for 21.11, thanks to everybody involved!