Bug 19361 - Linking an authorised value category to a field in a framework can lose data
Summary: Linking an authorised value category to a field in a framework can lose data
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low major with 5 votes (vote)
Assignee: Nick Clemens
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks: 32748
  Show dependency treegraph
 
Reported: 2017-09-24 22:54 UTC by Liz Rea
Modified: 2023-12-28 20:47 UTC (History)
12 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:
23.05.00,22.11.04


Attachments
Bug 19361 - Display the value of a MARC field in dropdown box after it is linked to an authorised value (4.30 KB, patch)
2017-09-26 21:43 UTC, Alex Buckley
Details | Diff | Splinter Review
screenshot of MARC during step 10 (27.58 KB, image/png)
2017-10-05 18:35 UTC, Dominic Pichette
Details
Screenshot of success! (30.59 KB, image/png)
2017-10-05 20:42 UTC, Caroline Cyr La Rose
Details
Bug 19361 - Display the value of a MARC field in dropdown box after it is linked to an authorised value (4.37 KB, patch)
2017-10-05 20:49 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
Bug 19361 - Followup altered wording of the default value (1.35 KB, patch)
2017-10-05 21:12 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 19361 - Followup altered wording of the default value (1.42 KB, patch)
2017-10-11 13:05 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
[ALTERNATIVE-PATCH] Bug 19361: Add a tooltip warning if value not in AV list (6.81 KB, patch)
2020-06-12 16:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19361: Display unauthorized values when cataloging and add a warning (6.22 KB, patch)
2022-10-19 14:38 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19361: Fix advanced cataloging editor dropdowns (2.39 KB, patch)
2022-10-19 14:38 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19361: Display unauthorized values when cataloging and add a warning (6.28 KB, patch)
2022-10-19 19:47 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 19361: Fix advanced cataloging editor dropdowns (2.45 KB, patch)
2022-10-19 19:47 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 19361: Display unauthorized values when cataloging and add a warning (6.34 KB, patch)
2023-03-17 17:48 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19361: Fix advanced cataloging editor dropdowns (2.51 KB, patch)
2023-03-17 17:48 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19361: (QA follow-up) Make sure translatable string is wrapped in double quotes (1.60 KB, patch)
2023-03-17 17:48 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19361: (QA follow-up) Make sure "Not an authorised value" appears in the pull down (5.41 KB, patch)
2023-03-17 17:48 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 Liz Rea 2017-09-24 22:54:48 UTC
If you link an authorised value to a framework after you have done data entry on the same field, editing the record with the new authorised value drop down, and not filling it in will lose the data in the field.

To reproduce:

choose a free entry MARC field and add a record with some data in that field.

create an authorised value category and link it to that field in a framework

edit your record with the authorised value link now defined, do not select an authorised value from the field, and save it

note that the data for that field is now gone.

I have only heard this reported on 16.05, but it very well may be in Master. It would pay to check.
Comment 1 Alex Buckley 2017-09-26 21:43:21 UTC
Created attachment 67385 [details] [review]
Bug 19361 - Display the value of a MARC field in dropdown box after it is linked to an authorised value

This patch displays the previously entered input of a MARC subfield
(inputted before an authorised value was linked to the MARC field) in the new authorised
value dropdown so that it can be resubmitted and not lost from the
database.

Test plan:
1. Create a record and write a value into the Edition 942$e MARC field
and submit form

2. Create a new authorised value category and link it to the same
Edition MARC subfield

3. Load the MARC record again and notice that the Edition field is now
an empty dropdown box. Also for future reference in this test note the values in the 942$2 and 942$c dropdown
boxes

4. Query the biblio_metadata table with the biblionumber of the
record that you created in step 1 and notice that the value you entered
in step 1 into the Edition (<subfield_code="e") still exists

5. Submit the form

6. Repeat step 4 and notice that <subfield_code="e" line no longer exists
as the existing data for the edition MARC subfield has been lost by
linking a new authorised value category to the edition subfield

7. Apply patch

8. Repeat steps 1 but this time write a value into the Classification
part MARC subfield

9. Repeat step 2 but link the new authorised value category to the
Classification part MARC subfield

10. Repeat step 3 and notice that the value you wrote into the
Classification part subfield is displayed in the dropdown box (selected by default), along with a message in
brackets informing you that this value is not an authorised value

11. Note that the other dropdown boxes 942$2 and 942$c both display the
same values as in step 3

