Bug 10854 - Choose a CSV profile for exporting serial claims
Summary: Choose a CSV profile for exporting serial claims
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 10853
Blocks: 11168
  Show dependency treegraph
 
Reported: 2013-09-10 11:46 UTC by Jonathan Druart
Modified: 2014-08-27 13:09 UTC (History)
6 users (show)

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


Attachments
Bug 10854: Export serial claims with a csv profile. (9.74 KB, patch)
2013-09-10 13:29 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10854: Export serial claims with a csv profile. (9.79 KB, patch)
2013-09-23 13:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10854: Export serial claims with a csv profile. (10.83 KB, patch)
2013-09-24 09:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 10854: Export serial claims with a csv profile. (11.29 KB, patch)
2013-09-29 18:18 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 10854: Follow-up - Some changes to the template (1.93 KB, patch)
2013-09-29 20:16 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 10854: Export serial claims with a csv profile. (11.28 KB, patch)
2013-09-29 20:16 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 10854: Export serial claims with a csv profile. (11.45 KB, patch)
2013-09-29 20:21 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 10854: Follow-up - Some changes to the template (1.93 KB, patch)
2013-09-29 20:22 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 10854: Followup: Small changes to the template (1.50 KB, patch)
2013-09-30 09:28 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2013-09-10 11:46:58 UTC
Bug 10853 allows to create CSV profile with DB fields.
This feature will allow to select a defined CSV profile for serial claims.
Comment 1 Jonathan Druart 2013-09-10 13:29:33 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2013-09-22 20:21:22 UTC
Some notes:

1) This will conflict with bug 7298 which is close to pass QA but currently failed qa for serials/lateissues-excel.pl being renamed.

2) As noted on 10853 I think we need a way to prevent this CSV profile from showing up in OPAC and the staff client where it does not work. One solution I could think of is adding a 'module' column or similar to indicate where the profile should be displayed.

3) There is an error in the databaseupdate SQL statement:
[Sun Sep 22 22:15:20 2013] updatedatabase.pl: DBD::mysql::db do failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '"SUPPLIER=aqbooksellers.name|TITLE=subscription.title|ISSUE NUMBER=serial.serial' at line 1 at /home/katrin/kohaclone/installer/data/mysql/updatedatabase.pl line 7175.

Applying the SQL directly to my database worked - not sure what's wrong here.
INSERT IGNORE is not needed, but instead we'd need another way to prevent duplicates - the profile column is not unique.

Failing QA to get some feedback about 2) and 3).
Comment 3 Jonathan Druart 2013-09-23 13:16:21 UTC
(In reply to Katrin Fischer from comment #2)
> Some notes:
> 
> 1) This will conflict with bug 7298 which is close to pass QA but currently
> failed qa for serials/lateissues-excel.pl being renamed.

Bug 7298 is on the pending bugs list for 18 months ago. I don't want to mark a feature dependent on it.
I will provide a followup when it will be pushed.

> 2) As noted on 10853 I think we need a way to prevent this CSV profile from
> showing up in OPAC and the staff client where it does not work. One solution
> I could think of is adding a 'module' column or similar to indicate where
> the profile should be displayed.

I provided a patch on bug 10853.

> 3) There is an error in the databaseupdate SQL statement:
> [Sun Sep 22 22:15:20 2013] updatedatabase.pl: DBD::mysql::db do failed: You
> have an error in your SQL syntax; check the manual that corresponds to your
> MySQL server version for the right syntax to use near
> '"SUPPLIER=aqbooksellers.name|TITLE=subscription.title|ISSUE
> NUMBER=serial.serial' at line 1 at
> /home/katrin/kohaclone/installer/data/mysql/updatedatabase.pl line 7175.

Hum, it looks like I did not finish my sql query... :)
Comment 4 Jonathan Druart 2013-09-23 13:16:34 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2013-09-23 13:26:47 UTC
>Bug 7298 is on the pending bugs list for 18 months ago.

... I know, just noting, so we don't forget as I had just tested it that day. 
The problem with exporting CSV and the untranslatable headers could be fixed long term by this development being used in other places as well.
Comment 6 Katrin Fischer 2013-09-23 22:06:30 UTC
Hi Jonathan,

there are a few problems with this:
1) Using the "Export item data" link in each line causes a perl error message:
There is no valid csv profile given at /home/katrin/kohaclone/serials/lateissues-excel.pl line 36.

2) The CSV exported is 'semicolon separated' - That required me to change the settings for LibreOffice to detect it correctly. Is there a reason for that change? It seems the old file was comma separated.

3) Maybe for a follow up: Maybe we could make the export link an export button next to the claim button? 

Failing for 1), but please consider 2).
Comment 7 Katrin Fischer 2013-09-23 22:37:29 UTC
Another suggestion, maybe rename the sql profile to: "issues to claim"
Comment 8 Jonathan Druart 2013-09-24 09:03:00 UTC
(In reply to Katrin Fischer from comment #6)
> Hi Jonathan,

Katrin,

> there are a few problems with this:
> 1) Using the "Export item data" link in each line causes a perl error
> message:
> There is no valid csv profile given at
> /home/katrin/kohaclone/serials/lateissues-excel.pl line 36.

