Bug 29334 - Do not apply framework defaultvalue to existing authority records
Summary: Do not apply framework defaultvalue to existing authority records
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: MARC Authority data support (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-10-27 12:12 UTC by Marcel de Rooy
Modified: 2021-11-19 16:30 UTC (History)
3 users (show)

See Also:
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 (4.81 KB, patch)
2021-10-28 09:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29334: Do not apply defaultvalue to existing auth record (4.82 KB, patch)
2021-10-28 11:40 UTC, David Nind
Details | Diff | Splinter Review
Bug 29334: Do not apply defaultvalue to existing auth record (4.88 KB, patch)
2021-11-12 11:23 UTC, Martin Renvoize
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-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!