Bug 18111 - Import default framework is broken
Summary: Import default framework is broken
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: master
Hardware: All Linux
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 16035
Blocks:
  Show dependency treegraph
 
Reported: 2017-02-14 11:22 UTC by Viliam
Modified: 2017-03-15 11:11 UTC (History)
7 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:


Attachments
Bug 18111: Fix import of default framework (4.32 KB, patch)
2017-02-14 16:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18111: Fix import of default framework (4.33 KB, patch)
2017-02-15 07:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18111: Swap framework_name and frameworkcode (2.86 KB, patch)
2017-02-19 04:14 UTC, M. Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18111: Fix import of default framework (4.40 KB, patch)
2017-02-19 20:40 UTC, Josef Moravec
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18111: Swap framework_name and frameworkcode (2.92 KB, patch)
2017-02-19 20:41 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18111: Fix import of default framework (4.45 KB, patch)
2017-02-21 14:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18111: Swap framework_name and frameworkcode (2.98 KB, patch)
2017-02-21 14:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 18111: Follow-up for 3.22.x (6.73 KB, patch)
2017-03-15 11:11 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Viliam 2017-02-14 11:22:33 UTC
We can't import our custom framework as default framework. We try it in KOHA 16.01 and 16.05. We also try to export default framework, edit only one name of field and still it's impossible to import it as default framework. In version 3.20 everything is OK.
Comment 1 Jonathan Druart 2017-02-14 16:04:31 UTC
Confirmed on master.
Comment 2 Jonathan Druart 2017-02-14 16:24:44 UTC
Created attachment 60216 [details] [review]
Bug 18111: Fix import of default framework

Caused by bug 16035.

Recreate the issue:
Go to Home › Administration › MARC frameworks
For the default framework: Actions > import
It will not work

Test plan:
Confirm test plans from bug 16035 and bug 17389 still pass
Confirm that the import for the default framework now works
Comment 3 Jonathan Druart 2017-02-14 16:25:02 UTC
Upping severity.
Comment 4 M. Tompsett 2017-02-15 00:15:29 UTC
Comment on attachment 60216 [details] [review]
Bug 18111: Fix import of default framework

Review of attachment 60216 [details] [review]:
-----------------------------------------------------------------

Overall, I like this cleaner code.

::: admin/import_export_framework.pl
@@ +53,5 @@
>  ## Exporting
>  if ($action eq 'export' && $input->request_method() eq 'GET') {
>      my $strXml = '';
> +    my $format = $input->param('type_export_' . $framework_name);
> +    ExportFramework('', \$strXml, $format);

Shouldn't this be $frameworkcode? You split the code and name logic, so the code is set properly.
Comment 5 Jonathan Druart 2017-02-15 07:45:41 UTC
Created attachment 60239 [details] [review]
Bug 18111: Fix import of default framework

Caused by bug 16035.

Recreate the issue:
Go to Home › Administration › MARC frameworks
For the default framework: Actions > import
It will not work

Test plan:
Confirm test plans from bug 16035 and bug 17389 still pass
Confirm that the import for the default framework now works
Comment 6 Jonathan Druart 2017-02-15 07:46:20 UTC
(In reply to M. Tompsett from comment #4)
> > +    ExportFramework('', \$strXml, $format);
> 
> Shouldn't this be $frameworkcode? You split the code and name logic, so the
> code is set properly.

Erk yes of course, I kept the wrong line!
Comment 7 M. Tompsett 2017-02-19 04:14:07 UTC
Created attachment 60436 [details] [review]
Bug 18111: Swap framework_name and frameworkcode

With the frameworkcode changed to an empty string, which really
is closer to the physical reality, some javascript somewhere
breaks. This means the spinner does not appear when you confirm
that you wish to import it.

TEST PLAN
----------
1) apply first patch
2) export default framework, doesn't matter format
3) import default framework using that file.
4) answer yes or whatever it is.
   -- spinner does not appear.
5) apply this patch
6) import default framework use the same file.
7) answer yes or whatever it is.
   -- spinner does appear.
8) decide if you wish to find and fix the javascript conditions.
   if yes -- do so, and then obsolete this once you have.
   if not -- sign this off after running koha qa test tools
Comment 8 Josef Moravec 2017-02-19 20:40:53 UTC
Created attachment 60437 [details] [review]
[SIGNED-OFF] Bug 18111: Fix import of default framework

Caused by bug 16035.

Recreate the issue:
Go to Home › Administration › MARC frameworks
For the default framework: Actions > import
It will not work

Test plan:
Confirm test plans from bug 16035 and bug 17389 still pass
Confirm that the import for the default framework now works

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 9 Josef Moravec 2017-02-19 20:41:46 UTC
Created attachment 60438 [details] [review]
[SIGNED-OFF] Bug 18111: Swap framework_name and frameworkcode

With the frameworkcode changed to an empty string, which really
is closer to the physical reality, some javascript somewhere
breaks. This means the spinner does not appear when you confirm
that you wish to import it.

TEST PLAN
----------
1) apply first patch
2) export default framework, doesn't matter format
3) import default framework using that file.
4) answer yes or whatever it is.
   -- spinner does not appear.
5) apply this patch
6) import default framework use the same file.
7) answer yes or whatever it is.
   -- spinner does appear.
8) decide if you wish to find and fix the javascript conditions.
   if yes -- do so, and then obsolete this once you have.
   if not -- sign this off after running koha qa test tools

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 10 Tomás Cohen Arazi 2017-02-21 14:40:16 UTC
Created attachment 60522 [details] [review]
Bug 18111: Fix import of default framework

Caused by bug 16035.

Recreate the issue:
Go to Home › Administration › MARC frameworks
For the default framework: Actions > import
It will not work

Test plan:
Confirm test plans from bug 16035 and bug 17389 still pass
Confirm that the import for the default framework now works

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 11 Tomás Cohen Arazi 2017-02-21 14:40:25 UTC
Created attachment 60523 [details] [review]
Bug 18111: Swap framework_name and frameworkcode

With the frameworkcode changed to an empty string, which really
is closer to the physical reality, some javascript somewhere
breaks. This means the spinner does not appear when you confirm
that you wish to import it.

TEST PLAN
----------
1) apply first patch
2) export default framework, doesn't matter format
3) import default framework using that file.
4) answer yes or whatever it is.
   -- spinner does not appear.
5) apply this patch
6) import default framework use the same file.
7) answer yes or whatever it is.
   -- spinner does appear.
8) decide if you wish to find and fix the javascript conditions.
   if yes -- do so, and then obsolete this once you have.
   if not -- sign this off after running koha qa test tools

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 12 Tomás Cohen Arazi 2017-02-21 14:41:04 UTC
The fix works as expected.
Comment 13 Kyle M Hall 2017-02-21 19:53:36 UTC
Pushed to master for 17.05, thanks Jonathan, Mark!
Comment 14 Katrin Fischer 2017-02-21 20:58:40 UTC
These patches have been pushed to 16.11.x and will be in 16.11.04.
Comment 15 Viliam 2017-02-23 07:23:30 UTC
import default framework in 16.11.04 is OK, thanks
Comment 16 Mason James 2017-02-23 20:25:53 UTC
Pushed to 16.05.x, for 16.05.10 release
Comment 17 Julian Maurice 2017-03-15 11:11:17 UTC
Created attachment 61099 [details] [review]
Bug 18111: Follow-up for 3.22.x

Replace [% frameworkcode %] by 'default' wherever needed
Comment 18 Julian Maurice 2017-03-15 11:11:57 UTC
Pushed to 3.22.x, will be in 3.22.18