Bug 18577

Summary: Importing a batch using a framework not fully set up causes and endless loop
Product: Koha Reporter: Bob Ewart <bob-ewart>
Component: MARC Bibliographic record staging/importAssignee: Nick Clemens <nick>
Status: Pushed to Stable --- QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: bob-ewart, fridolin.somers, jonathan.druart, julian.maurice, katrin.fischer, nick
Version: unspecified   
Hardware: All   
OS: All   
Whiteboard:
Change sponsored?: --- Patch complexity: Trivial patch
Bot Control: --- When did the bot last check this:
Who signed the patch off: Text to go in the release notes:
Attachments: Bug 18577 - Importing a batch using a framework not fully set up causes and endless loop
Bug 18577 - Importing a batch using a framework not fully set up causes and endless loop
Bug 18577 - Importing a batch using a framework not fully set up causes and endless loop
Bug 18577: Add FIXME about the missing FK

Description Bob Ewart 2017-05-11 03:28:59 UTC
Importing a batch using a framework code that is defined but not set up causes the import to go into an endless loop.

Create a new framework code but don't set what it is based on.  
Then import a batch using that framework.
Comment 1 Nick Clemens 2017-06-23 14:14:36 UTC
Created attachment 64597 [details] [review]
Bug 18577 - Importing a batch using a framework not fully set up causes and endless loop

To test:
1 - Create a new framework - don't set it up
2 - Stage some records for import
3 - Manage the import, note your new framwork is in the list
4 - Apply patch
5 - Reload the page
6 - Note the framework is no longer an option
Comment 2 Alex Buckley 2017-06-25 14:39:32 UTC
Created attachment 64615 [details] [review]
Bug 18577 - Importing a batch using a framework not fully set up causes and endless loop

To test:
1 - Create a new framework - don't set it up
2 - Stage some records for import
3 - Manage the import, note your new framwork is in the list
4 - Apply patch
5 - Reload the page
6 - Note the framework is no longer an option

Followed test plan,  worked as intended
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 3 Julian Maurice 2017-07-06 10:02:02 UTC
QA:

I think you should add a foreign key constraint on marc_subfield_structure.frameworkcode, so that the DBIx schema can be generated by update_dbix_class_files.pl.
Is there any reason why you didn't do that ?
Comment 4 Nick Clemens 2017-07-07 11:11:25 UTC
(In reply to Julian Maurice from comment #3)
> QA:
> 
> I think you should add a foreign key constraint on
> marc_subfield_structure.frameworkcode, so that the DBIx schema can be
> generated by update_dbix_class_files.pl.
> Is there any reason why you didn't do that ?

Because the 'default' framework breaks the constraint - it doesn't exist in biblio_frameworks, it is just relied upon to be there, adding it is a larger change than I wanted to make here
Comment 5 Julian Maurice 2017-07-07 11:54:16 UTC
(In reply to Nick Clemens from comment #4)
> Because the 'default' framework breaks the constraint - it doesn't exist in
> biblio_frameworks, it is just relied upon to be there, adding it is a larger
> change than I wanted to make here
Ah that's true, I had forgotten that.
Passing QA then
Comment 6 Julian Maurice 2017-07-07 11:54:36 UTC
Created attachment 64908 [details] [review]
Bug 18577 - Importing a batch using a framework not fully set up causes and endless loop

To test:
1 - Create a new framework - don't set it up
2 - Stage some records for import
3 - Manage the import, note your new framwork is in the list
4 - Apply patch
5 - Reload the page
6 - Note the framework is no longer an option

Followed test plan,  worked as intended
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 7 Jonathan Druart 2017-07-10 21:04:32 UTC
Why do not we have this constraint at DB level?
Comment 8 Jonathan Druart 2017-07-10 21:05:06 UTC
(In reply to Jonathan Druart from comment #7)
> Why do not we have this constraint at DB level?

... Read the other comments!
Comment 9 Jonathan Druart 2017-07-10 21:09:49 UTC
Created attachment 64985 [details] [review]
Bug 18577: Add FIXME about the missing FK
Comment 10 Jonathan Druart 2017-07-10 21:12:27 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 11 Fridolin SOMERS 2017-07-18 12:11:08 UTC
Pushed to 17.05.x, will be in 17.05.02
Comment 12 Katrin Fischer 2017-07-19 21:14:57 UTC
This patch has been pushed to 16.11.x and will be in 16.11.10.