Description
Jonathan Druart
2013-09-10 11:46:58 UTC
Created attachment 20957 [details] [review] 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. 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). (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... :) Created attachment 21376 [details] [review] 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. >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.
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). Another suggestion, maybe rename the sql profile to: "issues to claim" (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. Created attachment 21402 [details] [review] 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.
>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.
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". This bug depends on bug10853 which is in status Passed QA but the patches for it do not apply cleanly I think we need this one to make sense of the other depending bug - can I please get a sign off? Created attachment 21605 [details] [review] [SIGNED-OFF] 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 Created attachment 21607 [details] [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 :) Created attachment 21608 [details] [review] 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 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. Created attachment 21610 [details] [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 :) 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. (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 :) Hmpf :) It's ok, but maybe we should fix the wording a bit then? Created attachment 21654 [details] [review] Bug 10854: Followup: Small changes to the template Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> 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. Pushed to master. Thanks, Jonathan! |