Bug 8612 - CSV export profile to have custom fields in export csv basket
Summary: CSV export profile to have custom fields in export csv basket
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement with 1 vote (vote)
Assignee: Rémi Mayrand-Provencher
QA Contact: Marcel de Rooy
URL:
Keywords:
: 10654 18717 (view as bug list)
Depends on:
Blocks: 18717 18720 18917 19200
  Show dependency treegraph
 
Reported: 2012-08-09 21:33 UTC by Maxime Pelletier
Modified: 2018-12-03 20:05 UTC (History)
14 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
patch that adds new syspref and uses it (5.47 KB, patch)
2012-08-09 21:36 UTC, Maxime Pelletier
Details | Diff | Splinter Review
Bug 8612 - System preference to have custom fields in exort csv basket (7.67 KB, patch)
2015-03-13 13:31 UTC, simith.doliveira
Details | Diff | Splinter Review
Bug 8612 - System preference to have custom fields in export csv basket (9.32 KB, patch)
2015-03-26 20:17 UTC, Blou
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 8612 - System preference to have custom fields in export csv basket (9.42 KB, patch)
2015-03-27 07:59 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - Cleans basket.tt by using TT blocks (3.84 KB, patch)
2015-07-29 13:09 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
Bug 8612 - Cleans basket.tt by using TT blocks (3.84 KB, patch)
2015-07-29 13:12 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
Bug 8612 - System preference to have custom fields in export csv basket (9.39 KB, patch)
2015-07-29 13:22 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
Bug 8612 - Cleans basket.tt by using TT blocks (3.84 KB, patch)
2015-07-29 13:31 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
Bug 8612 - System preference to have custom fields in export csv basket (9.39 KB, patch)
2015-09-14 18:04 UTC, Genevieve Plantin
Details | Diff | Splinter Review
Bug 8612 - Cleans basket.tt by using TT blocks (3.84 KB, patch)
2015-09-14 18:04 UTC, Genevieve Plantin
Details | Diff | Splinter Review
Bug 8612 - Clean basket.pl getting rid of subroutines with sql query and use GetBasketAsCSV instead (9.00 KB, patch)
2015-09-14 18:04 UTC, Genevieve Plantin
Details | Diff | Splinter Review
Bug 8612 - System preference to have custom fields in export csv basket (9.17 KB, patch)
2016-02-03 14:20 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
Bug 8612 - Cleans basket.tt by using TT blocks (3.78 KB, patch)
2016-02-03 14:20 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
Bug 8612 - Clean basket.pl getting rid of subroutines with sql query and use GetBasketAsCSV instead (9.00 KB, patch)
2016-02-03 14:20 UTC, Rémi Mayrand-Provencher
Details | Diff | Splinter Review
Bug 8612 - System preference to have custom fields in export csv basket (9.93 KB, patch)
2016-10-12 07:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - Cleans basket.tt by using TT blocks (4.64 KB, patch)
2016-10-12 07:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - Clean basket.pl getting rid of subroutines with sql query and use GetBasketAsCSV instead (9.15 KB, patch)
2016-10-12 07:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - Followup - use Koha::CsvProfile[s] module (see bug 15451) (2.77 KB, patch)
2016-10-12 07:08 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA followup - Make export button split button, unify button labels (3.96 KB, patch)
2016-11-01 07:55 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: Add tests for GetBasketAsCSV (2.74 KB, patch)
2016-11-01 07:55 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - System preference to have custom fields in export csv basket (10.20 KB, patch)
2016-11-01 11:43 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - Cleans basket.tt by using TT blocks (4.39 KB, patch)
2016-11-01 11:43 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - Clean basket.pl getting rid of subroutines with sql query and use GetBasketAsCSV instead (9.20 KB, patch)
2016-11-01 11:43 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - Followup - use Koha::CsvProfile[s] module (see bug 15451) (2.79 KB, patch)
2016-11-01 11:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA followup - Make export button split button, unify button labels (3.96 KB, patch)
2016-11-01 11:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: Add tests for GetBasketAsCSV (2.74 KB, patch)
2016-11-01 11:44 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA follow-up - distinguish between late issues and basket export csv profiles (7.54 KB, patch)
2016-11-02 12:19 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - System preference to have custom fields in export csv basket (10.20 KB, patch)
2016-12-09 13:28 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - Cleans basket.tt by using TT blocks (4.39 KB, patch)
2016-12-09 13:28 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - Clean basket.pl getting rid of subroutines with sql query and use GetBasketAsCSV instead (9.20 KB, patch)
2016-12-09 13:28 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - Followup - use Koha::CsvProfile[s] module (see bug 15451) (2.79 KB, patch)
2016-12-09 13:28 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA followup - Make export button split button, unify button labels (3.96 KB, patch)
2016-12-09 13:29 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: Add tests for GetBasketAsCSV (2.74 KB, patch)
2016-12-09 13:29 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA follow-up - distinguish between late issues and basket export csv profiles (7.54 KB, patch)
2016-12-09 13:29 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - System preference to have custom fields in export csv basket (10.24 KB, patch)
2017-01-14 08:03 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - Cleans basket.tt by using TT blocks (3.93 KB, patch)
2017-01-14 08:03 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - Clean basket.pl getting rid of subroutines with sql query and use GetBasketAsCSV instead (9.20 KB, patch)
2017-01-14 08:03 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - Followup - use Koha::CsvProfile[s] module (see bug 15451) (2.86 KB, patch)
2017-01-14 08:04 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA followup - Make export button split button, unify button labels (4.01 KB, patch)
2017-01-14 08:04 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: Add tests for GetBasketAsCSV (2.74 KB, patch)
2017-01-14 08:04 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA follow-up - distinguish between late issues and basket export csv profiles (7.54 KB, patch)
2017-01-14 08:04 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: Fix test due to bug 13726 (1.33 KB, patch)
2017-01-14 08:04 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: Use CSV profile for exporting basket (20.85 KB, patch)
2017-02-01 21:39 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: Use CSV profile for exporting basket (20.90 KB, patch)
2017-02-20 21:42 UTC, Mehdi Hamidi
Details | Diff | Splinter Review
Bug 8612: Use CSV profile for exporting basket (20.98 KB, patch)
2017-03-20 11:14 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA-followup - Fix test (1.23 KB, patch)
2017-03-20 11:14 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA followup - make usage and type different columns in db (13.69 KB, patch)
2017-03-20 11:15 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA followup - update dbix schema (1.48 KB, patch)
2017-03-20 11:15 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - QA followup, fix kohastructure.sql, fix typo in template (1.92 KB, patch)
2017-03-21 17:14 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: [QA Follow-up] Just a cosmetic space (1018 bytes, patch)
2017-03-23 09:55 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 8612: Use CSV profile for exporting basket (20.97 KB, patch)
2017-05-04 07:16 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA-followup - Fix test (1.23 KB, patch)
2017-05-04 07:16 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA followup - make usage and type different columns in db (13.68 KB, patch)
2017-05-04 07:16 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA followup - update dbix schema (1.48 KB, patch)
2017-05-04 07:17 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612 - QA followup, fix kohastructure.sql, fix typo in template (1.92 KB, patch)
2017-05-04 07:17 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: [QA Follow-up] Just a cosmetic space (978 bytes, patch)
2017-05-04 07:17 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA folowup - fix regular expression to do what is described in the comment (818 bytes, patch)
2017-05-04 08:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA followup - make header in CSV profile definition optional (1.08 KB, patch)
2017-05-04 08:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA followup - expand tests (1.65 KB, patch)
2017-05-04 08:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA followup - expand tests (1.65 KB, patch)
2017-05-04 13:39 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 8612: QA followup - strip white chars from csv profile definition (828 bytes, patch)
2017-05-05 08:43 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: QA followup - strip white chars from csv profile definition, expand test (1.65 KB, patch)
2017-05-09 08:05 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 8612: Use CSV profile for exporting basket (21.07 KB, patch)
2017-06-02 06:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 8612: [Follow-up] Make usage and type different columns in table (13.95 KB, patch)
2017-06-02 06:53 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 8612: [Follow-up] Update DBIx schema (1.57 KB, patch)
2017-06-02 06:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 8612: [Follow-up] Fix regular expression (1.61 KB, patch)
2017-06-02 06:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 8612: [Follow-up] Fix unit test GetBasketAsCSV.t (2.46 KB, patch)
2017-06-02 06:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 8612: [QA Follow-up] Remove two newlines from template output (2.47 KB, patch)
2017-06-02 06:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 8612: Remove warnings from tests (1.07 KB, patch)
2017-06-05 15:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8612: Update installer files for late_issues (4.27 KB, patch)
2017-06-05 15:56 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 Maxime Pelletier 2012-08-09 21:33:40 UTC
New system preference where you can write the fields you want to have in the "Export this basket as CSV" button in a basket page of the acquisition module.
This could be done in better ways, but this is what I came up with, in the time I had.
Comment 1 Maxime Pelletier 2012-08-09 21:36:48 UTC Comment hidden (obsolete)
Comment 2 Maxime Pelletier 2013-03-13 17:39:10 UTC
This needs to be completly redone because the basket csv export is now in a template.
Comment 3 Katrin Fischer 2014-05-18 19:27:18 UTC
Hi Maxime, maybe you could consider doing this similar to the new option for exporting late serial issues, where the export format is defined as a CSV profile. This could also allow to have multiple formats instead of only one (see bug 10654)
Comment 4 Katrin Fischer 2014-08-27 13:09:36 UTC
Just a quick update: you can use CSV profiles now to define the export format for late serials - it would be nice if this could work similarly, so we don't add too many different ways of how to configure the CSV exports. (bug 10854)
Comment 5 simith.doliveira 2015-03-13 13:31:40 UTC Comment hidden (obsolete)
Comment 6 Blou 2015-03-26 20:17:22 UTC Comment hidden (obsolete)
Comment 7 Blou 2015-03-26 20:19:30 UTC
Comment on attachment 36860 [details] [review]
Bug 8612 - System preference to have custom fields in exort csv basket

