Bug 20280 - Authorised value categories list in bibliographic frameworks has 2 empty entries
Summary: Authorised value categories list in bibliographic frameworks has 2 empty entries
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Target Milestone: ---
Assignee: Bugs List
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-23 07:39 UTC by Katrin Fischer
Modified: 2019-05-04 16:10 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2018-02-23 07:39:01 UTC
To test:
- Edit a subfield in any of your bibliographic frameworks
- In the last section, take a look at the authorised value categories list
- Confirm there are 2 empty entries

Tested with sample data.
Comment 1 Jonathan Druart 2018-03-01 15:16:09 UTC
installer/data/mysql/mandatory/auth_val_cat.sql:
  1 INSERT IGNORE INTO authorised_value_categories( category_name )
  2     VALUES
  3     (''),

but installer/data/mysql/updatedatabase.pl:
13164     INSERT IGNORE INTO authorised_value_categories( category_name )
13165     VALUES
13166     ('Asort1'),

"" is missing


Moreover:
  commit 62aa3f4292d83bc9304e296de5f0830e28510492
    Bug 17216: Use Koha::AVC from mss.pl
Added this additional blank entry in the template:
+                                <option value=""></option>

Additional info, in admin/authorised_values.pl we have:
215     my @categories = Koha::AuthorisedValueCategories->search({ category_name => { -not_in => ['', 'branches', 'itemtypes', 'cn_source']}}, { order_by => ['category_name'] } );

So maybe a solution could be to:
- remove all the additional entries in template (if there are others than in admin/marc_subfields_structure.tt)
- the add the AV category "" to existing install (unless it already exists)
Comment 2 Katrin Fischer 2019-02-10 13:23:10 UTC
Sorry Jonathan, not sure I understand. I think there should be no empty entry in our db table, but not sure why it was added in the first place.
Comment 3 Jonathan Druart 2019-02-22 13:48:02 UTC
(In reply to Katrin Fischer from comment #2)
> Sorry Jonathan, not sure I understand. I think there should be no empty
> entry in our db table, but not sure why it was added in the first place.

from auth_val_cat.sql?
Comment 4 Katrin Fischer 2019-02-22 14:42:38 UTC
(In reply to Jonathan Druart from comment #3)
> (In reply to Katrin Fischer from comment #2)
> > Sorry Jonathan, not sure I understand. I think there should be no empty
> > entry in our db table, but not sure why it was added in the first place.
> 
> from auth_val_cat.sql?

I saw that, but why is it there? :)
Comment 5 Jonathan Druart 2019-05-04 16:09:25 UTC
So, weird history.

On bug 17216 (which created the authorised_value_categories table), there is one commit adding the empty option in the template:
+                                <option value=""></option>

(Bug 17216: Use Koha::AVC from mss.pl)

Then one commit hiding the empty value from the DB:
  Bug 17216: Do no display the empty string category name

And finally the last commit (Bug 17216: Move the AV cat list file to the top level) changed the name of .sql installer file and add the empty entry in the DB (nothing in the commit description or the bug report tells something about that change).

Also, another occurrence is in auth_subfields_structure.pl where the empty entry comes from the DB.

Prior to 17216 the empty option was added once from the .pl and once from the .tt (not consistent but at least appeared once and only once (I think!).

The thing that is wrong is that, I guess, the empty row in DB was added to satisfy the FK (marc_subfield_structure.authorised_value), but it must actually be set to NULL instead of "". The update DB entry (16.06.00.033) set it to NULL, but next change to the framework set it to "".

All of that to say that I think we should:
1. uniformize the code to make sure we are using the same code everywhere (also see Koha::AuthorisedValues->new->categories vs Koha::AuthorisedValueCategories->search)
2. Add the empty option in template, but insert NULL when picked
3. In DB, update the '' occurrences with NULL
4. Remove the '' DB row from the .sql installer file

Hope that makes sense!
Comment 6 Jonathan Druart 2019-05-04 16:10:03 UTC
(And so comment 1 was wrong!)