Bug 13952 - Import and export of authority types
Summary: Import and export of authority types
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: Aleisha Amohia
QA Contact: Josef Moravec
URL:
Keywords: Academy, Manual, release-notes-needed
Depends on: 19594
Blocks:
  Show dependency treegraph
 
Reported: 2015-04-03 07:30 UTC by Josef Moravec
Modified: 2019-10-21 09:37 UTC (History)
13 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Large patch
Who signed the patch off:
Text to go in the release notes:
This feature allows the import and export of authority types to match the capabilities of biblio frameworks.
Version(s) released in:


Attachments
Bug 13592: Add an option to charge for any hold placed (7.19 KB, patch)
2015-11-11 15:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (18.96 KB, patch)
2016-01-26 23:55 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (22.80 KB, patch)
2016-01-27 21:25 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (22.92 KB, patch)
2016-01-27 22:18 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (24.52 KB, patch)
2016-04-07 22:42 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (24.72 KB, patch)
2016-07-03 23:51 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (24.69 KB, patch)
2016-07-03 23:52 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (26.43 KB, patch)
2016-08-25 04:04 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (26.48 KB, patch)
2016-08-26 02:11 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (26.56 KB, patch)
2016-08-26 02:14 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (26.85 KB, patch)
2016-12-02 02:37 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (27.55 KB, patch)
2017-02-02 22:10 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing authority type import (12.31 KB, patch)
2017-06-19 01:02 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (27.66 KB, patch)
2017-10-10 22:29 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (27.33 KB, patch)
2017-10-10 22:41 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (27.33 KB, patch)
2017-10-19 23:59 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing authority type import (12.31 KB, patch)
2017-10-20 00:00 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing import for default auth type (2.39 KB, patch)
2017-10-20 00:00 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing export on default biblio framework (2.35 KB, patch)
2017-10-24 20:47 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (27.39 KB, patch)
2017-10-25 06:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing authority type import (12.37 KB, patch)
2017-10-25 06:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing import for default auth type (2.45 KB, patch)
2017-10-25 06:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing export on default biblio framework (2.41 KB, patch)
2017-10-25 06:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing JS error (1.31 KB, patch)
2017-10-25 20:24 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (27.56 KB, patch)
2017-12-20 03:35 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing authority type import (12.37 KB, patch)
2017-12-20 03:35 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing import for default auth type (2.45 KB, patch)
2017-12-20 03:35 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing export on default biblio framework (2.41 KB, patch)
2017-12-20 03:36 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing JS error (1.31 KB, patch)
2017-12-20 03:36 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (27.70 KB, patch)
2018-07-04 02:18 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing authority type import (12.32 KB, patch)
2018-07-04 02:18 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing import for default auth type (2.45 KB, patch)
2018-07-04 02:18 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing export on default biblio framework (2.41 KB, patch)
2018-07-04 02:18 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (27.91 KB, patch)
2018-10-18 04:32 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing authority type import (12.32 KB, patch)
2018-10-18 04:32 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing import for default auth type (2.45 KB, patch)
2018-10-18 04:32 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing export on default biblio framework (2.41 KB, patch)
2018-10-18 04:32 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: (follow-up) Fixing leftover merge conflict stuff (1.46 KB, patch)
2018-10-25 23:25 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (27.73 KB, patch)
2018-12-05 00:35 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing authority type import (12.20 KB, patch)
2018-12-05 00:35 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing import for default auth type (2.45 KB, patch)
2018-12-05 00:35 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: [FOLLOW-UP] Fixing export on default biblio framework (2.41 KB, patch)
2018-12-05 00:35 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: (follow-up) template filters, moving js to separate file (19.41 KB, patch)
2018-12-05 21:33 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (27.73 KB, patch)
2018-12-10 18:42 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 13952: (follow-up) Fixing authority type import (12.20 KB, patch)
2018-12-10 18:42 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 13952: (follow-up) Fixing import for default auth type (2.45 KB, patch)
2018-12-10 18:42 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 13952: (follow-up) Fixing export on default biblio framework (2.41 KB, patch)
2018-12-10 18:42 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 13952: (follow-up) template filters, moving js to separate file (18.28 KB, patch)
2018-12-10 18:42 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 13952: (follow-up) Tests (41.66 KB, patch)
2019-02-14 03:42 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: (follow-up) Tests (40.97 KB, patch)
2019-03-12 20:27 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 13952: Import and export authority types (27.85 KB, patch)
2019-03-12 20:35 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 13952: (follow-up) Fixing authority type import (12.31 KB, patch)
2019-03-12 20:35 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 13952: (follow-up) Fixing import for default auth type (2.55 KB, patch)
2019-03-12 20:35 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 13952: (follow-up) Fixing export on default biblio framework (2.51 KB, patch)
2019-03-12 20:35 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 13952: (follow-up) template filters, moving js to separate file (18.39 KB, patch)
2019-03-12 20:35 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 13952: (follow-up) Tests (41.09 KB, patch)
2019-03-12 20:35 UTC, Michal Denar
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Josef Moravec 2015-04-03 07:30:43 UTC
We need the function to export and import authority types. It should work the way like import/export of biblio marc frameworks works.
Comment 1 Jonathan Druart 2015-11-11 15:19:44 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2015-11-11 15:20:42 UTC
Oops, typo in the bug number, sorry for the noise.
Comment 3 Aleisha Amohia 2016-01-26 23:55:27 UTC Comment hidden (obsolete)
Comment 4 Michal Denar 2016-01-27 11:26:50 UTC
Hello Aleisha,
I tested patch but output are non correct.