12. Submit the form

13. Query the biblio_metadata table and notice that the
<subfield_code="h" line still exists with the value you entered  and
that was shown in the dropdown box

As this is quite a complex test plan, I have done my best to keep it as
simple as possible, however if you are confused please let me
know and I am more than happy to clarify

Sponsored-By: Catalyst IT
Comment 2 Dominic Pichette 2017-10-05 18:35:18 UTC
Created attachment 67652 [details]
screenshot of MARC during step 10
Comment 3 Dominic Pichette 2017-10-05 18:38:30 UTC
I just attached a screen shot of the result I get at step 10. It doesn't seem to have changed since I can see what i first wrote in the field inside the dropdown. Also, (<subfield_code="e") line is absent from the database after submitting the form...
Comment 4 Dominic Pichette 2017-10-05 18:39:32 UTC
meant to write "I can't see what I first wrote in the field..."
Comment 5 Caroline Cyr La Rose 2017-10-05 20:42:05 UTC
Created attachment 67655 [details]
Screenshot of success!
Comment 6 Caroline Cyr La Rose 2017-10-05 20:47:33 UTC
Hi Alex,

I tested your patch both in 16.05 ans in the master.

It didn't work in 16.05. I didn't see the old value as the default in the drop down menu. I think it has to do with the db structure that is different. In 16.05 there is no biblio_metadata. The MARC information is stored in the biblioitems table in the marc field.

In the master, everything work as you described it. I attached a screenshot of the  field with the value followed by the mention that it is not authorized. However, I would change the phrasing to "This is a previously entered..." instead of "previous".
Comment 7 Caroline Cyr La Rose 2017-10-05 20:49:54 UTC
Created attachment 67657 [details] [review]
Bug 19361 - Display the value of a MARC field in dropdown box after it is linked to an authorised value

This patch displays the previously entered input of a MARC subfield
(inputted before an authorised value was linked to the MARC field) in the new authorised
value dropdown so that it can be resubmitted and not lost from the
database.

Test plan:
1. Create a record and write a value into the Edition 942$e MARC field
and submit form

2. Create a new authorised value category and link it to the same
Edition MARC subfield

3. Load the MARC record again and notice that the Edition field is now
an empty dropdown box. Also for future reference in this test note the values in the 942$2 and 942$c dropdown
boxes

4. Query the biblio_metadata table with the biblionumber of the
record that you created in step 1 and notice that the value you entered
in step 1 into the Edition (<subfield_code="e") still exists

5. Submit the form

6. Repeat step 4 and notice that <subfield_code="e" line no longer exists
as the existing data for the edition MARC subfield has been lost by
linking a new authorised value category to the edition subfield

7. Apply patch

8. Repeat steps 1 but this time write a value into the Classification
part MARC subfield

9. Repeat step 2 but link the new authorised value category to the
Classification part MARC subfield

10. Repeat step 3 and notice that the value you wrote into the
Classification part subfield is displayed in the dropdown box (selected by default), along with a message in
brackets informing you that this value is not an authorised value

11. Note that the other dropdown boxes 942$2 and 942$c both display the
same values as in step 3

12. Submit the form

13. Query the biblio_metadata table and notice that the
<subfield_code="h" line still exists with the value you entered  and
that was shown in the dropdown box

As this is quite a complex test plan, I have done my best to keep it as
simple as possible, however if you are confused please let me
know and I am more than happy to clarify

Sponsored-By: Catalyst IT
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 8 Alex Buckley 2017-10-05 21:12:27 UTC
Thanks Caroline

I am attaching a amendment with the new wording now
Comment 9 Alex Buckley 2017-10-05 21:12:49 UTC
Created attachment 67658 [details] [review]
Bug 19361 - Followup altered wording of the default value

Sponsored-By: Catalyst IT
Comment 10 Caroline Cyr La Rose 2017-10-11 13:04:22 UTC
It's ok now!
Comment 11 Caroline Cyr La Rose 2017-10-11 13:05:17 UTC
Created attachment 67955 [details] [review]
Bug 19361 - Followup altered wording of the default value

Sponsored-By: Catalyst IT
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 12 Marcel de Rooy 2017-10-13 09:53:26 UTC
QA: Looking here now
Comment 13 Marcel de Rooy 2017-10-13 10:09:23 UTC
QA Comment:
The problem described in the first comment does not really seem a problem to me. This is just what authorized values mean. The value is controlled now; and you should not be able to save another value. Note that if you change it here, why not change it in the items form or even other places?
I think this exception needs more discussion/consensus. Maybe raise it on the dev list or a dev meeting. And what do librarians want here?
Furthermore I would not hardcode the [% ELSIF mv.default != 'ddc' %] exception.