The left panel was removed by this patch, and it when used on a closed basket it caused a crash.  These are fixed by the latest version.
Comment 8 Michal Denar 2015-03-26 21:11:54 UTC
Hello Biou,
thanks for this pattch. I tested it in sandbox but get some messs with some character for example "í". But not for all characters in natioanl alphabet. We need more tests. 

I confirm that patch works with open or close basket.

Mike
Comment 9 Josef Moravec 2015-03-27 07:59:18 UTC Comment hidden (obsolete)
Comment 10 Josef Moravec 2015-03-31 07:02:57 UTC
*** Bug 10654 has been marked as a duplicate of this bug. ***
Comment 11 Jonathan Druart 2015-04-17 14:12:41 UTC
Comment on attachment 37293 [details] [review]
[SIGNED-OFF] Bug 8612 - System preference to have custom fields in export csv basket

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

::: acqui/basket.pl
@@ +197,5 @@
>          -type       => 'text/csv',
>          -attachment => 'basket' . $basket->{'basketno'} . '.csv',
>      );
> +    if ( $query->param('csv_profile') eq 'default'){
> +        print GetBasketAsCSV($query->param('basketno'), $query);

Actually this subroutine should take the csv profile in parameter, don't you think?

@@ +531,4 @@
>      return \%line;
>  }
>  
> +sub get_basket_DB_info{

Don't you have something in the module to do that? (I didn't check).
In all cases, you cannot add it here in the pl. Yes that means tests.

::: koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
@@ +190,5 @@
>                      [% END %]
> +                       <div class="btn-group">
> +                       <fieldset class="action">
> +                          <label for="csv_code">Select CSV profile:</label>
> +                          <select id="csv_profile_for_export">

This block is c/p twice, should be in a TT BLOCK.
Comment 12 Rémi Mayrand-Provencher 2015-07-29 12:57:17 UTC
Hi Jonathan,