Examples:
empty export of unification names
http://pastebin.com/qnCassBC

Personal names, just some mess:
http://pastebin.com/1X6NXgeN

Expected is same type of output like export from biblio frameworks. Lets find bug :-)

Thank You.

Mike
Comment 5 Michal Denar 2016-01-27 11:31:59 UTC
Hello Aleisha,
I tested patch but outputs are non correct.

Examples:
empty export of unification names
http://pastebin.com/qnCassBC

Personal names, just some mess:
http://pastebin.com/1X6NXgeN

Expected is same type of output like export from biblio frameworks. Lets find bug :-)

Export from default biblio framework don't work - empty file

Thank You.

Mike
Comment 6 Josef Moravec 2016-01-27 11:43:13 UTC
First of all, thank you Aleisha!

But this doesn't work, because functions _export_table_* take data from database filtered by column "frameworkcode" which in auth_subfield_structure is not always populated and in auth_tag_structure is not present at all.

I'm not sure how to deal with it, just some ideas for now:
1. we could add new parameter to export routines, which can say the column should be used to filter data (authtypecode if we are working with authorities)
2. add and populate column frameworkcode to tables auth_tag_structure and auth_subfield_structure, which should have the same value as authtypecode. But I don't really like this solution as it's more work ending with dumb duplicate column
3. something else
Comment 7 Josef Moravec 2016-01-27 11:58:23 UTC
or 
4. just unify the authtypecode/frameworkcode and name it the same
Comment 8 Michal Denar 2016-01-27 12:16:58 UTC
Hello.

I vote for solutions 1 or 4.

Thanks for response.