I will delete this column. It is useless.

> 2) The CSV exported is 'semicolon separated' - That required me to change
> the settings for LibreOffice to detect it correctly. Is there a reason for
> that change? It seems the old file was comma separated.

I don't know :) I will change it to comma.

> 3) Maybe for a follow up: Maybe we could make the export link an export
> button next to the claim button? 

Whaou, it isn't easy at all. Moreover, bug 5342 is pending for a long time ago and I don't want to rewrite it. To finish, I planned to add datatables using a server-side processing on this page (comment 16 from bug 5342)...

(In reply to Katrin Fischer from comment #7)
> Another suggestion, maybe rename the sql profile to: "issues to claim"

Will be fixed.
Comment 9 Jonathan Druart 2013-09-24 09:04:48 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2013-09-24 09:18:36 UTC

>Whaou, it isn't easy at all. Moreover, bug 5342 is pending for a long time ago >and I don't want to rewrite it. To finish, I planned to add datatables using a >server-side processing on this page (comment 16 from bug 5342)..

No worries, I agree - I wrote follow-up, but actually I was thinking about a separate bug. Owen might have some idea about this later on. Datatables could also go on a separate bug, getting this in before the deadline would be nice.
Comment 11 I'm just a bot 2013-09-29 06:12:40 UTC
Applying: Bug 10853: Add DB field export_format.type ('marc' or 'sql').
Using index info to reconstruct a base tree...
M	installer/data/mysql/updatedatabase.pl
Falling back to patching base and 3-way merge...
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Patch failed at 0001 Bug 10853: Add DB field export_format.type ('marc' or 'sql').
The copy of the patch that failed is found in:
   /home/christopher/git/koha/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 12 I'm just a bot 2013-09-29 06:12:54 UTC
This bug depends on bug10853 which is in status Passed QA but the patches for it do not apply cleanly
Comment 13 Katrin Fischer 2013-09-29 13:41:46 UTC
I think we need this one to make sense of the other depending bug - can I please get a sign off?
Comment 14 Bernardo Gonzalez Kriegel 2013-09-29 18:18:31 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2013-09-29 20:16:16 UTC Comment hidden (obsolete)
Comment 16 Katrin Fischer 2013-09-29 20:16:35 UTC Comment hidden (obsolete)
Comment 17 Katrin Fischer 2013-09-29 20:21:58 UTC
Created attachment 21609 [details] [review]
[PASSED QA] Bug 10854: Export serial claims with a csv profile.

Test plan:
- Create a csv profile (or use the default one) with a type 'sql'.
- Go to serials/claims.pl, select the wanted csv profile and click on
  the "Export selected items data".
- Verify the csv file is correctly generated.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described. No koha-qa errors

On top of 10853 (solving merge conflict)

Need to do homework to test. Add subscription and serial claim
notice.

1) Create a new CSV profile, type SQL, copy sample fields
2) Go to claims, select vendor
3) Go to Export selected items with created profile
4) CSV (in my case I use | as separator) download successfully
5) All fields present, file correct

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script.
Works as advertised, keeps existing format by porting it to
a SQL profile.
Comment 18 Katrin Fischer 2013-09-29 20:22:11 UTC Comment hidden (obsolete)
Comment 19 Katrin Fischer 2013-09-29 20:23:17 UTC
Ok, now I finally got the patches up and in the right order... getting a little late her. Jonathan, could you take a look at the follow-up? Leaving it to Passed QA as the first patch is fine alone too.
Comment 20 Jonathan Druart 2013-09-30 08:43:28 UTC
(In reply to Katrin Fischer from comment #18)
> Created attachment 21610 [details] [review] [review]
> Bug 10854: Follow-up - Some changes to the template
> 
> Changing the link to a button and a pull down, matching the
> layout for the notification pull down and send notification button.
> 
> To test:
> - Try to export claims as CSV and verify that the layout
>   change didn't break the functionality.
> - Decide if you like the change or not :)

Katrin,

This patch introduces an issue.
To reproduce:
- go on serials/claims.pl
- select a vendor with serials (claimdate should be != today)
- check 1 or + serials and export them as CSV
- refresh the page
- select a vendor
- the issues have been claimed!

It is why I left the link. Having 2 submit buttons (with different actions) on the same form is not a good idea.
I let you obsolete the patch.
Thanks to tried :)
Comment 21 Katrin Fischer 2013-09-30 08:47:17 UTC
Hmpf :) It's ok, but maybe we should fix the wording a bit then?
Comment 22 Jonathan Druart 2013-09-30 09:28:36 UTC
Created attachment 21654 [details] [review]
Bug 10854: Followup: Small changes to the template

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 23 Katrin Fischer 2013-09-30 18:18:17 UTC
I was thinking maybe not use 'items' as this is confusing in translation and we are exporting information about issues here. But not sure how to phrase it best.
Comment 24 Galen Charlton 2013-10-11 03:57:18 UTC
Pushed to master.  Thanks, Jonathan!