> ::: acqui/basket.pl
> @@ +197,5 @@
> >          -type       => 'text/csv',
> >          -attachment => 'basket' . $basket->{'basketno'} . '.csv',
> >      );
> > +    if ( $query->param('csv_profile') eq 'default'){
> > +        print GetBasketAsCSV($query->param('basketno'), $query);
> 
> Actually this subroutine should take the csv profile in parameter, don't you
> think?

Could you be more specific about what are the changes you would like to see here?

Thank you,

(In reply to Jonathan Druart from comment #11)
> Comment on attachment 37293 [details] [review] [review]
> [SIGNED-OFF] Bug 8612 - System preference to have custom fields in export
> csv basket
> 
> Review of attachment 37293 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: acqui/basket.pl
> @@ +197,5 @@
> >          -type       => 'text/csv',
> >          -attachment => 'basket' . $basket->{'basketno'} . '.csv',
> >      );
> > +    if ( $query->param('csv_profile') eq 'default'){
> > +        print GetBasketAsCSV($query->param('basketno'), $query);
> 
> Actually this subroutine should take the csv profile in parameter, don't you
> think?
> 
> @@ +531,4 @@
> >      return \%line;
> >  }
> >  
> > +sub get_basket_DB_info{
> 
> Don't you have something in the module to do that? (I didn't check).
> In all cases, you cannot add it here in the pl. Yes that means tests.
> 
> ::: koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
> @@ +190,5 @@
> >                      [% END %]
> > +                       <div class="btn-group">
> > +                       <fieldset class="action">
> > +                          <label for="csv_code">Select CSV profile:</label>
> > +                          <select id="csv_profile_for_export">
> 
> This block is c/p twice, should be in a TT BLOCK.
Comment 13 Rémi Mayrand-Provencher 2015-07-29 13:09:44 UTC Comment hidden (obsolete)
Comment 14 Rémi Mayrand-Provencher 2015-07-29 13:12:54 UTC Comment hidden (obsolete)
Comment 15 Rémi Mayrand-Provencher 2015-07-29 13:22:46 UTC Comment hidden (obsolete)
Comment 16 Rémi Mayrand-Provencher 2015-07-29 13:31:41 UTC Comment hidden (obsolete)
Comment 17 Jonathan Druart 2015-08-04 08:22:46 UTC
(In reply to Rémi Mayrand-Provencher from comment #12)
> Hi Jonathan,
> 
> > ::: acqui/basket.pl
> > @@ +197,5 @@
> > >          -type       => 'text/csv',
> > >          -attachment => 'basket' . $basket->{'basketno'} . '.csv',
> > >      );
> > > +    if ( $query->param('csv_profile') eq 'default'){
> > > +        print GetBasketAsCSV($query->param('basketno'), $query);
> > 
> > Actually this subroutine should take the csv profile in parameter, don't you
> > think?
> 
> Could you be more specific about what are the changes you would like to see
> here?

You added 2 subroutines to the script acqui/basket.pl to build a CSV file.
But the subroutine GetBasketAsCSV already does it.
I think you should modify this subroutine to do what you want to do: export a basket as CSV given a specific CSV profile.
The call would be:
  my $csv = GetBasketAsCSV($basketno, $query, $csv_profile_id);

Yes, it's a little bit more work, but more robust and reusable :)
Comment 18 Genevieve Plantin 2015-09-14 18:04:26 UTC Comment hidden (obsolete)
Comment 19 Genevieve Plantin 2015-09-14 18:04:38 UTC Comment hidden (obsolete)
Comment 20 Genevieve Plantin 2015-09-14 18:04:48 UTC Comment hidden (obsolete)
Comment 21 Josef Moravec 2015-11-27 12:12:49 UTC
Could you please rebase and change the icon to Font Awesome icon? 
See http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13696
Comment 22 Rémi Mayrand-Provencher 2016-02-03 14:20:03 UTC
Created attachment 47591 [details] [review]
Bug 8612 - System preference to have custom fields in export csv basket

This patch allows the user use a CSV export profile to create the fields to export the basket as CSV in a basket page.

To test:

1) Apply the patch;
2) Create a CSV export profile - type sql -  Home › Tools › CSV export profiles
  ex: biblionumber=biblio.biblionumber|auteur=biblio.author|titre=biblio.title|date=biblioitems.copyrightdate|editeur=biblioitems.publishercode|isbn=biblioitems.isbn|quantite=aqorders.quantity|prix=aqorders.rrp|panier=aqorders.basketno
3) Create a new basket and add an order to the basket
4) Validate: the text "Select CSV profile:" and a dropdown in the basket view
5) In the new dropdown, select the CSV export profile created
6) Validate the CSV file.
7) Do the same validation with a closed basket.
 a) close the basket
 b) View the basket
 c) validate that there is a dropdown
 d) test it with an export

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Works as expected for me
Comment 23 Rémi Mayrand-Provencher 2016-02-03 14:20:09 UTC
Created attachment 47592 [details] [review]
Bug 8612 - Cleans basket.tt by using TT blocks
Comment 24 Rémi Mayrand-Provencher 2016-02-03 14:20:16 UTC
Created attachment 47593 [details] [review]
Bug 8612 - Clean basket.pl getting rid of subroutines with sql query and use GetBasketAsCSV instead

GetBasketAsCSV now has a new argument the profile_csv_id, when this param is defined, it loads the profile_content, searches the value of the predefined fields and assembles the data to create the output of the csv file.
Comment 25 Rémi Mayrand-Provencher 2016-02-03 14:21:18 UTC
Fixed conflicts and changed icons to font awesome
Comment 26 Aleisha Amohia 2016-02-05 00:45:14 UTC
This didn't work properly for me - I selected the pipe | as my CSV separator and copied your data from step 2 but the CSV file did not separate my files on pipes and separated on commas instead.
Comment 27 Josef Moravec 2016-10-12 07:08:31 UTC
Created attachment 56222 [details] [review]
Bug 8612 - System preference to have custom fields in export csv basket

This patch allows the user use a CSV export profile to create the fields to export the basket as CSV in a basket page.

To test:

1) Apply the patch;
2) Create a CSV export profile - type sql -  Home › Tools › CSV export profiles
  ex: biblionumber=biblio.biblionumber|auteur=biblio.author|titre=biblio.title|date=biblioitems.copyrightdate|editeur=biblioitems.publishercode|isbn=biblioitems.isbn|quantite=aqorders.quantity|prix=aqorders.rrp|panier=aqorders.basketno
3) Create a new basket and add an order to the basket
4) Validate: the text "Select CSV profile:" and a dropdown in the basket view
5) In the new dropdown, select the CSV export profile created
6) Validate the CSV file.
7) Do the same validation with a closed basket.
 a) close the basket
 b) View the basket
 c) validate that there is a dropdown
 d) test it with an export

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Works as expected for me

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 28 Josef Moravec 2016-10-12 07:08:40 UTC
Created attachment 56223 [details] [review]
Bug 8612 - Cleans basket.tt by using TT blocks

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 29 Josef Moravec 2016-10-12 07:08:48 UTC
Created attachment 56224 [details] [review]
Bug 8612 - Clean basket.pl getting rid of subroutines with sql query and use GetBasketAsCSV instead

GetBasketAsCSV now has a new argument the profile_csv_id, when this param is defined, it loads the profile_content, searches the value of the predefined fields and assembles the data to create the output of the csv file.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 30 Josef Moravec 2016-10-12 07:08:53 UTC
Created attachment 56225 [details] [review]
Bug 8612 - Followup - use Koha::CsvProfile[s] module (see bug 15451)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 31 Josef Moravec 2016-10-12 07:12:43 UTC
I've rebased patch set on master.

Aleisha, I tried the pipe as separator too, and it worked for me well, so I changed status back to signed off.

If it's still not working for you, could you please add more details?
Comment 32 Jonathan Druart 2016-10-25 08:56:17 UTC
No tests provided.
Comment 33 Katrin Fischer 2016-10-26 17:49:55 UTC
Did a quick code review:

1) Could we make this a split button maybe (like the one in cataloguing for z39.50 import)? (no blocker)