Mike
Comment 9 Chris Cormack 2016-01-27 18:54:14 UTC
(In reply to Mike from comment #8)
> Hello.
> 
> I vote for solutions 1 or 4.
> 
>
A twist one 1, we are already passing a value which tells us if we are looking at authorities or not, if we pass that down to the next layer of functions also, as the new parameter you were talking about, that would be an ok way of doing it I think?

4/ would need massive amounts of testing. 1/ only effects the import and export so I think is a lot safer.
Comment 10 Aleisha Amohia 2016-01-27 21:25:13 UTC Comment hidden (obsolete)
Comment 11 Aleisha Amohia 2016-01-27 22:18:54 UTC Comment hidden (obsolete)
Comment 12 Bernardo Gonzalez Kriegel 2016-04-04 18:02:25 UTC
Sorry, does not apply

CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/authtypes.tt
Comment 13 Aleisha Amohia 2016-04-07 22:42:56 UTC Comment hidden (obsolete)
Comment 14 Bernardo Gonzalez Kriegel 2016-04-11 18:49:14 UTC
Hi, testing this.
Export works Ok, all formats.

Import does not work. I created a new auth type, then 'imported' some
of the exported formats, auth_tag_structure and auth_subfield_structure are not populated, instead auth_tag_structure.pl gives you the option to fill fields using another auth_type (after some time).

Looking at the logs you find

DBD::mysql::st execute failed: Unknown column 'frameworkcode' in 'where clause' [for Statement "DELETE FROM auth_tag_structure WHERE frameworkcode=? AND ...

and that's right because, unfortunately,  auth tables use authtypecode instead of frameworkcode.

Also in the patch
         $query = 'SELECT * FROM ' . $table . ' WHERE frameworkcode=?';
         $query = 'SELECT * FROM ' . $table . ' WHERE frameworkcode=?';
         $query = 'SELECT * FROM ' . $table . ' WHERE frameworkcode=?';
         $query = 'SELECT ' . join(',', @fields) . ' FROM ' . $table . ' WHERE frameworkcode=?';

Aditionally there are a few tabs that must be removed,
 FAIL   C4/ImportExportFramework.pm
   OK     critic
   FAIL   forbidden patterns
                forbidden pattern: tab char (line 234)
                forbidden pattern: tab char (line 285)
                forbidden pattern: tab char (line 236)
                forbidden pattern: tab char (line 235)
                forbidden pattern: tab char (line 237)
                forbidden pattern: tab char (line 232)
                forbidden pattern: tab char (line 233)
Comment 15 Bernardo Gonzalez Kriegel 2016-04-11 18:52:59 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #14)
> Also in the patch
>          $query = 'SELECT * FROM ' . $table . ' WHERE frameworkcode=?';
>          $query = 'SELECT * FROM ' . $table . ' WHERE frameworkcode=?';
>          $query = 'SELECT * FROM ' . $table . ' WHERE frameworkcode=?';
>          $query = 'SELECT ' . join(',', @fields) . ' FROM ' . $table . '
> WHERE frameworkcode=?';
> 

This is present code, not in the patch, sorry.
Comment 16 Aleisha Amohia 2016-07-03 23:51:30 UTC Comment hidden (obsolete)
Comment 17 Aleisha Amohia 2016-07-03 23:52:54 UTC Comment hidden (obsolete)
Comment 18 Aleisha Amohia 2016-07-03 23:54:10 UTC
Can you please test this again? The import seems to work fine for me because we have put in checks for if it is an authority or not.
Comment 19 Josef Moravec 2016-07-27 09:07:40 UTC
I've just tested it. Exporting works as expected, but Importing doesn't. After selecting file and confirming it, it takes me to auth_tag_structure.pl as expected, but the newly created authtype is still empty - nothing is imported.

Exporting and importing bibliographics frameworks works fine.
Comment 20 Aleisha Amohia 2016-08-25 04:04:49 UTC Comment hidden (obsolete)
Comment 21 Aleisha Amohia 2016-08-25 04:11:09 UTC
Just to explain further:
We worked out why Import wasn't working. The patch itself appears to be fine. 

When we try to Import and exported file, the database sees that authtypecode already exists and instead of importing into a new authority type, it simply updates the existing authority type with that authtypecode. This is why we see no error when it doesn't work - it still 'worked', just didn't do what we wanted it to do.

This means we must change the file itself so that authtypecode matches the Code of our new authority type, however after spending way too long fiddling around, we realised that LibreOffice formatted the file weirdly after saving and caused errors in the file, so the Import didn't work properly. This is probably a separate bug, regarding the way the code handles incorrectly formatted files and the error message it shows.

Changing the authtype code in Vim did not stuff up the formatting and meant we were able to Import correctly. So, the patch itself should work as long as all the speechmarks etc. are in the right places after you have changed the authtypecode to match your new authority type.
Comment 22 Aleisha Amohia 2016-08-26 02:11:37 UTC Comment hidden (obsolete)
Comment 23 Aleisha Amohia 2016-08-26 02:14:36 UTC Comment hidden (obsolete)
Comment 24 Aleisha Amohia 2016-09-16 02:26:23 UTC
Why was this set to Patch doesn't apply?
Comment 25 Aleisha Amohia 2016-09-16 02:33:05 UTC
Appears to apply cleanly for me still so will set back to Needs Signoff. :)
Comment 26 Michael Kuhn 2016-10-13 10:11:37 UTC
Applying: Bug 13952: Import and export authority types
fatal: sha1 information is lacking or useless (admin/import_export_framework.pl).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 13952: Import and export authority types
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-13952-Import-and-export-authority-types-u9za7y.patch
Comment 27 Aleisha Amohia 2016-12-02 02:37:28 UTC Comment hidden (obsolete)
Comment 28 Aleisha Amohia 2017-02-02 22:10:27 UTC Comment hidden (obsolete)
Comment 29 M. Tompsett 2017-05-02 00:00:42 UTC
Comment on attachment 59831 [details] [review]
Bug 13952: Import and export authority types

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

Just eyeballed this.

::: admin/import_export_authtype.pl
@@ +18,5 @@
> +# along with Koha; if not, see <http://www.gnu.org/licenses>.
> +
> +
> +use strict;
> +use warnings;

The recommendation is use Modern::Perl; for the equivalent of these.

@@ +26,5 @@
> +use C4::Auth qw/check_cookie_auth/;
> +use C4::ImportExportFramework;
> +
> +my %cookies = CGI::Cookie->fetch();
> +my $authenticated = 0;

The variable seems extraneous.
Optimized, you could $auth_status //= q{}; and unless ($auth_status eq 'ok').
Comment 30 Josef Moravec 2017-05-23 17:49:24 UTC
Exporting does work as expected.

But I don't think the importing should work this way: it does NOT matter, to which authtype you import, it imports everytime to the framework with the same typecode - I think step 6 in test plan is bad - biblio framework import does not need that, so it should be consistent.

And please update your patch according to comment 29.
Comment 31 Aleisha Amohia 2017-06-19 01:02:58 UTC Comment hidden (obsolete)
Comment 32 Katrin Fischer 2017-10-07 20:57:24 UTC
Patch doesn't apply, not sure how to fix, sorry!

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 13952: Import and export authority types
Using index info to reconstruct a base tree...
M	admin/import_export_framework.pl
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/authtypes.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/authtypes.tt
Auto-merging admin/import_export_framework.pl
CONFLICT (content): Merge conflict in admin/import_export_framework.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 13952: Import and export authority types
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-13952-Import-and-export-authority-types-CLgCHQ.patch
Comment 33 Aleisha Amohia 2017-10-10 22:29:59 UTC Comment hidden (obsolete)
Comment 34 Aleisha Amohia 2017-10-10 22:41:49 UTC Comment hidden (obsolete)
Comment 35 Aleisha Amohia 2017-10-10 22:44:23 UTC
Rebased, updated test plan, ready to test
Comment 36 Katrin Fischer 2017-10-14 20:05:45 UTC
Sequence of attached patches is wrong, but works when you apply them in order :)

