Bug 17216 - Add a new table to store authorized value categories
Summary: Add a new table to store authorized value categories
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 17357 17358
Blocks: 5670 15799 17248 17355 17538 17540 17676 18097 18248 21144 21730
  Show dependency treegraph
 
Reported: 2016-08-29 13:21 UTC by Jonathan Druart
Modified: 2018-10-31 20:38 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This patch set adds a new table authorised_value_categories to store authori(s|z)ed value categories into a separate table. The problematic is explained on bug 15799 comment 4: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15799#c4
Version(s) released in:


Attachments
Bug 17216: Add Koha::AuthorisedValueCategory|ies classes (3.14 KB, patch)
2016-08-29 14:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add new table authorised_value_categories and populate it (5.68 KB, patch)
2016-08-29 14:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add some other AV categories (1.15 KB, patch)
2016-08-29 14:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: DBIC Schema changes (4.74 KB, patch)
2016-08-29 14:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Update the admin interface (7.44 KB, patch)
2016-08-29 14:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add FK on mss.authorised_value (2.47 KB, patch)
2016-08-29 14:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Use Koha::AVC from mss.pl (2.42 KB, patch)
2016-08-29 14:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add Koha::AuthorisedValueCategory|ies classes (3.14 KB, patch)
2016-08-29 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add new table authorised_value_categories and populate it (5.71 KB, patch)
2016-08-29 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add some other AV categories (1.77 KB, patch)
2016-08-29 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: DBIC Schema changes (4.74 KB, patch)
2016-08-29 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Update the admin interface (7.44 KB, patch)
2016-08-29 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add FK on mss.authorised_value (2.38 KB, patch)
2016-08-29 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Use Koha::AVC from mss.pl (2.42 KB, patch)
2016-08-29 16:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add FK on mss.authorised_value (2.38 KB, patch)
2016-08-30 09:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Use Koha::AVC from mss.pl (2.42 KB, patch)
2016-08-30 09:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Category name is required when adding a new AV cat (1.79 KB, patch)
2016-08-30 09:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add FK on authorised_values.category (2.00 KB, patch)
2016-08-30 10:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Update DBIC Schema (3.56 KB, patch)
2016-08-30 10:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Update DBIC schema for MSS (3.19 KB, patch)
2016-08-30 11:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Fix tests (1.08 KB, patch)
2016-08-30 11:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17216: Add Koha::AuthorisedValueCategory|ies classes (3.25 KB, patch)
2016-08-30 14:36 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17216: Add new table authorised_value_categories and populate it (5.81 KB, patch)
2016-08-30 14:36 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17216: Add some other AV categories (1.89 KB, patch)
2016-08-30 14:36 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17216: DBIC Schema changes (4.85 KB, patch)
2016-08-30 14:36 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17216: Update the admin interface (7.54 KB, patch)
2016-08-30 14:37 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17216: Add FK on mss.authorised_value (2.48 KB, patch)
2016-08-30 14:37 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17216: Use Koha::AVC from mss.pl (2.52 KB, patch)
2016-08-30 14:37 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17216: Category name is required when adding a new AV cat (1.89 KB, patch)
2016-08-30 14:37 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17216: Add FK on authorised_values.category (2.10 KB, patch)
2016-08-30 14:37 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17216: Update DBIC Schema (3.66 KB, patch)
2016-08-30 14:37 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17216: Update DBIC schema for MSS (3.29 KB, patch)
2016-08-30 14:37 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17216: Fix tests (1.19 KB, patch)
2016-08-30 14:37 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 17216: Fix other tests (1.98 KB, patch)
2016-08-30 16:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add Koha::AuthorisedValueCategory|ies classes (3.19 KB, patch)
2016-09-06 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add new table authorised_value_categories and populate it (5.76 KB, patch)
2016-09-06 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add some other AV categories (1.84 KB, patch)
2016-09-06 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: DBIC Schema changes (4.79 KB, patch)
2016-09-06 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Update the admin interface (7.67 KB, patch)
2016-09-06 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add FK on mss.authorised_value (2.43 KB, patch)
2016-09-06 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Use Koha::AVC from mss.pl (2.46 KB, patch)
2016-09-06 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add FK on authorised_values.category (2.05 KB, patch)
2016-09-06 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Update DBIC Schema (3.61 KB, patch)
2016-09-06 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Update DBIC schema for MSS (3.24 KB, patch)
2016-09-06 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Fix tests (1.13 KB, patch)
2016-09-06 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Fix other tests (1.98 KB, patch)
2016-09-06 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add Koha::AuthorisedValueCategory|ies classes (3.19 KB, patch)
2016-09-08 17:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add new table authorised_value_categories and populate it (5.76 KB, patch)
2016-09-08 17:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add some other AV categories (1.84 KB, patch)
2016-09-08 17:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: DBIC Schema changes (4.79 KB, patch)
2016-09-08 17:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Update the admin interface (7.66 KB, patch)
2016-09-08 17:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add FK on mss.authorised_value (2.43 KB, patch)
2016-09-08 17:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Use Koha::AVC from mss.pl (2.46 KB, patch)
2016-09-08 17:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Add FK on authorised_values.category (2.05 KB, patch)
2016-09-08 17:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Update DBIC Schema (3.61 KB, patch)
2016-09-08 17:43 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Update DBIC schema for MSS (3.24 KB, patch)
2016-09-08 17:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Fix tests (1.13 KB, patch)
2016-09-08 17:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Fix other tests (1.98 KB, patch)
2016-09-08 17:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Do not display branches, itemtypes and cn_source (1004 bytes, patch)
2016-09-19 11:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Do no display the empty string category name (1.05 KB, patch)
2016-09-19 11:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Internal AV should be correctly handled when creating a new category (1.54 KB, patch)
2016-09-27 08:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Move the AV cat list file to the top level (4.86 KB, patch)
2016-09-27 12:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Add Koha::AuthorisedValueCategory|ies classes (3.26 KB, patch)
2016-10-11 15:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Add new table authorised_value_categories and populate it (5.83 KB, patch)
2016-10-11 15:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Add some other AV categories (1.91 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: DBIC Schema changes (4.86 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Update the admin interface (7.72 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Add FK on mss.authorised_value (2.50 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Use Koha::AVC from mss.pl (2.53 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Add FK on authorised_values.category (2.12 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Update DBIC Schema (3.68 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Update DBIC schema for MSS (3.30 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Fix tests (1.20 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Fix other tests (2.05 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Do not display branches, itemtypes and cn_source (1.05 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Do no display the empty string category name (1.12 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Internal AV should be correctly handled when creating a new category (1.60 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 17216: Move the AV cat list file to the top level (4.93 KB, patch)
2016-10-11 15:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 17216: Revert changes to ItemsSearchField schema file (1.49 KB, patch)
2016-10-18 08:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17216: Fix SQL statement to setting marc_subfield_structure.authorised_value to NULL (1017 bytes, patch)
2016-11-23 15:29 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2016-08-29 13:21:46 UTC
The way we manage authorized values is not consistent: Some are present in the DB, others are not (see bug 15799 comment 4 for more details).
Worse, most of the time the FK does not exist (and datatype is not consistent: varchar 32 vs 20, 16,...).

To help cleaning up this area, a new table to store authorized value categories would be helpful.
Comment 1 Jonathan Druart 2016-08-29 14:42:31 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2016-08-29 14:42:34 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-08-29 14:42:39 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-08-29 14:42:42 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2016-08-29 14:42:46 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2016-08-29 14:42:49 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-08-29 14:42:52 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2016-08-29 16:19:31 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2016-08-29 16:19:34 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2016-08-29 16:19:38 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2016-08-29 16:19:42 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2016-08-29 16:19:46 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2016-08-29 16:19:50 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2016-08-29 16:19:53 UTC Comment hidden (obsolete)
Comment 15 Owen Leonard 2016-08-29 16:57:42 UTC
- bug_17216_2_add_fk.sql is still missing a semicolon on the 'UPDATE marc_subfield_structure' line.
- Shouldn't category be required in the "New category" form?
- In testing the effect of these patches on marc_subfield_structure.pl, what should we be looking for? Simply that the categories show up correctly?
Comment 16 Jonathan Druart 2016-08-30 09:02:58 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2016-08-30 09:03:01 UTC Comment hidden (obsolete)
Comment 18 Jonathan Druart 2016-08-30 09:03:07 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2016-08-30 09:07:03 UTC
(In reply to Owen Leonard from comment #15)
> - bug_17216_2_add_fk.sql is still missing a semicolon on the 'UPDATE
> marc_subfield_structure' line.

Oops, sorry about that.
I have tested the file directly in the CLI, line by line, that's why I did not spot it.

> - Shouldn't category be required in the "New category" form?

Yep, done.

> - In testing the effect of these patches on marc_subfield_structure.pl, what
> should we be looking for? Simply that the categories show up correctly?

Yes.
You will also think about the structure changes: now all the AV cat is displayed, when before only the AV cat *in used* were displayed.
Think also about the "branches", "itemtypes" and "cn_source". does that make sense to have them in the table, etc.
Comment 20 Jonathan Druart 2016-08-30 10:56:30 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2016-08-30 10:56:33 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2016-08-30 11:02:30 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2016-08-30 11:09:18 UTC Comment hidden (obsolete)
Comment 24 Owen Leonard 2016-08-30 14:36:36 UTC Comment hidden (obsolete)
Comment 25 Owen Leonard 2016-08-30 14:36:44 UTC Comment hidden (obsolete)
Comment 26 Owen Leonard 2016-08-30 14:36:52 UTC Comment hidden (obsolete)
Comment 27 Owen Leonard 2016-08-30 14:36:58 UTC Comment hidden (obsolete)
Comment 28 Owen Leonard 2016-08-30 14:37:05 UTC Comment hidden (obsolete)
Comment 29 Owen Leonard 2016-08-30 14:37:10 UTC Comment hidden (obsolete)
Comment 30 Owen Leonard 2016-08-30 14:37:19 UTC Comment hidden (obsolete)
Comment 31 Owen Leonard 2016-08-30 14:37:26 UTC Comment hidden (obsolete)
Comment 32 Owen Leonard 2016-08-30 14:37:34 UTC Comment hidden (obsolete)
Comment 33 Owen Leonard 2016-08-30 14:37:40 UTC Comment hidden (obsolete)
Comment 34 Owen Leonard 2016-08-30 14:37:47 UTC Comment hidden (obsolete)
Comment 35 Owen Leonard 2016-08-30 14:37:53 UTC Comment hidden (obsolete)
Comment 36 Owen Leonard 2016-08-30 14:39:23 UTC
These patches work well for me from the point of view of an end user. The patches apply, QA tools don't complain, and database updates correctly. I have followed the test plan successfully.
Comment 37 Jonathan Druart 2016-08-30 16:05:42 UTC Comment hidden (obsolete)
Comment 38 Claire Gravely 2016-09-05 22:36:09 UTC
could not apply patch to test bug 17248. 

CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt
Failed to merge in the changes.
Comment 39 Jonathan Druart 2016-09-06 07:18:09 UTC Comment hidden (obsolete)
Comment 40 Jonathan Druart 2016-09-06 07:18:12 UTC Comment hidden (obsolete)
Comment 41 Jonathan Druart 2016-09-06 07:18:19 UTC Comment hidden (obsolete)
Comment 42 Jonathan Druart 2016-09-06 07:18:26 UTC Comment hidden (obsolete)
Comment 43 Jonathan Druart 2016-09-06 07:18:30 UTC Comment hidden (obsolete)
Comment 44 Jonathan Druart 2016-09-06 07:18:33 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2016-09-06 07:18:37 UTC Comment hidden (obsolete)
Comment 46 Jonathan Druart 2016-09-06 07:18:41 UTC Comment hidden (obsolete)
Comment 47 Jonathan Druart 2016-09-06 07:18:45 UTC Comment hidden (obsolete)
Comment 48 Jonathan Druart 2016-09-06 07:18:48 UTC Comment hidden (obsolete)
Comment 49 Jonathan Druart 2016-09-06 07:18:51 UTC Comment hidden (obsolete)
Comment 50 Jonathan Druart 2016-09-06 07:18:56 UTC Comment hidden (obsolete)
Comment 51 Jonathan Druart 2016-09-06 07:19:45 UTC
Last patch set fixes conflict with bug 11019.
Comment 52 Jonathan Druart 2016-09-08 17:43:28 UTC
Created attachment 55365 [details] [review]
Bug 17216: Add Koha::AuthorisedValueCategory|ies classes

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 53 Jonathan Druart 2016-09-08 17:43:32 UTC
Created attachment 55366 [details] [review]
Bug 17216: Add new table authorised_value_categories and populate it

This patch set adds a new table authorised_value_categories to store
authori(s|z)ed value categories into a separate table.
The problematic is explained on bug 15799 comment 4:
We need FK to the AV categories but some may not have authorized values
yet.

What does this patch set:
- Add a new authorised_value_categories table
- Populate it with known categories
- Update FK items_search_fields.authorised_values_category
- Create a new FK marc_subfield_structure.authorised_value (FIXME should
  be authorised_value_categories instead)

They are some problems this patch set do not take into account:
- The .sql installer files won't insert correctly (will have to be
  updated when this patch set will be ready to be pushed)
- All the categories (even the ones without authorized values defined)
  are listed when you edit frameworks (marc_subfield_structure.pl)
- There is no way to delete a category (TODO). But to do so it would be
  good to have a authorised_value_categories.is_internal field to mark
  some categories as "cannot be deleted".

Test plan:
0/ Execute the DB entry to create and populate the new table and set the FK
1/ Create a new AV category from the admin module (admin/authorised_values.pl)
2/ Add/edit subfield linked to a AV category
(admin/marc_subfield_structure.pl)
3/ You won't be allowed to add AV for branches, itemtypes or cn_source.
They are used internally.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 54 Jonathan Druart 2016-09-08 17:43:35 UTC
Created attachment 55367 [details] [review]
Bug 17216: Add some other AV categories

Found in
installer/data/mysql/en/marcflavour/marc21/mandatory/marc21_framework_DEFAULT.sql

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 55 Jonathan Druart 2016-09-08 17:43:39 UTC
Created attachment 55368 [details] [review]
Bug 17216: DBIC Schema changes

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 56 Jonathan Druart 2016-09-08 17:43:42 UTC
Created attachment 55369 [details] [review]
Bug 17216: Update the admin interface

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 57 Jonathan Druart 2016-09-08 17:43:45 UTC
Created attachment 55370 [details] [review]
Bug 17216: Add FK on mss.authorised_value

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 58 Jonathan Druart 2016-09-08 17:43:49 UTC
Created attachment 55371 [details] [review]
Bug 17216: Use Koha::AVC from mss.pl

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 59 Jonathan Druart 2016-09-08 17:43:53 UTC
Created attachment 55372 [details] [review]
Bug 17216: Add FK on authorised_values.category

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 60 Jonathan Druart 2016-09-08 17:43:56 UTC
Created attachment 55373 [details] [review]
Bug 17216: Update DBIC Schema

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 61 Jonathan Druart 2016-09-08 17:44:00 UTC
Created attachment 55374 [details] [review]
Bug 17216: Update DBIC schema for MSS

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 62 Jonathan Druart 2016-09-08 17:44:03 UTC
Created attachment 55375 [details] [review]
Bug 17216: Fix tests

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 63 Jonathan Druart 2016-09-08 17:44:07 UTC
Created attachment 55376 [details] [review]
Bug 17216: Fix other tests
Comment 64 Katrin Fischer 2016-09-18 14:28:24 UTC
Hi Jonathan, mostly good - please take a look:

Administration > Authorized values

1) I think the 'virtual' authorized values branches, itemtypes and cn_sort should not show up in the authorized value list in administration. It's not a nice effect that it looks like you can add to them and then get an error message - we should avoid that kind of user experience.

2) Small typo when adding to 'branches'. Should be:
The authorized value categories 'branches', 'itemtypes' and 'cn_source' are used internally by Koha and are not valid.

3) If you typo in an authorized value category, there is no way to remove it. That can get annoying pretty fast. We should offer a way to delete an empty authorized value category (maybe if not empty too?).

4) The new empty option in the pull down list leads to some confusing display:
Authorized values for category :
There are no authorized values defined for 

Editing items

5) There is a problem with the STACK on 952$j as this doesn't exist. Should we clean it up by removing it from the frameworks if there are no values defined for it?

Authority types
6) Should we add the same FK relationship there? (separate bug?)

Successfully tested:
- Bibliographic MARC frameworks
- Cataloguing
- Patron attribute types / Patrons
Comment 65 Jonathan Druart 2016-09-19 11:36:17 UTC
Created attachment 55692 [details] [review]
Bug 17216: Do not display branches, itemtypes and cn_source
Comment 66 Jonathan Druart 2016-09-19 11:36:22 UTC
Created attachment 55693 [details] [review]
Bug 17216: Do no display the empty string category name
Comment 67 Jonathan Druart 2016-09-19 11:39:35 UTC
(In reply to Katrin Fischer from comment #64)
> Hi Jonathan, mostly good - please take a look:
> 
> Administration > Authorized values
> 
> 1) I think the 'virtual' authorized values branches, itemtypes and cn_sort
> should not show up in the authorized value list in administration. It's not
> a nice effect that it looks like you can add to them and then get an error
> message - we should avoid that kind of user experience.

Done, removed.

> 2) Small typo when adding to 'branches'. Should be:
> The authorized value categories 'branches', 'itemtypes' and 'cn_source' are
> used internally by Koha and are not valid.

Done, thanks.

> 3) If you typo in an authorized value category, there is no way to remove
> it. That can get annoying pretty fast. We should offer a way to delete an
> empty authorized value category (maybe if not empty too?).

Yes, it's in the TODO section of the commit msg.

> 4) The new empty option in the pull down list leads to some confusing
> display:
> Authorized values for category :
> There are no authorized values defined for 

Fixed, the empty one is no longer displayed.

> Editing items
> 
> 5) There is a problem with the STACK on 952$j as this doesn't exist. Should
> we clean it up by removing it from the frameworks if there are no values
> defined for it?

I have not found regression with master. So let's deal with it on another bug report.

> Authority types
> 6) Should we add the same FK relationship there? (separate bug?)

Yes, and others as well. But that should be done on another bug report.
Comment 68 Katrin Fischer 2016-09-27 05:35:08 UTC
Hi Jonathan,

this is looking good, but I found something a bit disturbing (to me :) ):
Try to add an authorized value category "Branches".
The system tells you, that the category already exists.
But: It will allow you then to add a new value to it.

Can we show the "used internally message" and now allow to do that?

I will move the "delete category" to a separate bug.
Comment 69 Jonathan Druart 2016-09-27 08:31:32 UTC
Created attachment 55835 [details] [review]
Bug 17216: Internal AV should be correctly handled when creating a new category
Comment 70 Katrin Fischer 2016-09-27 08:39:25 UTC
Cool, thx Jonathan - one last thing - could you take a look at the installer files? 

You noted that before this is pushed we need to fix the files there. I think this is really close to PQA now.

Could we handle this like sysprefs.sql having one file on top level used for all? I think we got no need for translations here.
Comment 71 Jonathan Druart 2016-09-27 12:55:45 UTC
Created attachment 55840 [details] [review]
Bug 17216: Move the AV cat list file to the top level

This way it will get used for any languages.
Comment 72 Jonathan Druart 2016-09-27 12:56:51 UTC
(In reply to Katrin Fischer from comment #70)
> Cool, thx Jonathan - one last thing - could you take a look at the installer
> files? 
> 
> You noted that before this is pushed we need to fix the files there. I think
> this is really close to PQA now.
> 
> Could we handle this like sysprefs.sql having one file on top level used for
> all? I think we got no need for translations here.

Done, see the last patch.
You can use the script from bug 17360 to test it.
Comment 73 Katrin Fischer 2016-10-11 15:36:53 UTC
Created attachment 56202 [details] [review]
[PASSED QA] Bug 17216: Add Koha::AuthorisedValueCategory|ies classes

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 74 Katrin Fischer 2016-10-11 15:36:58 UTC
Created attachment 56203 [details] [review]
[PASSED QA] Bug 17216: Add new table authorised_value_categories and populate it

This patch set adds a new table authorised_value_categories to store
authori(s|z)ed value categories into a separate table.
The problematic is explained on bug 15799 comment 4:
We need FK to the AV categories but some may not have authorized values
yet.

What does this patch set:
- Add a new authorised_value_categories table
- Populate it with known categories
- Update FK items_search_fields.authorised_values_category
- Create a new FK marc_subfield_structure.authorised_value (FIXME should
  be authorised_value_categories instead)

They are some problems this patch set do not take into account:
- The .sql installer files won't insert correctly (will have to be
  updated when this patch set will be ready to be pushed)
- All the categories (even the ones without authorized values defined)
  are listed when you edit frameworks (marc_subfield_structure.pl)
- There is no way to delete a category (TODO). But to do so it would be
  good to have a authorised_value_categories.is_internal field to mark
  some categories as "cannot be deleted".

Test plan:
0/ Execute the DB entry to create and populate the new table and set the FK
1/ Create a new AV category from the admin module (admin/authorised_values.pl)
2/ Add/edit subfield linked to a AV category
(admin/marc_subfield_structure.pl)
3/ You won't be allowed to add AV for branches, itemtypes or cn_source.
They are used internally.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 75 Katrin Fischer 2016-10-11 15:37:03 UTC
Created attachment 56204 [details] [review]
[PASSED QA] Bug 17216: Add some other AV categories

Found in
installer/data/mysql/en/marcflavour/marc21/mandatory/marc21_framework_DEFAULT.sql

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 76 Katrin Fischer 2016-10-11 15:37:07 UTC
Created attachment 56205 [details] [review]
[PASSED QA] Bug 17216: DBIC Schema changes

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 77 Katrin Fischer 2016-10-11 15:37:11 UTC
Created attachment 56206 [details] [review]
[PASSED QA] Bug 17216: Update the admin interface

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 78 Katrin Fischer 2016-10-11 15:37:16 UTC
Created attachment 56207 [details] [review]
[PASSED QA] Bug 17216: Add FK on mss.authorised_value

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 79 Katrin Fischer 2016-10-11 15:37:20 UTC
Created attachment 56208 [details] [review]
[PASSED QA] Bug 17216: Use Koha::AVC from mss.pl

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 80 Katrin Fischer 2016-10-11 15:37:24 UTC
Created attachment 56209 [details] [review]
[PASSED QA] Bug 17216: Add FK on authorised_values.category

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 81 Katrin Fischer 2016-10-11 15:37:28 UTC
Created attachment 56210 [details] [review]
[PASSED QA] Bug 17216: Update DBIC Schema

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 82 Katrin Fischer 2016-10-11 15:37:32 UTC
Created attachment 56211 [details] [review]
[PASSED QA] Bug 17216: Update DBIC schema for MSS

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 83 Katrin Fischer 2016-10-11 15:37:36 UTC
Created attachment 56212 [details] [review]
[PASSED QA] Bug 17216: Fix tests

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 84 Katrin Fischer 2016-10-11 15:37:40 UTC
Created attachment 56213 [details] [review]
[PASSED QA] Bug 17216: Fix other tests

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 85 Katrin Fischer 2016-10-11 15:37:44 UTC
Created attachment 56214 [details] [review]
[PASSED QA] Bug 17216: Do not display branches, itemtypes and cn_source

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 86 Katrin Fischer 2016-10-11 15:37:48 UTC
Created attachment 56215 [details] [review]
[PASSED QA] Bug 17216: Do no display the empty string category name

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 87 Katrin Fischer 2016-10-11 15:37:52 UTC
Created attachment 56216 [details] [review]
[PASSED QA] Bug 17216: Internal AV should be correctly handled when creating a new category

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 88 Katrin Fischer 2016-10-11 15:37:56 UTC
Created attachment 56217 [details] [review]
[PASSED QA] Bug 17216: Move the AV cat list file to the top level

This way it will get used for any languages.

Signed-off-by: Katrin Fischer  <katrin.fischer@bsz-bw.de>
Comment 89 Brendan Gallagher 2016-10-13 13:32:10 UTC
Pushed to Master - Should be in the November 2016 Release.   Thanks!
Comment 90 Jonathan Druart 2016-10-18 08:55:09 UTC
Created attachment 56619 [details] [review]
Bug 17216: Revert changes to ItemsSearchField schema file
Comment 91 Jonathan Druart 2016-10-18 08:55:45 UTC
(In reply to Jonathan Druart from comment #90)
> Created attachment 56619 [details] [review] [review]
> Bug 17216: Revert changes to ItemsSearchField schema file

@RM, could you please push this follow-up please? It seems that I have generated a wrong file (80 vs 32).
Comment 92 Brendan Gallagher 2016-10-18 16:01:40 UTC
(In reply to Jonathan Druart from comment #91)
> (In reply to Jonathan Druart from comment #90)
> > Created attachment 56619 [details] [review] [review] [review]
> > Bug 17216: Revert changes to ItemsSearchField schema file
> 
> @RM, could you please push this follow-up please? It seems that I have
> generated a wrong file (80 vs 32).

Thanks!  Pushed
Comment 93 Mirko Tietgen 2016-11-23 14:44:09 UTC
With a Debian package for 16.11, on upgrade from 16.05 the following happens:

Upgrade to 16.06.00.032 done (Bug 17357 - WTHDRAWN is still used in installer files)
DBD::mysql::db do failed: Can't create table 'koha_upgrade.#sql-329_a5' (errno: 150) [for Statement "
            ALTER TABLE marc_subfield_structure
            MODIFY COLUMN authorised_value VARCHAR(32) DEFAULT NULL,
            ADD CONSTRAINT marc_subfield_structure_ibfk_1 FOREIGN KEY (authorised_value) REFERENCES authorised_value_categories (category_name) ON UPDATE CASCADE ON DELETE SET NULL;
            "] at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 13175.
Upgrade to 16.06.00.033 done (Bug 17216 - Add a new table to store authorized value categories)

I assume it is related to this bug? Is it safe to ignore? It does sound more scary than the duplicate warnings we get from time to time.
Comment 94 Mirko Tietgen 2016-11-23 14:50:22 UTC
FYI: I'll wait with the package release for 16.11.00 until this is solved. :)
Comment 95 Jonathan Druart 2016-11-23 15:29:15 UTC
Created attachment 57729 [details] [review]
Bug 17216: Fix SQL statement to setting marc_subfield_structure.authorised_value to NULL

We want to update it if it's empty, not equal to ';'
nopaste vim mode is to blame
Comment 96 Jacek Ablewicz 2016-11-23 15:45:37 UTC
(In reply to Mirko Tietgen from comment #93)
> With a Debian package for 16.11, on upgrade from 16.05 the following happens:
> 
> Upgrade to 16.06.00.032 done (Bug 17357 - WTHDRAWN is still used in
> installer files)
> DBD::mysql::db do failed: Can't create table 'koha_upgrade.#sql-329_a5'
> (errno: 150) [for Statement "
>             ALTER TABLE marc_subfield_structure
>             MODIFY COLUMN authorised_value VARCHAR(32) DEFAULT NULL,
>             ADD CONSTRAINT marc_subfield_structure_ibfk_1 FOREIGN KEY
> (authorised_value) REFERENCES authorised_value_categories (category_name) ON
> UPDATE CASCADE ON DELETE SET NULL;
>             "] at
> /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line
> 13175.
> Upgrade to 16.06.00.033 done (Bug 17216 - Add a new table to store
> authorized value categories)
> 
> I assume it is related to this bug? Is it safe to ignore? It does sound more
> scary than the duplicate warnings we get from time to time.

Got the same error in my test setup. For undetermined reason, authorised_value field in marc_subfield_structure table did not have utf8_unicode_ci collation.

I replaced

    MODIFY COLUMN authorised_value VARCHAR(32) DEFAULT NULL,

with

    MODIFY COLUMN authorised_value VARCHAR(32) CHARACTER SET utf8 COLLATE utf8_unicode_ci DEFAULT NULL,

and after that it seems to work fine. No idea why this DB update is tagged  with "Bug 17357 - WTHDRAWN is still used in installer files)" and not with a "Bug 17216 ..." instead.

BTW, there are some other text fields in my test DB with inconsistent collation too, but this is a database with a long history of accumulated DB updates (including various unstable / atomic ones).
Comment 97 Katrin Fischer 2016-11-23 16:39:17 UTC
>Got the same error in my test setup. For undetermined reason, authorised_value >field in marc_subfield_structure table did not have utf8_unicode_ci collation.

That's intentional. ci = case insensitive breaks uppercase subfield codes which are often used to create additional fields in 952.
Comment 98 Jacek Ablewicz 2016-11-23 18:06:41 UTC
(In reply to Katrin Fischer from comment #97)
> >Got the same error in my test setup. For undetermined reason, authorised_value >field in marc_subfield_structure table did not have utf8_unicode_ci collation.
> 
> That's intentional. ci = case insensitive breaks uppercase subfield codes
> which are often used to create additional fields in 952.

marc_subfield_structure.tagsubfield indeed has utf8_bin collation, but should the same rule apply for authorised_value? By default (in fresh installs), marc_subfield_structure.authorised_value has utf8_unicode_ci collation, for some reasons (I still have no idea why, maybe update from Bug 11944 partially failed) in my old test DB, this field has no collation defined, and that's why adding FK failed for me with error 150.

But it can be failing for different reason[s] for other people - and probably is, this collation issue seems to be some kind of problem which was specific to this particular DB.

There is one other table (overduerules) in that DB which does not have utf8_unicode_ci collation set by default for all text fields, but it looks more like an omission in updatedatabase (Bug 13624, DB version: 3.23.00.007).
Comment 99 Jacek Ablewicz 2016-11-23 18:08:55 UTC
(In reply to Jacek Ablewicz from comment #96)

> No idea why this DB update is tagged 
> with "Bug 17357 - WTHDRAWN is still used in installer files)" and not with a
> "Bug 17216 ..." instead.

Uhm, scratch that; in fact it is tagged correctly
Comment 100 Jacek Ablewicz 2016-11-23 19:22:51 UTC
Huh, this is what happens (mysql 5.5.49): for a table with no default collation defined on table level (like marc_subfield_structure)

   MODIFY COLUMN authorised_value VARCHAR(32) DEFAULT NULL

does not preserve existing collation for that field (which was, after all, utf8_unicode_ci in my test DB before attempting that upgrade: manually set up to utf8_unicode_ci for a bunch of marc_subfield_structure fields in DB update from Bug 11944), and it subsequently fails with error 150 on adding FK constraint because of collations inconsistency (I think.. not quite sure, but a solution from comment #96 fixes this problem).

It's reproductible; note that I have different default collation defined mysql server-side on that box (utf8_general_ci) - if it was utf8_unicode_ci server-side, I guess there is a possibility this issue does not manifest itself (?).
Comment 101 Mirko Tietgen 2016-11-23 23:12:07 UTC
Did an install with the latest patch added, got

Update errors :

    [Thu Nov 24 00:06:08 2016] updatedatabase.pl: DBD::mysql::db do failed: Duplicate column name 'updated_on' [for Statement "
    [Thu Nov 24 00:06:08 2016] updatedatabase.pl: ALTER TABLE borrowers
    [Thu Nov 24 00:06:08 2016] updatedatabase.pl: ADD COLUMN updated_on timestamp NULL DEFAULT CURRENT_TIMESTAMP
    [Thu Nov 24 00:06:08 2016] updatedatabase.pl: ON UPDATE CURRENT_TIMESTAMP
    [Thu Nov 24 00:06:08 2016] updatedatabase.pl: AFTER privacy_guarantor_checkouts;
    [Thu Nov 24 00:06:08 2016] updatedatabase.pl: "] at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 12626.
    [Thu Nov 24 00:06:08 2016] updatedatabase.pl: DBD::mysql::db do failed: Duplicate column name 'updated_on' [for Statement "
    [Thu Nov 24 00:06:08 2016] updatedatabase.pl: ALTER TABLE deletedborrowers
    [Thu Nov 24 00:06:08 2016] updatedatabase.pl: ADD COLUMN updated_on timestamp NULL DEFAULT CURRENT_TIMESTAMP
    [Thu Nov 24 00:06:08 2016] updatedatabase.pl: ON UPDATE CURRENT_TIMESTAMP
    [Thu Nov 24 00:06:08 2016] updatedatabase.pl: AFTER privacy_guarantor_checkouts;
    [Thu Nov 24 00:06:08 2016] updatedatabase.pl: "] at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 12632.
    [Thu Nov 24 00:06:15 2016] updatedatabase.pl: DBD::mysql::db do failed: Can't create table 'koha_koha.#sql-2221_18d' (errno: 150) [for Statement "
    [Thu Nov 24 00:06:15 2016] updatedatabase.pl: ALTER TABLE marc_subfield_structure
    [Thu Nov 24 00:06:15 2016] updatedatabase.pl: MODIFY COLUMN authorised_value VARCHAR(32) DEFAULT NULL,
    [Thu Nov 24 00:06:15 2016] updatedatabase.pl: ADD CONSTRAINT marc_subfield_structure_ibfk_1 FOREIGN KEY (authorised_value) REFERENCES authorised_value_categories (category_name) ON UPDATE CASCADE ON DELETE SET NULL;
    [Thu Nov 24 00:06:15 2016] updatedatabase.pl: "] at /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line 13175.


@Jacek: that is what you are talking about, right?
Comment 102 Jacek Ablewicz 2016-11-24 08:33:55 UTC
(In reply to Mirko Tietgen from comment #101)

>     [Thu Nov 24 00:06:08 2016] updatedatabase.pl: "] at
> /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line
> 12632.
>     [Thu Nov 24 00:06:15 2016] updatedatabase.pl: DBD::mysql::db do failed:
> Can't create table 'koha_koha.#sql-2221_18d' (errno: 150) [for Statement "
>     [Thu Nov 24 00:06:15 2016] updatedatabase.pl: ALTER TABLE
> marc_subfield_structure
>     [Thu Nov 24 00:06:15 2016] updatedatabase.pl: MODIFY COLUMN
> authorised_value VARCHAR(32) DEFAULT NULL,
>     [Thu Nov 24 00:06:15 2016] updatedatabase.pl: ADD CONSTRAINT
> marc_subfield_structure_ibfk_1 FOREIGN KEY (authorised_value) REFERENCES
> authorised_value_categories (category_name) ON UPDATE CASCADE ON DELETE SET
> NULL;
>     [Thu Nov 24 00:06:15 2016] updatedatabase.pl: "] at
> /usr/share/koha/intranet/cgi-bin/installer/data/mysql/updatedatabase.pl line
> 13175.
> 
> 
> @Jacek: that is what you are talking about, right?

Possibly ;), hard to tell (errno: 150 by itself is not very informative).
What I did while trying to troubleshoot this problem (following the guidelines from <http://stackoverflow.com/questions/4061293/mysql-cant-create-table-errno-150>, especially nr 4 is quite helpful):

- split erroneous statement in two

    1) ALTER TABLE marc_subfield_structure
    MODIFY COLUMN authorised_value VARCHAR(32) DEFAULT NULL;

    2) ALTER TABLE marc_subfield_structure
ADD CONSTRAINT marc_subfield_structure_ibfk_1 FOREIGN KEY (authorised_value) REFERENCES authorised_value_categories (category_name) ON UPDATE CASCADE ON DELETE SET NULL;

- part 1) executed OK, but part 2) still resulted in the same error (150)

- noticed that after 1), utf8_unicode_ci collation is no longer there for authorised_value field (mysqldump --no-data marc_subfield_structure)

- replaced 1) with

    ALTER TABLE marc_subfield_structure
    MODIFY COLUMN authorised_value VARCHAR(32) CHARACTER SET utf8 COLLATE utf8_unicode_ci DEFAULT NULL;

- re-run 1) and 2) - no error, FK constrain got created successfully and collation for authorised_value is utf8_unicode_ci.
Comment 103 Jonathan Druart 2016-11-24 11:37:38 UTC
All of this sounds related to bug 13810
Comment 104 Jonathan Druart 2016-11-24 13:44:24 UTC
I am going to provide a patch on bug 17676
Comment 105 Jonathan Druart 2017-02-24 13:59:14 UTC
(In reply to Jonathan Druart from comment #95)
> Created attachment 57729 [details] [review] [review]
> Bug 17216: Fix SQL statement to setting
> marc_subfield_structure.authorised_value to NULL
> 
> We want to update it if it's empty, not equal to ';'
> nopaste vim mode is to blame

This is not in, we absolutely need it!
Comment 106 Katrin Fischer 2017-03-12 22:00:24 UTC
This changed an existing database entry - what about the installations that have already updated to 16.11.x?
Comment 107 Katrin Fischer 2017-03-12 22:00:59 UTC
Please use bug 18248 for follow-ups!