Bug 22275 - 18.06.00.060 DB update fails (incomplete/incorrect defaults)
Summary: 18.06.00.060 DB update fails (incomplete/incorrect defaults)
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Label/patron card printing (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Josef Moravec
URL:
Keywords:
Depends on: 15836
Blocks:
  Show dependency treegraph
 
Reported: 2019-02-05 14:12 UTC by Oliver Behnke
Modified: 2020-01-06 20:15 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 22275: (bug 15836 follow-up) Fix class_sources.class_split_rule FK creation (2.45 KB, patch)
2019-02-27 13:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22275: (bug 15836 follow-up) Fix class_sources.class_split_rule FK creation (2.56 KB, patch)
2019-03-05 09:02 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Behnke 2019-02-05 14:12:48 UTC
The DB schema changes in https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=82289 are incomplete/incorrect.
 
1) Table "class_sources" gets its "class_split_rule" pre-filled with all existing "class_sort_rule" counterparts, yet the "class_split_rules" table is only filled with fixed presets. Thus adding the "class_source_ibfk_2" constraint will fail as soon as there are more sort rules than split rules, that is, as soon as there are any custom sort rules.
 
2) The "class_split_rules" preset values are apparently copied from the "class_sort_rules" rules, leaving the "sorting rules" fragment in their descriptions. Those are presumably meant to be called "splitting rules".

Best,
Oliver
Comment 1 Katrin Fischer 2019-02-06 06:14:40 UTC
Hi Oliver, 

thx for the bug!

1) Table "class_sources" gets its "class_split_rule" pre-filled with all existing "class_sort_rule" counterparts, yet the "class_split_rules" table is only filled with fixed presets. Thus adding the "class_source_ibfk_2" constraint will fail as soon as there are more sort rules than split rules, that is, as soon as there are any custom sort rules.
 
I assume you added the custom sort rules yourself. I've not done that before. What did it involve? Just adding the file in the right directory?

> 2) The "class_split_rules" preset values are apparently copied from the "class_sort_rules" rules, leaving the "sorting rules" fragment in their descriptions. Those are presumably meant to be called "splitting rules".

Would it be possible for you to share a screenshot of your classification sources page with the 3 tables? I think it would be helpful to understand the issue.
Comment 2 Oliver Behnke 2019-02-06 08:56:02 UTC
Hi Katrin,

> I assume you added the custom sort rules yourself. I've not done that before. What did it involve? Just adding the file in the right directory?

That's just an administrative action at "Administration > Catalog > Classification sources". Since we're using MCS/PACS classification we simply added a custom entries (source and filing/sorting rule) when we set up Koha. To be honest, I have no idea what the actual effect of these settings/rules is, aside from describing things.

But since adding custom filing/sorting rules is clearly a normal administrative task the DB update procedure must take that into account.

> I think it would be helpful to understand the issue.

It's just about a potential misnomer: I was referring to the fact that the table holds "Classification **splitting** rules" yet the records' description always says "**sorting** rules", eg. "Default sorting rules for DDC" in case of Dewey. That seemed like a copy 'n' paste error, as it should probably read "Default **splitting** rules for DDC" in this example. That was just an informed guess. Ignore me here if my assumptions are wrong.


Cheers
Comment 3 Katrin Fischer 2019-02-06 09:02:33 UTC
I think I get it now - you mean you added a custom classification source and filing rule. I was thinking of a custom filing routine that is set in the filing rule (which wasn't changed/added to, right?)
Comment 4 Oliver Behnke 2019-02-06 09:15:26 UTC
Correct, all our custom *rules* use the pre-defined "Generic" *routines*.
Comment 5 Jonathan Druart 2019-02-16 16:22:27 UTC
Can you post a step-by-step plan to recreate the issue?
Comment 6 Oliver Behnke 2019-02-27 10:18:26 UTC
Hi Jonathan,

please see 1) in my first post:

- take a pre 18.06.00.060 instance (DB schema)
- add a custom sort rule at "Administration > Catalog > Classification sources"
- run the 18.06.00.060 DB update

Hope that helps,
Oliver
Comment 7 Jonathan Druart 2019-02-27 12:56:56 UTC
We will need a new update DB entry for 18.11.x
Comment 8 Jonathan Druart 2019-02-27 13:36:33 UTC
Created attachment 85772 [details] [review]
Bug 22275: (bug 15836 follow-up) Fix class_sources.class_split_rule FK creation

18.06.00.060 will fail if custom Classification sources were created.

In that case we should use the 'generic' split rule.