I have started testing by regression testing the bibliographic frameworks export and import and for the default framework some things seem not quite right:

- When exporting the default framework to Excel format the file ending changed from xml to ods for bibliographic frameworks
- Reimporting the exported file for Default always results in unspecific error message.
- QA might ask for tests for the changes to ImportExportFramework.pm
Comment 37 Aleisha Amohia 2017-10-19 23:59:53 UTC Comment hidden (obsolete)
Comment 38 Aleisha Amohia 2017-10-20 00:00:02 UTC Comment hidden (obsolete)
Comment 39 Aleisha Amohia 2017-10-20 00:00:45 UTC Comment hidden (obsolete)
Comment 40 Aleisha Amohia 2017-10-20 00:02:15 UTC
(In reply to Katrin Fischer from comment #36)
> Sequence of attached patches is wrong, but works when you apply them in
> order :)

Fixed!


> I have started testing by regression testing the bibliographic frameworks
> export and import and for the default framework some things seem not quite
> right:
> 
> - When exporting the default framework to Excel format the file ending
> changed from xml to ods for bibliographic frameworks

hmm I could not recreate this! it seemed to work fine for me

> - Reimporting the exported file for Default always results in unspecific
> error message.

I believe I have fixed this in latest follow-up patch!

> - QA might ask for tests for the changes to ImportExportFramework.pm

I had a look for the tests but couldn't find them for some reason - do you know what file they're in?
Comment 41 Katrin Fischer 2017-10-21 21:50:31 UTC
I am sorry Aleisha, but the bibliographic default framework still exports as .ods when I select csv. 