2) The button itself is labelled differently on a closed basket and an open basket. Should be the same label for both. I'd suggest something in the middle between both versions "Export as CSV"?

2) Please provide tests for changes to GetBasketAsCSV.
Comment 34 Josef Moravec 2016-11-01 07:55:33 UTC
Created attachment 57012 [details] [review]
Bug 8612: QA followup - Make export button split button, unify button labels
Comment 35 Josef Moravec 2016-11-01 07:55:51 UTC
Created attachment 57013 [details] [review]
Bug 8612: Add tests for GetBasketAsCSV
Comment 36 Josef Moravec 2016-11-01 08:01:53 UTC
(In reply to Katrin Fischer from comment #33)
> Did a quick code review:
> 
> 1) Could we make this a split button maybe (like the one in cataloguing for
> z39.50 import)? (no blocker)

Good point, I've done it

> 
> 2) The button itself is labelled differently on a closed basket and an open
> basket. Should be the same label for both. I'd suggest something in the
> middle between both versions "Export as CSV"?

Fixed

> 
> 2) Please provide tests for changes to GetBasketAsCSV.

Test added

As have little experiences with writing test, feel free to point me to the right direction if it's not written as it should be. Thanks
Comment 37 Jonathan Druart 2016-11-01 11:21:43 UTC
Apply? [(y)es, (n)o, (i)nteractive] Applying: Bug 8612 - System preference to have custom fields in export csv basket
Using index info to reconstruct a base tree...
M       acqui/basket.pl
M       koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
Auto-merging acqui/basket.pl
CONFLICT (content): Merge conflict in acqui/basket.pl
Comment 38 Josef Moravec 2016-11-01 11:43:45 UTC
Created attachment 57024 [details] [review]
Bug 8612 - System preference to have custom fields in export csv basket

This patch allows the user use a CSV export profile to create the fields to export the basket as CSV in a basket page.

To test:

1) Apply the patch;
2) Create a CSV export profile - type sql -  Home › Tools › CSV export profiles
  ex: biblionumber=biblio.biblionumber|auteur=biblio.author|titre=biblio.title|date=biblioitems.copyrightdate|editeur=biblioitems.publishercode|isbn=biblioitems.isbn|quantite=aqorders.quantity|prix=aqorders.rrp|panier=aqorders.basketno
3) Create a new basket and add an order to the basket
4) Validate: the text "Select CSV profile:" and a dropdown in the basket view
5) In the new dropdown, select the CSV export profile created
6) Validate the CSV file.
7) Do the same validation with a closed basket.
 a) close the basket
 b) View the basket
 c) validate that there is a dropdown
 d) test it with an export

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Works as expected for me

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 39 Josef Moravec 2016-11-01 11:43:52 UTC
Created attachment 57025 [details] [review]
Bug 8612 - Cleans basket.tt by using TT blocks

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 40 Josef Moravec 2016-11-01 11:43:59 UTC
Created attachment 57026 [details] [review]
Bug 8612 - Clean basket.pl getting rid of subroutines with sql query and use GetBasketAsCSV instead

GetBasketAsCSV now has a new argument the profile_csv_id, when this param is defined, it loads the profile_content, searches the value of the predefined fields and assembles the data to create the output of the csv file.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 41 Josef Moravec 2016-11-01 11:44:24 UTC
Created attachment 57027 [details] [review]
Bug 8612 - Followup - use Koha::CsvProfile[s] module (see bug 15451)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 42 Josef Moravec 2016-11-01 11:44:29 UTC
Created attachment 57028 [details] [review]
Bug 8612: QA followup - Make export button split button, unify button labels
Comment 43 Josef Moravec 2016-11-01 11:44:34 UTC
Created attachment 57029 [details] [review]
Bug 8612: Add tests for GetBasketAsCSV
Comment 44 Josef Moravec 2016-11-01 11:45:05 UTC
Sorry, rebased on master
Comment 45 Katrin Fischer 2016-11-01 22:53:36 UTC
I really like the new split button! Tests pass.

Sorry, but noticed this now for the first time:
When you have a CSV profile defined for your late issues export, it will also show up as export option for the CSV export in acquisitions. I think this could get confusing as both will require different fields from the database.

I'd really like a way to indicate what the CSV profile is to be used for. I am hesitating to pass this as is, as I think it will be harder to do this later on. Once the libraries have added profiles for both, there will be no way to assign them automatically to the correct module. If we do it now, we can safely assume that all SQL profiles are used for exporting late issues. Hope that makes sense?
Comment 46 Josef Moravec 2016-11-02 12:19:42 UTC
Created attachment 57079 [details] [review]
Bug 8612: QA follow-up - distinguish between late issues and basket export csv profiles

tatus: Signed Off
Comment 47 Josef Moravec 2016-11-02 12:23:22 UTC
(In reply to Katrin Fischer from comment #45)
> 
> I'd really like a way to indicate what the CSV profile is to be used for. I
> am hesitating to pass this as is, as I think it will be harder to do this
> later on. Once the libraries have added profiles for both, there will be no
> way to assign them automatically to the correct module. If we do it now, we
> can safely assume that all SQL profiles are used for exporting late issues.
> Hope that makes sense?

It absolutely makes sense, I agree with you. I've submitted follow-up, to you think I'm on right way, or should I take another approach to this problem?
Comment 48 Josef Moravec 2016-12-09 13:28:27 UTC
Created attachment 58073 [details] [review]
Bug 8612 - System preference to have custom fields in export csv basket

This patch allows the user use a CSV export profile to create the fields to export the basket as CSV in a basket page.

To test:

1) Apply the patch;
2) Create a CSV export profile - type sql -  Home › Tools › CSV export profiles
  ex: biblionumber=biblio.biblionumber|auteur=biblio.author|titre=biblio.title|date=biblioitems.copyrightdate|editeur=biblioitems.publishercode|isbn=biblioitems.isbn|quantite=aqorders.quantity|prix=aqorders.rrp|panier=aqorders.basketno
3) Create a new basket and add an order to the basket
4) Validate: the text "Select CSV profile:" and a dropdown in the basket view
5) In the new dropdown, select the CSV export profile created
6) Validate the CSV file.
7) Do the same validation with a closed basket.
 a) close the basket
 b) View the basket
 c) validate that there is a dropdown
 d) test it with an export

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Works as expected for me

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 49 Josef Moravec 2016-12-09 13:28:35 UTC
Created attachment 58074 [details] [review]
Bug 8612 - Cleans basket.tt by using TT blocks

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 50 Josef Moravec 2016-12-09 13:28:51 UTC
Created attachment 58075 [details] [review]
Bug 8612 - Clean basket.pl getting rid of subroutines with sql query and use GetBasketAsCSV instead