Test plan:
% git checkout -b bug_22395_pre 3a0cba67f1387595f570ab9d9efdeabc856345b4
Reset the DB
Go to  Home › Administration › Classification sources
Create a custom "Classification filing rules"
Create a custom "Classification sources" using this filing rules
% git checkout master
% perl installer/data/mysql/updatedatabase.pl
=> Without this patch you will get:
DBD::mysql::db do failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`#sql-33a_67`, CONSTRAINT `class_source_ibfk_2` FOREIGN KEY (`class_split_rule`) REFERENCES `class_split
_rules` (`class_split_rule`)) [for Statement "
            ALTER TABLE class_sources
            ADD CONSTRAINT class_source_ibfk_2 FOREIGN KEY (class_split_rule)
            REFERENCES class_split_rules (class_split_rule)
        "] at installer/data/mysql/updatedatabase.pl line 17104.

=> With this patch applied the FK will be created successfully
class_sources.class_split_rule will be set to "generic" for these custom values.
Comment 9 Oliver Behnke 2019-02-27 14:26:05 UTC
Looking at the proposed patch, how can resetting custom values be a solution to the problem? What's worse: users running the DB update won't even notice their settings got silently reset in the background. If users aren't meant to use custom values, why are those (sources, sorting and filing rules) fully editable in the first place?

Cheers
Comment 10 Oliver Behnke 2019-02-27 14:33:47 UTC
Please ignore my previous comment as I indeed was confused :) I sign off on the patch.

Cheers
Comment 11 Katrin Fischer 2019-02-27 17:03:57 UTC
(In reply to Oliver Behnke from comment #10)
> Please ignore my previous comment as I indeed was confused :) I sign off on
> the patch.
> 
> Cheers

Please switch to 'signed off' when signing off and add your sign-off to the patch. One way to do this is through the sandboxes: https://wiki.koha-community.org/wiki/Sandboxes Please let me know if you need help with that!
Comment 12 Oliver Behnke 2019-02-28 12:49:32 UTC
I wasn't sure if I'm supposed to act as the gatekeeper here :-)
Comment 13 Oliver Behnke 2019-02-28 13:57:15 UTC
FYI, I'm going to sign-off via a sandbox as well, as soon as I got one set up to test this patch for real...
Comment 14 Biblibre Sandboxes 2019-03-04 09:09:11 UTC
Patch tested with a sandbox, by Oliver Behnke <oliver.behnke@aei.mpg.de>
Comment 15 Oliver Behnke 2019-03-04 09:15:48 UTC
FYI, I couldn't coerce any sandbox into first deploying a pre-15836 database, then provide manually the necessary test and finally apply the path without further DB changes. Anyhow, I double checked the patch itself so my sign-off still holds.

Cheers
Comment 16 Josef Moravec 2019-03-05 09:02:06 UTC
Created attachment 86048 [details] [review]
Bug 22275: (bug 15836 follow-up) Fix class_sources.class_split_rule FK creation

18.06.00.060 will fail if custom Classification sources were created.

In that case we should use the 'generic' split rule.

Test plan:
% git checkout -b bug_22395_pre 3a0cba67f1387595f570ab9d9efdeabc856345b4
Reset the DB
Go to  Home › Administration › Classification sources
Create a custom "Classification filing rules"
Create a custom "Classification sources" using this filing rules
% git checkout master
% perl installer/data/mysql/updatedatabase.pl
=> Without this patch you will get:
DBD::mysql::db do failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`#sql-33a_67`, CONSTRAINT `class_source_ibfk_2` FOREIGN KEY (`class_split_rule`) REFERENCES `class_split
_rules` (`class_split_rule`)) [for Statement "
            ALTER TABLE class_sources
            ADD CONSTRAINT class_source_ibfk_2 FOREIGN KEY (class_split_rule)
            REFERENCES class_split_rules (class_split_rule)
        "] at installer/data/mysql/updatedatabase.pl line 17104.

=> With this patch applied the FK will be created successfully
class_sources.class_split_rule will be set to "generic" for these custom values.

Signed-off-by: Oliver Behnke <oliver.behnke@aei.mpg.de>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 17 Nick Clemens 2019-03-07 21:21:17 UTC
Awesome work all!

Pushed to master for 19.05
Comment 18 Oliver Behnke 2019-03-08 08:56:52 UTC
Just to make sure: is this going to be pushed to 18.11 as well? I think it has to be.

Thanks everyone!
Comment 19 Katrin Fischer 2019-03-08 09:18:49 UTC
(In reply to Oliver Behnke from comment #18)
> Just to make sure: is this going to be pushed to 18.11 as well? I think it
> has to be.
> 
> Thanks everyone!

RMaints have the final decision, but since this is marked as a major bug and appears int he older versions, this will be considered (all bugs usually are, while enhancements are not).
Comment 20 Martin Renvoize 2019-03-12 08:42:04 UTC
Pushed to 18.11.x for 18.11.04
Comment 21 Lucas Gass 2019-03-15 16:52:10 UTC
missing 15836 dependency, not backporting to 18.05.x