To test:
- Go to administration > bibliographic marc frameworks
- Export > CSV
- Verify file ending is .csv
- Apply patches
- restart_all on kohadevbox
- Repeat export
- Verify file ending is now .ods
Comment 42 Aleisha Amohia 2017-10-24 20:47:06 UTC Comment hidden (obsolete)
Comment 43 Aleisha Amohia 2017-10-24 20:47:49 UTC
(In reply to Katrin Fischer from comment #41)
> I am sorry Aleisha, but the bibliographic default framework still exports as
> .ods when I select csv. 
> 
> To test:
> - Go to administration > bibliographic marc frameworks
> - Export > CSV
> - Verify file ending is .csv
> - Apply patches
> - restart_all on kohadevbox
> - Repeat export
> - Verify file ending is now .ods

Sorry Katrin I was missing the 'default' part. Found the problem, fixed in latest patch.
Comment 44 Katrin Fischer 2017-10-25 06:21:45 UTC
Hi Aleisha, export is all looking good now. Signing off.

Not a blocker for me:

1) When exporting authority types, the window stays open in the background, when exporting bibliographic frameworks it closes after clicking the 'export' button.

2) Not a blocker as this is the current behaviour, but maybe something for the future:

>6) IMPORTANT STEP:
>    Before you import, make sure that the authtypecode of the file you
>    are going to import matches the Code of your new authority type.

Wouldn't it be better to display a warning in the case that the selected framework for import and the code in the file don't match? It seems like this could cause some unwanted data loss.
Comment 45 Katrin Fischer 2017-10-25 06:23:04 UTC Comment hidden (obsolete)
Comment 46 Katrin Fischer 2017-10-25 06:23:09 UTC Comment hidden (obsolete)
Comment 47 Katrin Fischer 2017-10-25 06:23:14 UTC Comment hidden (obsolete)
Comment 48 Katrin Fischer 2017-10-25 06:23:19 UTC Comment hidden (obsolete)
Comment 49 Aleisha Amohia 2017-10-25 20:23:42 UTC
(In reply to Katrin Fischer from comment #44)
> Hi Aleisha, export is all looking good now. Signing off.
> 
> Not a blocker for me:
> 
> 1) When exporting authority types, the window stays open in the background,
> when exporting bibliographic frameworks it closes after clicking the
> 'export' button.

Will fix this.

> 
> 2) Not a blocker as this is the current behaviour, but maybe something for
> the future:
> 
> >6) IMPORTANT STEP:
> >    Before you import, make sure that the authtypecode of the file you
> >    are going to import matches the Code of your new authority type.
> 
> Wouldn't it be better to display a warning in the case that the selected
> framework for import and the code in the file don't match? It seems like
> this could cause some unwanted data loss.

I fixed this in Comment 31, you don't have to match up the authtypecode anymore, this step is from an old test plan.

For those in the future testing, please use the most recent test plan from Comment 45.
Comment 50 Aleisha Amohia 2017-10-25 20:24:00 UTC Comment hidden (obsolete)
Comment 51 Katrin Fischer 2017-10-25 20:28:43 UTC
Hm, it didn't work for me until I had changed the frameworkcode this morning - I actually had missed the step at first and then went back to look for a reason why it would not work. 

I had created a new framework, imported into the empty framework, page relaoded, but it remained empty.
Comment 52 Liz Rea 2017-10-25 20:59:30 UTC
Hallo,

Aleisha and I just walked through this patch and it is all working ok for me. We didn't have to do anything to the export files to get them to import into a new framework, nor did we have any trouble exporting the files. Bibliographic frameworks still worked as well.

tl;dr: Sorry, wasn't able to replicate your problem here :(

Not sure what exactly the difference could be, Aleisha and I were both using the devbox, with and without Plack enabled, on current master.

Any thoughts on what might be different?