GetBasketAsCSV now has a new argument the profile_csv_id, when this param is defined, it loads the profile_content, searches the value of the predefined fields and assembles the data to create the output of the csv file.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 51 Josef Moravec 2016-12-09 13:28:57 UTC
Created attachment 58076 [details] [review]
Bug 8612 - Followup - use Koha::CsvProfile[s] module (see bug 15451)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 52 Josef Moravec 2016-12-09 13:29:03 UTC
Created attachment 58077 [details] [review]
Bug 8612: QA followup - Make export button split button, unify button labels
Comment 53 Josef Moravec 2016-12-09 13:29:10 UTC
Created attachment 58078 [details] [review]
Bug 8612: Add tests for GetBasketAsCSV
Comment 54 Josef Moravec 2016-12-09 13:29:19 UTC
Created attachment 58079 [details] [review]
Bug 8612: QA follow-up - distinguish between late issues and basket export csv profiles
Comment 55 Josef Moravec 2016-12-09 13:29:43 UTC
Just rebased on master
Comment 56 Josef Moravec 2017-01-14 08:03:41 UTC
Created attachment 58985 [details] [review]
Bug 8612 - System preference to have custom fields in export csv basket

This patch allows the user use a CSV export profile to create the fields to export the basket as CSV in a basket page.

To test:

1) Apply the patch;
2) Create a CSV export profile - type sql -  Home › Tools › CSV export profiles
  ex: biblionumber=biblio.biblionumber|auteur=biblio.author|titre=biblio.title|date=biblioitems.copyrightdate|editeur=biblioitems.publishercode|isbn=biblioitems.isbn|quantite=aqorders.quantity|prix=aqorders.rrp|panier=aqorders.basketno
3) Create a new basket and add an order to the basket
4) Validate: the text "Select CSV profile:" and a dropdown in the basket view
5) In the new dropdown, select the CSV export profile created
6) Validate the CSV file.
7) Do the same validation with a closed basket.
 a) close the basket
 b) View the basket
 c) validate that there is a dropdown
 d) test it with an export

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Works as expected for me

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 57 Josef Moravec 2017-01-14 08:03:51 UTC
Created attachment 58986 [details] [review]
Bug 8612 - Cleans basket.tt by using TT blocks

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 58 Josef Moravec 2017-01-14 08:03:58 UTC
Created attachment 58987 [details] [review]
Bug 8612 - Clean basket.pl getting rid of subroutines with sql query and use GetBasketAsCSV instead

GetBasketAsCSV now has a new argument the profile_csv_id, when this param is defined, it loads the profile_content, searches the value of the predefined fields and assembles the data to create the output of the csv file.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 59 Josef Moravec 2017-01-14 08:04:05 UTC
Created attachment 58988 [details] [review]
Bug 8612 - Followup - use Koha::CsvProfile[s] module (see bug 15451)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 60 Josef Moravec 2017-01-14 08:04:12 UTC
Created attachment 58989 [details] [review]
Bug 8612: QA followup - Make export button split button, unify button labels
Comment 61 Josef Moravec 2017-01-14 08:04:18 UTC
Created attachment 58990 [details] [review]
Bug 8612: Add tests for GetBasketAsCSV
Comment 62 Josef Moravec 2017-01-14 08:04:24 UTC
Created attachment 58991 [details] [review]
Bug 8612: QA follow-up - distinguish between late issues and basket export csv profiles
Comment 63 Josef Moravec 2017-01-14 08:04:34 UTC
Created attachment 58992 [details] [review]
Bug 8612: Fix test due to bug 13726
Comment 64 Josef Moravec 2017-01-14 08:14:34 UTC
Rebased on master, added another followup. Asking for new sign off - too many followups and conflicts with new work pushed to master. Also note, that the test plan should be amended. 

New test plan:

1) Apply the patches
2) Go to Tools › CSV export profiles and create a profile of type "SQL for basket export in acquisition"
  example: biblionumber=biblio.biblionumber|auteur=biblio.author|titre=biblio.title|date=biblioitems.copyrightdate|editeur=biblioitems.publishercode|isbn=biblioitems.isbn|quantite=aqorders.quantity|prix=aqorders.rrp|panier=aqorders.basketno
3) In acquisition module, create a new basket and add an order to the basket
4) On basket detail page, there should be the split button labelled "Export to CSV"
5) Try to use the button and export CSV with your CSV profile you defined in step 2
6) Validate the CSV file.
7) Repeat 4-6 with a closed basket.
 a) close the basket
 b) View the basket
 c) validate that there is an export button
 d) test it with an export
8) prove t/db_dependent/Acquisition/GetBasketAsCSV.t t/db_dependent/Koha/CsvProfiles.t
Comment 65 Blou 2017-01-23 21:35:28 UTC
Josef,
I'd be happy to have someone in our team test and sign this, but right now I get a 
Applying: Bug 8612 - Followup - use Koha::CsvProfile[s] module (see bug 15451)
fatal: sha1 information is lacking or useless (C4/Acquisition.pm).

Damn sha1...
Making one big patch out of it would probably limit future rebases, btw, since a lot of these do-undo-redo the same code...

Reminder for the initial work:

****
Sponsored by : CCSR
****
Comment 66 Josef Moravec 2017-02-01 21:39:10 UTC
Created attachment 59757 [details] [review]
Bug 8612: Use CSV profile for exporting basket

This patch allows the user to use a CSV export profile to create the fields to export the basket as CSV in a basket page.

Test plan:
1) Apply the patch
2) Go to Tools › CSV export profiles and create a profile of type "SQL for basket export in acquisition"
  example:
  biblionumber=biblio.biblionumber|auteur=biblio.author|titre=biblio.title|date=biblioitems.copyrightdate|editeur=biblioitems.publishercode|isbn=biblioitems.isbn|quantite=aqorders.quantity|prix=aqorders.rrp|panier=aqorders.basketno
3) In acquisition module, create a new basket and add an order to the basket
4) On basket detail page, there should be the split button labelled "Export to CSV"
5) Try to use the button and export CSV with your CSV profile you defined in step 2
6) Validate the CSV file.
7) Repeat 4-6 with a closed basket.
    a) close the basket
    b) View the basket
    c) validate that there is an export button
    d) test it with an export
8) prove t/db_dependent/Acquisition/GetBasketAsCSV.t t/db_dependent/Koha/CsvProfiles.t

Initial work:

Sponsored by: CCSR

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 67 Josef Moravec 2017-02-01 21:40:42 UTC
(In reply to Blou from comment #65)
> Josef,
> I'd be happy to have someone in our team test and sign this, but right now I
> get a 
> Applying: Bug 8612 - Followup - use Koha::CsvProfile[s] module (see bug
> 15451)
> fatal: sha1 information is lacking or useless (C4/Acquisition.pm).
> 
> Damn sha1...
> Making one big patch out of it would probably limit future rebases, btw,
> since a lot of these do-undo-redo the same code...
> 
> Reminder for the initial work:
> 
> ****
> Sponsored by : CCSR
> ****

I squashed the patches.
Comment 68 Mehdi Hamidi 2017-02-20 21:42:21 UTC
Created attachment 60480 [details] [review]
Bug 8612: Use CSV profile for exporting basket

This patch allows the user to use a CSV export profile to create the fields to export the basket as CSV in a basket page.

Test plan:
1) Apply the patch
2) Go to Tools › CSV export profiles and create a profile of type "SQL for basket export in acquisition"
  example:
  biblionumber=biblio.biblionumber|auteur=biblio.author|titre=biblio.title|date=biblioitems.copyrightdate|editeur=biblioitems.publishercode|isbn=biblioitems.isbn|quantite=aqorders.quantity|prix=aqorders.rrp|panier=aqorders.basketno
3) In acquisition module, create a new basket and add an order to the basket
4) On basket detail page, there should be the split button labelled "Export to CSV"
5) Try to use the button and export CSV with your CSV profile you defined in step 2
6) Validate the CSV file.
7) Repeat 4-6 with a closed basket.
    a) close the basket
    b) View the basket
    c) validate that there is an export button
    d) test it with an export
8) prove t/db_dependent/Acquisition/GetBasketAsCSV.t t/db_dependent/Koha/CsvProfiles.t

Initial work:

Sponsored by: CCSR

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: mehdi <mehdi.hamidi@inlibro.com>
Comment 69 Jonathan Druart 2017-02-24 07:49:30 UTC
I have the feeling that there is a design issue. I'd have let the type="sql" but add another DB field ('usage' or similar) to identify how the profile can be used.
Comment 70 Marcel de Rooy 2017-02-24 09:24:41 UTC
(In reply to Jonathan Druart from comment #69)
> I have the feeling that there is a design issue. I'd have let the type="sql"
> but add another DB field ('usage' or similar) to identify how the profile
> can be used.

I agree with Jonathan.
Although I acknowledge that this patch already has quite a history already.

Export format is the name of the table for the CSV profiles (this is already confusing btw). Actually, csv profiles would be a better name. 
The column type should be either marc or sql: we refer to marc fields or database columns.
But yes, we need an additional column where to use which profile. Probably we can start here with the simple list we have now. Could be expanded later..

About t/db_dependent/Acquisition/GetBasketAsCSV.t:
Remove the dbh statements. You do not need them. You use schema txn..
You pass a new CGI object in your test. You probably can; just wondering if you should mock C4::Languages::getlanguage and not use CGI here.
Typo defautl
Comment 71 Josef Moravec 2017-03-20 11:14:49 UTC
Created attachment 61264 [details] [review]
Bug 8612: Use CSV profile for exporting basket

This patch allows the user to use a CSV export profile to create the fields to export the basket as CSV in a basket page.

Test plan:
1) Apply the patch
2) Go to Tools › CSV export profiles and create a profile of type "SQL for basket export in acquisition"
  example:
  biblionumber=biblio.biblionumber|auteur=biblio.author|titre=biblio.title|date=biblioitems.copyrightdate|editeur=biblioitems.publishercode|isbn=biblioitems.isbn|quantite=aqorders.quantity|prix=aqorders.rrp|panier=aqorders.basketno
3) In acquisition module, create a new basket and add an order to the basket
4) On basket detail page, there should be the split button labelled "Export to CSV"
5) Try to use the button and export CSV with your CSV profile you defined in step 2
6) Validate the CSV file.
7) Repeat 4-6 with a closed basket.
    a) close the basket
    b) View the basket
    c) validate that there is an export button
    d) test it with an export
8) prove t/db_dependent/Acquisition/GetBasketAsCSV.t t/db_dependent/Koha/CsvProfiles.t

Initial work:

Sponsored by: CCSR

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: mehdi <mehdi.hamidi@inlibro.com>
Comment 72 Josef Moravec 2017-03-20 11:14:59 UTC
Created attachment 61265 [details] [review]
Bug 8612: QA-followup - Fix test
Comment 73 Josef Moravec 2017-03-20 11:15:06 UTC
Created attachment 61266 [details] [review]
Bug 8612: QA followup - make usage and type different columns in db
Comment 74 Josef Moravec 2017-03-20 11:15:12 UTC
Created attachment 61267 [details] [review]
Bug 8612: QA followup - update dbix schema
Comment 75 Josef Moravec 2017-03-20 11:21:00 UTC
(In reply to Marcel de Rooy from comment #70)
> (In reply to Jonathan Druart from comment #69)
> > I have the feeling that there is a design issue. I'd have let the type="sql"
> > but add another DB field ('usage' or similar) to identify how the profile
> > can be used.
> 
> I agree with Jonathan.
> Although I acknowledge that this patch already has quite a history already.
> 
> Export format is the name of the table for the CSV profiles (this is already
> confusing btw). Actually, csv profiles would be a better name. 
> The column type should be either marc or sql: we refer to marc fields or
> database columns.
> But yes, we need an additional column where to use which profile. Probably
> we can start here with the simple list we have now. Could be expanded later..
> 
> About t/db_dependent/Acquisition/GetBasketAsCSV.t:
> Remove the dbh statements. You do not need them. You use schema txn..
> You pass a new CGI object in your test. You probably can; just wondering if
> you should mock C4::Languages::getlanguage and not use CGI here.
> Typo defautl

So, I rebased the patch on top of current master and added some followups.

Changes made - 
 - revert the changes in csv profile types so there are again only two - marc and sql
 - added new db column used_for, which should say where the profile could be used
  - fix typo and remove dbh from test GetBasketAsCSV.t

How do you think the mocked getlanguage could be used? The soubroutine GetBasketAsCSV does need the CGI object as a param...
Comment 76 Marcel de Rooy 2017-03-21 13:22:47 UTC
used_for => 'export_records'
BUT in kohastructure: `usage` varchar(255) DEFAULT 'export_records'
You have to choose ;)

Typo:      [% ELSE %] Uknown type
Comment 77 Josef Moravec 2017-03-21 17:14:08 UTC
Created attachment 61405 [details] [review]
Bug 8612 - QA followup, fix kohastructure.sql, fix typo in template
Comment 78 Josef Moravec 2017-03-21 17:14:26 UTC
(In reply to Marcel de Rooy from comment #76)
> used_for => 'export_records'
> BUT in kohastructure: `usage` varchar(255) DEFAULT 'export_records'
> You have to choose ;)
> 
> Typo:      [% ELSE %] Uknown type

I wanted to use "usage" first, but it is mysql keyword

Both fixed in followup.
Comment 79 Marcel de Rooy 2017-03-23 09:25:19 UTC
QA: Looking here now
Comment 80 Marcel de Rooy 2017-03-23 09:55:12 UTC
Created attachment 61520 [details] [review]
Bug 8612: [QA Follow-up] Just a cosmetic space

Could not resist to add it :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 81 Marcel de Rooy 2017-03-23 09:58:51 UTC
See two things so far:

[1] Regex for field does not work:
$field =~ s/[^\.]*\.?//; # Remove the table name if exists.
Does not work well if there is no table in front.
Try this:
$field="title";
print "$field\n";
$field =~ s/[^\.]*\.?//; # Remove the table name if exists.
print "$field\n";

[2] Regex for header and fieldname makes header no longer optional.
        while ( $csv_profile_content =~ /
            ([^=]+) # header
            =
            ([^\|]+) # fieldname (table.row or row)
            \|? /gxms
        ) {
But if I understand correctly from this feature, the header should be optional.
=== Quote
You have to define which fields you want to export, separated by pipes.
You can also use your own headers (instead of the ones from Koha) by prefixing the field name with an header, followed by the equal sign.
===
If I remove the headers now, I will have an empty csv file.
Comment 82 Philippe Audet-Fortin 2017-05-03 20:49:50 UTC
Hi Marcel, I was wondering what you want to show in your first subparagraph, because it seems normal to me that it will not work if there is no table in front of the field. Koha need to know the table where the field come from.
(Sorry for my english :))
Comment 83 Marcel de Rooy 2017-05-04 06:42:13 UTC
(In reply to Philippe Audet-Fortin from comment #82)
> Hi Marcel, I was wondering what you want to show in your first subparagraph,
> because it seems normal to me that it will not work if there is no table in
> front of the field. Koha need to know the table where the field come from.
> (Sorry for my english :))

Hmm. Maybe a misunderstanding. Comment81 refers to some lines in the code including the comment "# Remove the table name if exists."
Comment 84 Josef Moravec 2017-05-04 07:16:43 UTC
Created attachment 63048 [details] [review]
Bug 8612: Use CSV profile for exporting basket

This patch allows the user to use a CSV export profile to create the fields to export the basket as CSV in a basket page.

Test plan:
1) Apply the patch
2) Go to Tools › CSV export profiles and create a profile of type "SQL for basket export in acquisition"
  example:
  biblionumber=biblio.biblionumber|auteur=biblio.author|titre=biblio.title|date=biblioitems.copyrightdate|editeur=biblioitems.publishercode|isbn=biblioitems.isbn|quantite=aqorders.quantity|prix=aqorders.rrp|panier=aqorders.basketno
3) In acquisition module, create a new basket and add an order to the basket
4) On basket detail page, there should be the split button labelled "Export to CSV"
5) Try to use the button and export CSV with your CSV profile you defined in step 2
6) Validate the CSV file.
7) Repeat 4-6 with a closed basket.
    a) close the basket
    b) View the basket
    c) validate that there is an export button
    d) test it with an export
8) prove t/db_dependent/Acquisition/GetBasketAsCSV.t t/db_dependent/Koha/CsvProfiles.t

Initial work:

Sponsored by: CCSR

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: mehdi <mehdi.hamidi@inlibro.com>
Comment 85 Josef Moravec 2017-05-04 07:16:50 UTC
Created attachment 63049 [details] [review]
Bug 8612: QA-followup - Fix test
Comment 86 Josef Moravec 2017-05-04 07:16:58 UTC
Created attachment 63050 [details] [review]
Bug 8612: QA followup - make usage and type different columns in db
Comment 87 Josef Moravec 2017-05-04 07:17:05 UTC
Created attachment 63051 [details] [review]
Bug 8612: QA followup - update dbix schema
Comment 88 Josef Moravec 2017-05-04 07:17:12 UTC
Created attachment 63052 [details] [review]
Bug 8612 - QA followup, fix kohastructure.sql, fix typo in template
Comment 89 Josef Moravec 2017-05-04 07:17:18 UTC
Created attachment 63053 [details] [review]
Bug 8612: [QA Follow-up] Just a cosmetic space

Could not resist to add it :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 90 Josef Moravec 2017-05-04 07:17:40 UTC
Just a rebase first...
Comment 91 Josef Moravec 2017-05-04 08:37:42 UTC
Created attachment 63054 [details] [review]
Bug 8612: QA folowup - fix regular expression to do what is described in the comment
Comment 92 Josef Moravec 2017-05-04 08:37:49 UTC
Created attachment 63055 [details] [review]
Bug 8612: QA followup - make header in CSV profile definition optional
Comment 93 Josef Moravec 2017-05-04 08:37:56 UTC
Created attachment 63056 [details] [review]
Bug 8612: QA followup - expand tests