Same thing is true when you delete a value from the auth values while still having the value in your data. Should Koha warn you for that or not allow it ?
Comment 14 Alex Buckley 2020-04-22 01:45:24 UTC
Looking over this bug report again.
Comment 15 David Cook 2020-05-18 05:25:08 UTC
(In reply to Marcel de Rooy from comment #13)
> QA Comment:
> The problem described in the first comment does not really seem a problem to
> me. This is just what authorized values mean. The value is controlled now;
> and you should not be able to save another value. Note that if you change it
> here, why not change it in the items form or even other places?
> I think this exception needs more discussion/consensus. Maybe raise it on
> the dev list or a dev meeting. 

Hmm, I figure if it's a controlled field, you shouldn't be able to save a non-controlled value, but I think the behaviour right now might be to silently delete the existing data.

Flagging that the current data isn't authorized and that they must use an authorized value is probably the optimal path forward?
Comment 16 Alex Buckley 2020-05-19 08:58:03 UTC
Many thanks David for your thoughts, much appreciated!

I think you're idea of flagging that the current data is not authorised and prompt to choose another authorised value is an excellent compromise so at least the library staff member is made aware and the data is not just silently lost.
Comment 17 Alex Buckley 2020-05-19 08:59:43 UTC
Any other/opposing thoughts anyone else?
Comment 18 Katrin Fischer 2020-05-20 06:30:00 UTC
Flagging sounds like a good idea and it should not let you save until you have resolved the issues.

Sometimes it would be nice if we could allow a mix of authorised values and free text entry, like we allow for the suggestion reason in the staff interface. You can either choose from a list or do a free text entry picking "other reason". But that would definitely be a new feature and separate bug. And not sure if "Select2" supports that.
Comment 19 Nicolas Legrand 2020-05-20 09:17:34 UTC
(In reply to Alex Buckley from comment #17)
> Any other/opposing thoughts anyone else?

A gut yor Alex,

I've asked people at Bulac library, though we don't catalogue with Koha, this answer is more about modifying items. My colleagues feel it's safer to keep the old value by default and let the librarian decide what to do.

So your work is seen as a better behaviour!
Comment 20 Alex Buckley 2020-05-26 04:08:38 UTC
Thanks very much for sharing your thoughts Katrin and Nicolas, much appreciated!

It sounds like the consensus is flagging that the data will be lost is the way to go. 
But that a nice future enhancement would be to allow librarians to choose to allow free text values in a linked field. 

I'll work on adding flagging alert to this patchset.
Comment 21 Jonathan Druart 2020-06-12 16:16:34 UTC
Created attachment 105848 [details] [review]
[ALTERNATIVE-PATCH] Bug 19361: Add a tooltip warning if value not in AV list

(This patch depends on bug 25728)

When cataloguing it may happen that a previous value of a subfield is
not longer in the list of AVs.

Prior to this patch the value was removed.

This patch suggests to display a tooltip next to the problematic
subfield. If nothing is done, the value is kept.

This patch work when cataloguing a bibliographic record, and
adding/editing items.

It could be implemented easily for authority cataloguing.
Comment 22 Jonathan Druart 2020-06-12 16:19:38 UTC
(In reply to Jonathan Druart from comment #21)
> Created attachment 105848 [details] [review] [review]
> [ALTERNATIVE-PATCH] Bug 19361: Add a tooltip warning if value not in AV list
> 
> (This patch depends on bug 25728)
> 
> When cataloguing it may happen that a previous value of a subfield is
> not longer in the list of AVs.
> 
> Prior to this patch the value was removed.
> 
> This patch suggests to display a tooltip next to the problematic
> subfield. If nothing is done, the value is kept.
> 
> This patch work when cataloguing a bibliographic record, and
> adding/editing items.
> 
> It could be implemented easily for authority cataloguing.

This is my try: https://snipboard.io/Eso74Y.jpg
Comment 23 Nick Clemens 2022-10-19 14:38:07 UTC
Created attachment 142158 [details] [review]
Bug 19361: Display unauthorized values when cataloging and add a warning

This patch adds a warning and tool tip when an unauthorised vlaue is encountered during item
or biblio editing, and adds the value to the list to allow preserving the value (in case the
librarian is only editing another part of the record, or there is some reason to retain)

To test:
1 - Edit a record in advanced cataloging editor
2 - Under settings click 'Show fields verbatim'
3 - Edit 942$c to 'HAM' or some other invalid itemtype
4 - Save record
5 - sudo koha-mysql kohadev
    UPDATE items SET location = 'HAM' WHERE biblionumber = {biblionumber from above}
6 - Edit the biblio and items, confirm the HAM value shows in dropdowns
Comment 24 Nick Clemens 2022-10-19 14:38:11 UTC
Created attachment 142159 [details] [review]
Bug 19361: Fix advanced cataloging editor dropdowns
Comment 25 Nick Clemens 2022-10-19 14:38:54 UTC
Trying to revive this - feel free to take back if you want Alex
Comment 26 Andrew Fuerste-Henry 2022-10-19 19:47:52 UTC
Created attachment 142208 [details] [review]
Bug 19361: Display unauthorized values when cataloging and add a warning

This patch adds a warning and tool tip when an unauthorised vlaue is encountered during item
or biblio editing, and adds the value to the list to allow preserving the value (in case the
librarian is only editing another part of the record, or there is some reason to retain)

To test:
1 - Edit a record in advanced cataloging editor
2 - Under settings click 'Show fields verbatim'
3 - Edit 942$c to 'HAM' or some other invalid itemtype
4 - Save record
5 - sudo koha-mysql kohadev
    UPDATE items SET location = 'HAM' WHERE biblionumber = {biblionumber from above}
6 - Edit the biblio and items, confirm the HAM value shows in dropdowns

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 27 Andrew Fuerste-Henry 2022-10-19 19:47:57 UTC
Created attachment 142209 [details] [review]
Bug 19361: Fix advanced cataloging editor dropdowns

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Comment 28 Marcel de Rooy 2023-01-27 09:16:04 UTC
 FAIL   koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc
   OK     filters
   FAIL   forbidden patterns
                forbidden pattern: simple-quote string (line 125)
Comment 29 Katrin Fischer 2023-03-17 16:12:53 UTC
Having a look here - I'll fix the QA script in a follow-up patch
Comment 30 Katrin Fischer 2023-03-17 17:48:12 UTC
Created attachment 148377 [details] [review]
Bug 19361: Display unauthorized values when cataloging and add a warning

This patch adds a warning and tool tip when an unauthorised vlaue is encountered during item
or biblio editing, and adds the value to the list to allow preserving the value (in case the
librarian is only editing another part of the record, or there is some reason to retain)

To test:
1 - Edit a record in advanced cataloging editor
2 - Under settings click 'Show fields verbatim'
3 - Edit 942$c to 'HAM' or some other invalid itemtype
4 - Save record
5 - sudo koha-mysql kohadev
    UPDATE items SET location = 'HAM' WHERE biblionumber = {biblionumber from above}
6 - Edit the biblio and items, confirm the HAM value shows in dropdowns

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 31 Katrin Fischer 2023-03-17 17:48:15 UTC
Created attachment 148378 [details] [review]
Bug 19361: Fix advanced cataloging editor dropdowns

Signed-off-by: Andrew Fuerste-Henry <andrewfh@dubcolib.org>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 32 Katrin Fischer 2023-03-17 17:48:19 UTC
Created attachment 148379 [details] [review]
Bug 19361: (QA follow-up) Make sure translatable string is wrapped in double quotes

This fixes the QA script complaint:

 FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/cateditor-ui.inc
   FAIL	  forbidden patterns
		forbidden pattern: simple-quote string (line 125)

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 33 Katrin Fischer 2023-03-17 17:48:22 UTC
Created attachment 148380 [details] [review]
Bug 19361: (QA follow-up) Make sure "Not an authorised value" appears in the pull down

This is to make sure that the small icon with the additional tooltip
style explanation is not as easily missed.

Also updates text slightly:
* Pull downs: "X (Not an authorised value)"
* Tooltip: "The current value X is not configured for the authorised value category controlling this subfield"

Switches from exclamation triangle to the more "alerting" exclamation-triangle :)

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 34 Tomás Cohen Arazi 2023-03-20 12:43:04 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 35 Matt Blenkinsop 2023-03-21 16:41:46 UTC
Nice work everyone!

Pushed to stable for 22.11.x