Cheers,
Liz
Comment 53 Katrin Fischer 2017-10-25 21:10:19 UTC
Not sure when I can test again :( Maybe it was just a data problem on my side.
Comment 54 Katrin Fischer 2017-12-19 07:32:12 UTC
Hi Aleisha, want to retest and try to push this through QA, can you please rebase?
Comment 55 Aleisha Amohia 2017-12-20 03:35:46 UTC Comment hidden (obsolete)
Comment 56 Aleisha Amohia 2017-12-20 03:35:52 UTC Comment hidden (obsolete)
Comment 57 Aleisha Amohia 2017-12-20 03:35:57 UTC Comment hidden (obsolete)
Comment 58 Aleisha Amohia 2017-12-20 03:36:01 UTC Comment hidden (obsolete)
Comment 59 Aleisha Amohia 2017-12-20 03:36:06 UTC
Created attachment 69944 [details] [review]
Bug 13952: [FOLLOW-UP] Fixing JS error

Missing a bracket that stopped the export modal from closing after
clicking the button. Now fixed.
Comment 60 Aleisha Amohia 2017-12-20 03:37:34 UTC
(In reply to Katrin Fischer from comment #54)
> Hi Aleisha, want to retest and try to push this through QA, can you please
> rebase?

Rebased, ready for you to retest
Comment 61 Katrin Fischer 2017-12-28 06:52:18 UTC
Hi Aleisha, I still have the problem with the modal not closing. I have used 'restart_all' and shift-reloaded the page a couple of times. Could you take another look please?
Comment 62 Katrin Fischer 2017-12-28 07:05:42 UTC
There are a few JavaScript errors on the page, I think the patches need to be adjusted for the changes made moving the JavaScript to the bottom recently:

http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=7c5c516844625592ed68ca91be5a76852b4bfdb6
Comment 63 Aleisha Amohia 2018-07-04 02:18:36 UTC Comment hidden (obsolete)
Comment 64 Aleisha Amohia 2018-07-04 02:18:43 UTC Comment hidden (obsolete)
Comment 65 Aleisha Amohia 2018-07-04 02:18:49 UTC Comment hidden (obsolete)
Comment 66 Aleisha Amohia 2018-07-04 02:18:57 UTC Comment hidden (obsolete)
Comment 67 Aleisha Amohia 2018-07-04 02:48:21 UTC
Hello, 

I've rebased the patches and tested them. When importing authority types, it appears that the modal box never closes, but I waited a few minutes and it did eventually close and all the data had loaded in correctly. So technically the feature works, but it's very slow. I'm not sure how to make it faster. Will set to Needs Signoff for now and allow testing to happen unless someone can give a tip for why it's slow and how to fix that - but for now, it does work.
Comment 68 Hugo Agud 2018-09-20 10:09:36 UTC
Applying: Bug 13952: Import and export authority types
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/authtypes.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/authtypes.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/authtypes.tt
Failed to merge in the changes.
Patch failed at 0001 Bug 13952: Import and export authority types
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
Comment 69 Aleisha Amohia 2018-10-18 04:32:02 UTC Comment hidden (obsolete)
Comment 70 Aleisha Amohia 2018-10-18 04:32:11 UTC Comment hidden (obsolete)
Comment 71 Aleisha Amohia 2018-10-18 04:32:20 UTC Comment hidden (obsolete)
Comment 72 Aleisha Amohia 2018-10-18 04:32:27 UTC Comment hidden (obsolete)
Comment 73 Michal Denar 2018-10-19 19:52:53 UTC
Everything work but:
- import non correct framework fails, but without any message
- export window freeze open after export = different behaviour from MARC framework export
Comment 74 Aleisha Amohia 2018-10-25 23:25:21 UTC Comment hidden (obsolete)
Comment 75 Aleisha Amohia 2018-12-05 00:35:35 UTC Comment hidden (obsolete)
Comment 76 Aleisha Amohia 2018-12-05 00:35:43 UTC Comment hidden (obsolete)
Comment 77 Aleisha Amohia 2018-12-05 00:35:49 UTC Comment hidden (obsolete)
Comment 78 Aleisha Amohia 2018-12-05 00:35:57 UTC Comment hidden (obsolete)
Comment 79 Aleisha Amohia 2018-12-05 00:36:18 UTC Comment hidden (obsolete)
Comment 80 Aleisha Amohia 2018-12-05 00:46:34 UTC
(In reply to Aleisha Amohia from comment #74)

> 
> Have noticed however that export breaks with authtypecodes that have a
> slash (/) in them. Will set to Assigned to fix this.
> 

I have noticed that this is an issue that already exists in biblio frameworks, so I do not think it is required for this patch. I can work on a patch that depends on this one that fixes this problem in biblio frameworks and authority types, but this patch can get through without this fix.
Comment 81 Aleisha Amohia 2018-12-05 21:33:05 UTC Comment hidden (obsolete)
Comment 82 Aleisha Amohia 2018-12-05 21:34:24 UTC
(In reply to Aleisha Amohia from comment #74)

> Also need to add a fail message for when authtypecodes do not match on
> import.

Upon further testing, I've realised that this is no longer an issue. You CAN import a file with unmatching authtypecodes and it will work and fix up the authtypecode in the import - it just takes a little while. I have added a note to the popup so the user expects to wait a few minutes.

This is ready to test
Comment 83 Owen Leonard 2018-12-10 18:42:34 UTC
Created attachment 83024 [details] [review]
Bug 13952: Import and export authority types

This patch amends C4::ImportExportFramework to work for authority types as well as MARC frameworks.
New file: admin/import_export_authtype.pl
Update: Ensuring we are passing the right column to the right tables.
Update2: Making the error messages the same to be consistent with patch on Bug 15665
Update3: Fixing merge conflicts
Update4: Fixing merge conflicts and removing tabs
Update5: Getting rid of warns, making sure Import and Export of default
authority will work
Update6: Merge conflicts and making sure export of default auth type
works
Update7: Fixing merge conflicts and updating buttons to bootstrap3

To test:
1) Go to Admin -> Authority types
2) Confirm there are two new columns 'Export' and 'Import' in the table
3) Click 'Export' on an existing authority type and choose a file type, click 'Export'
4) Confirm that the authority type is exported as your chosen file type. Save the file
5) Create a new authority type
6) Import into your new authority type using the file you just exported
7) Confirm you are taken to auth_tag_structure.pl
8) Go back to Authority types
9) Export your new authority type. View the exported file and confirm
the authtypecode has been updated to match the code you set for the new
auth type
10) Click 'Import' next to any existing authority type and attempt to import a file that is not XML, CSV or ODS. Confirm that this fails and you are asked to import a file of the correct file type
11) Confirm Export and Import work for the Default authority type
12) Go to Admin -> MARC bibliographic framework
13) Confirm that the 'Export' and 'Import' functions still work here as well