Test plan:
prove t/db_dependent/Acquisition/GetBasketAsCSV.t
Comment 94 Josef Moravec 2017-05-04 08:39:30 UTC
(In reply to Marcel de Rooy from comment #81)
> See two things so far:
> 
> [1] Regex for field does not work:
> $field =~ s/[^\.]*\.?//; # Remove the table name if exists.
> Does not work well if there is no table in front.
> Try this:
> $field="title";
> print "$field\n";
> $field =~ s/[^\.]*\.?//; # Remove the table name if exists.
> print "$field\n";

Fixed by first today's followup

> [2] Regex for header and fieldname makes header no longer optional.
>         while ( $csv_profile_content =~ /
>             ([^=]+) # header
>             =
>             ([^\|]+) # fieldname (table.row or row)
>             \|? /gxms
>         ) {
> But if I understand correctly from this feature, the header should be
> optional.
> === Quote
> You have to define which fields you want to export, separated by pipes.
> You can also use your own headers (instead of the ones from Koha) by
> prefixing the field name with an header, followed by the equal sign.
> ===
> If I remove the headers now, I will have an empty csv file.

Fixed by second today's followup

Third followup is adding one more test case
Comment 95 Marcel de Rooy 2017-05-04 13:04:53 UTC
I tested with a trival csv:
biblio.title | some_text=aqorders.quantity

The title is not shown, the quantity is. Have the impression that the regex might still be the cause.
What did you test?
Comment 96 Philippe Audet-Fortin 2017-05-04 13:39:04 UTC
Created attachment 63104 [details] [review]
Bug 8612: QA followup - expand tests

Test plan:
prove t/db_dependent/Acquisition/GetBasketAsCSV.t
Comment 97 Philippe Audet-Fortin 2017-05-04 14:11:46 UTC
(In reply to Marcel de Rooy from comment #95)
> I tested with a trival csv:
> biblio.title | some_text=aqorders.quantity
> 
> The title is not shown, the quantity is. Have the impression that the regex
> might still be the cause.
> What did you test?

I copy paste your csv and I got some blank fields too. I try by removing white spaces and it worked.
(I had not seen your comment and I signoff too early. Sorry.)
Comment 98 Josef Moravec 2017-05-05 08:43:40 UTC
Created attachment 63158 [details] [review]
Bug 8612: QA followup - strip white chars from csv profile definition
Comment 99 Josef Moravec 2017-05-05 08:46:40 UTC
(In reply to Marcel de Rooy from comment #95)
> I tested with a trival csv:
> biblio.title | some_text=aqorders.quantity
> 
> The title is not shown, the quantity is. Have the impression that the regex
> might still be the cause.
> What did you test?

The reason it doesn't work for you are the spaces between fields in your definition.

The help paragraph on csv profile page doesn't say anything about spaces, so i decided to strip the white chars from definition before generating csv. It doesn't hurt and could help to avoid unexpected issues.
Comment 100 Josef Moravec 2017-05-09 08:05:12 UTC
Created attachment 63279 [details] [review]
Bug 8612: QA followup - strip white chars from csv profile definition, expand test
Comment 101 Josef Moravec 2017-05-09 08:06:03 UTC
Ok, it was not good solution, now should be better ;)
Comment 102 Marcel de Rooy 2017-06-02 06:52:27 UTC
Rearranging extensive number of (qa) follow-ups according to this plan:

Patch 1 Original start
Patch 3 + 5 + 6 Change table design
Patch 4 DBIx Schema
Patch 7 + Patch 8 + Patch 10a Module fixes
Patch 2 + Patch 9 + Patch 10b Unit test
Patch 11 New QA follow-up
Comment 103 Marcel de Rooy 2017-06-02 06:53:42 UTC
Created attachment 63910 [details] [review]
Bug 8612: Use CSV profile for exporting basket

This patch allows the user to use a CSV export profile to create the fields to export the basket as CSV in a basket page.

Test plan:
1) Apply the patch
2) Go to Tools › CSV export profiles and create a profile of type "SQL for basket export in acquisition"
  example:
  biblionumber=biblio.biblionumber|auteur=biblio.author|titre=biblio.title|date=biblioitems.copyrightdate|editeur=biblioitems.publishercode|isbn=biblioitems.isbn|quantite=aqorders.quantity|prix=aqorders.rrp|panier=aqorders.basketno
3) In acquisition module, create a new basket and add an order to the basket
4) On basket detail page, there should be the split button labelled "Export to CSV"
5) Try to use the button and export CSV with your CSV profile you defined in step 2
6) Validate the CSV file.
7) Repeat 4-6 with a closed basket.
    a) close the basket
    b) View the basket
    c) validate that there is an export button
    d) test it with an export
8) prove t/db_dependent/Acquisition/GetBasketAsCSV.t t/db_dependent/Koha/CsvProfiles.t

Initial work:

Sponsored by: CCSR

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: mehdi <mehdi.hamidi@inlibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 104 Marcel de Rooy 2017-06-02 06:53:48 UTC
Created attachment 63911 [details] [review]
Bug 8612: [Follow-up] Make usage and type different columns in table

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 105 Marcel de Rooy 2017-06-02 06:54:01 UTC
Created attachment 63912 [details] [review]
Bug 8612: [Follow-up] Update DBIx schema

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 106 Marcel de Rooy 2017-06-02 06:54:08 UTC
Created attachment 63913 [details] [review]
Bug 8612: [Follow-up] Fix regular expression

Fix regular expression to do what is described in the comment
Make header in CSV profile definition optional
Strip white chars from csv profile definition

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 107 Marcel de Rooy 2017-06-02 06:54:15 UTC
Created attachment 63914 [details] [review]
Bug 8612: [Follow-up] Fix unit test GetBasketAsCSV.t

Test plan:
Run t/db_dependent/Acquisition/GetBasketAsCSV.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 108 Marcel de Rooy 2017-06-02 06:54:22 UTC
Created attachment 63915 [details] [review]
Bug 8612: [QA Follow-up] Remove two newlines from template output

When using the Default profile from the basket form, the resulting csv
file has an additional newline after the headers and at the end.
This patch removes them.
Unit test adjusted accordingly.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 109 Marcel de Rooy 2017-06-02 06:56:52 UTC
Another report may still deal with this warn:
Use of uninitialized value in string eq at C4/Acquisition.pm line 318.
Comment 110 Marcel de Rooy 2017-06-02 06:59:45 UTC
And:
die "There is no valid csv profile given" unless $csv_profile;
Being liberal now. But we should actually use Koha::Exceptions now according to a new rule in the coding guidelines.
Comment 111 Josef Moravec 2017-06-02 07:04:25 UTC
(In reply to Marcel de Rooy from comment #109)
> Another report may still deal with this warn:
> Use of uninitialized value in string eq at C4/Acquisition.pm line 318.

I filled new bug 18717 for this
Comment 112 Josef Moravec 2017-06-02 09:12:28 UTC
(In reply to Marcel de Rooy from comment #110)
> And:
> die "There is no valid csv profile given" unless $csv_profile;
> Being liberal now. But we should actually use Koha::Exceptions now according
> to a new rule in the coding guidelines.

I filled new bug 18720
Comment 113 Jonathan Druart 2017-06-05 15:35:30 UTC
Created attachment 63964 [details] [review]
Bug 8612: Remove warnings from tests
Comment 114 Jonathan Druart 2017-06-05 15:56:03 UTC
Created attachment 63966 [details] [review]
Bug 8612: Update installer files for late_issues
Comment 115 Jonathan Druart 2017-06-05 15:59:15 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 116 Josef Moravec 2017-06-06 06:51:01 UTC
*** Bug 18717 has been marked as a duplicate of this bug. ***
Comment 117 Jonathan Druart 2017-06-09 13:43:14 UTC
Comment on attachment 63910 [details] [review]
Bug 8612: Use CSV profile for exporting basket

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

::: C4/Acquisition.pm
@@ +310,5 @@
> +            my $bd = GetBiblioData( $order->{'biblionumber'} );
> +            my @biblioitems = GetBiblioItemByBiblioNumber( $order->{'biblionumber'});
> +            for my $biblioitem (@biblioitems) {
> +                if ($biblioitem->{isbn} eq $order->{isbn}) {
> +                    $order = {%$order, %$biblioitem};

Could you explain these 3 lines?
Comment 118 Fridolin SOMERS 2017-06-13 07:27:10 UTC
Enhancement not pushed to 17.05.x