Sponsored-by: Catalyst IT

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 84 Owen Leonard 2018-12-10 18:42:39 UTC
Created attachment 83025 [details] [review]
Bug 13952: (follow-up) Fixing authority type import

This patch fixes the comments from Comment 29 and the import
functionality. You should now be able to import an exported file without
editing the file at all and the authority type code will be overwritten
in the file (same behaviour as biblio frameworks).

Sponsored-by: Catalyst IT

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 85 Owen Leonard 2018-12-10 18:42:44 UTC
Created attachment 83026 [details] [review]
Bug 13952: (follow-up) Fixing import for default auth type

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 86 Owen Leonard 2018-12-10 18:42:49 UTC
Created attachment 83027 [details] [review]
Bug 13952: (follow-up) Fixing export on default biblio framework

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 87 Owen Leonard 2018-12-10 18:42:53 UTC
Created attachment 83028 [details] [review]
Bug 13952: (follow-up) template filters, moving js to separate file

This patch adds filters to the template file and moves the JS into a
separate file.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 88 Katrin Fischer 2019-02-02 20:47:54 UTC
Hi Owen, I see you signed off the follow-up, does this count for the whole patch set or only the follow-up? Looking to get my QA token back :)
Comment 89 Martin Renvoize 2019-02-08 15:01:26 UTC
QA failure I'm afraid:

1) Changes to a module without corresponding additions to the tests.
2) I 'think' this patch introduces noise in the existing tests (which makes me think it introduces noise in the logs too, but that is untested). - [Use of uninitialized value $frameworktype in string eq at ~/kohaclone/C4/ImportExportFramework.pm line 1157.]

Otherwise, we're really close now.
Comment 90 Martin Renvoize 2019-02-08 15:01:55 UTC
Also.. is this sponsored work?
Comment 91 Martin Renvoize 2019-02-08 15:02:24 UTC
Ignore that last one.. just spotted I missed the 'Sponsored-by' lines :)
Comment 92 Aleisha Amohia 2019-02-14 03:42:28 UTC
Created attachment 85073 [details] [review]
Bug 13952: (follow-up) Tests

Run:
prove -v t/db_dependent/ImportExportFramework.t and confirm all tests
pass.

I've also added a fix to remove the warns when exporting in ODS format.
Comment 93 Josef Moravec 2019-03-11 10:04:36 UTC
sorry, i got 
error: sha1 information is lacking or useless (C4/ImportExportFramework.pm).
error: could not build fake ancestor

on last patch
Comment 94 Aleisha Amohia 2019-03-12 20:27:37 UTC
Created attachment 86549 [details] [review]
Bug 13952: (follow-up) Tests

Run:
prove -v t/db_dependent/ImportExportFramework.t and confirm all tests
pass.

I've also added a fix to remove the warns when exporting in ODS format.
Comment 95 Michal Denar 2019-03-12 20:35:10 UTC
Created attachment 86550 [details] [review]
Bug 13952: Import and export authority types

This patch amends C4::ImportExportFramework to work for authority types as well as MARC frameworks.
New file: admin/import_export_authtype.pl
Update: Ensuring we are passing the right column to the right tables.
Update2: Making the error messages the same to be consistent with patch on Bug 15665
Update3: Fixing merge conflicts
Update4: Fixing merge conflicts and removing tabs
Update5: Getting rid of warns, making sure Import and Export of default
authority will work
Update6: Merge conflicts and making sure export of default auth type
works
Update7: Fixing merge conflicts and updating buttons to bootstrap3

To test:
1) Go to Admin -> Authority types
2) Confirm there are two new columns 'Export' and 'Import' in the table
3) Click 'Export' on an existing authority type and choose a file type, click 'Export'
4) Confirm that the authority type is exported as your chosen file type. Save the file
5) Create a new authority type
6) Import into your new authority type using the file you just exported
7) Confirm you are taken to auth_tag_structure.pl
8) Go back to Authority types
9) Export your new authority type. View the exported file and confirm
the authtypecode has been updated to match the code you set for the new
auth type
10) Click 'Import' next to any existing authority type and attempt to import a file that is not XML, CSV or ODS. Confirm that this fails and you are asked to import a file of the correct file type
11) Confirm Export and Import work for the Default authority type
12) Go to Admin -> MARC bibliographic framework
13) Confirm that the 'Export' and 'Import' functions still work here as well

Sponsored-by: Catalyst IT

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Michal Denar <black23@gmail.com>
Comment 96 Michal Denar 2019-03-12 20:35:15 UTC
Created attachment 86551 [details] [review]
Bug 13952: (follow-up) Fixing authority type import

This patch fixes the comments from Comment 29 and the import
functionality. You should now be able to import an exported file without
editing the file at all and the authority type code will be overwritten
in the file (same behaviour as biblio frameworks).

Sponsored-by: Catalyst IT

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 97 Michal Denar 2019-03-12 20:35:20 UTC
Created attachment 86552 [details] [review]
Bug 13952: (follow-up) Fixing import for default auth type

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 98 Michal Denar 2019-03-12 20:35:25 UTC
Created attachment 86553 [details] [review]
Bug 13952: (follow-up) Fixing export on default biblio framework

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 99 Michal Denar 2019-03-12 20:35:30 UTC
Created attachment 86554 [details] [review]
Bug 13952: (follow-up) template filters, moving js to separate file

This patch adds filters to the template file and moves the JS into a
separate file.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 100 Michal Denar 2019-03-12 20:35:35 UTC
Created attachment 86555 [details] [review]
Bug 13952: (follow-up) Tests

Run:
prove -v t/db_dependent/ImportExportFramework.t and confirm all tests
pass.

I've also added a fix to remove the warns when exporting in ODS format.

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 101 Josef Moravec 2019-03-13 12:51:58 UTC
Sorry, but still not ready,

Export looks OK, but import not.

When I try to import xml or ods, i end up with all fields deleted

When I try to import CSV I see error message, that framework could not be imported. But automated test pass...