Bug 13345 - Choose the format when sending a basket/list by email
Summary: Choose the format when sending a basket/list by email
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Keywords:
Depends on: 11944 13343
Blocks:
  Show dependency treegraph
 
Reported: 2014-11-26 15:42 UTC by Jonathan Druart
Modified: 2021-10-01 10:51 UTC (History)
12 users (show)

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


Attachments
Bug 13345: Allow patrons to choose format on sending basket/list by email (55.74 KB, patch)
2014-11-28 11:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13345: Allow patrons to choose format on sending basket/list by email (55.77 KB, patch)
2014-12-15 08:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13345: Allow patrons to choose format on sending basket/list by email (55.79 KB, patch)
2015-01-23 14:33 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13345: Allow patrons to choose format on sending basket/list by email (55.89 KB, patch)
2015-03-02 10:57 UTC, Nicole C. Engard
Details | Diff | Splinter Review
Bug 13345: Allow patrons to choose format on sending basket/list by email (56.52 KB, patch)
2015-04-17 10:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13345: Allow patrons to choose format on sending basket/list by email (56.62 KB, patch)
2016-01-26 08:12 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13345: Allow patrons to choose format on sending basket/list by email (55.60 KB, patch)
2016-10-14 09:51 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13345: Allow patrons to choose format on sending basket/list by email (55.79 KB, patch)
2017-03-17 10:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 13345: Allow patrons to choose format on sending basket/list by email (54.75 KB, patch)
2020-03-17 10:59 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13345: Allow patrons to choose format on sending basket/list by email (54.61 KB, patch)
2020-03-27 12:49 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13345: Fix BuildBiblioDataForExport tests (2.86 KB, patch)
2020-03-27 12:49 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 13345: Allow patrons to choose format on sending basket/list by email (54.94 KB, patch)
2020-04-22 00:52 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 13345: Fix BuildBiblioDataForExport tests (2.93 KB, patch)
2020-04-22 00:52 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 13345: follow-up remove call to GetMember (811 bytes, patch)
2020-04-22 00:53 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 13345: Allow patrons to choose format on sending basket/list by email (40.37 KB, patch)
2021-04-01 13:47 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2014-11-26 15:42:07 UTC
Patrons can export baskets and lists in different formats (iso2709, ris, bibtex, csv), but they cannot choose a format when sending baskets or lists by email.
Comment 1 Jonathan Druart 2014-11-28 11:30:47 UTC Comment hidden (obsolete)
Comment 2 Paul Poulain 2014-12-12 12:54:36 UTC
Jonathan: can you add a "Sponsored-by: Mines Paristech" in the patch ? thus, MPT will appear in the credits
Comment 3 Jonathan Druart 2014-12-15 08:45:03 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-01-23 14:33:58 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2015-01-23 14:34:51 UTC
Last patch fixes conflicts with bug 13510.
Comment 6 Nicole C. Engard 2015-03-02 10:57:04 UTC Comment hidden (obsolete)
Comment 7 Marcel de Rooy 2015-04-17 10:11:38 UTC
QA: Looking at this one now.
Comment 8 Marcel de Rooy 2015-04-17 10:14:50 UTC
CONFLICT (content): Merge conflict in opac/opac-sendbasket.pl
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-downloadshelf.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-downloadcart.tt
CONFLICT (content): Merge conflict in basket/sendbasket.pl
Comment 9 Jonathan Druart 2015-04-17 10:59:40 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2015-06-08 10:57:49 UTC
Hi Marcel, you started on this a while ago - do you want to continue or is it ok to change the QA contact? Patches apply now.
Comment 11 Marcel de Rooy 2015-06-22 08:04:25 UTC
(In reply to Katrin Fischer from comment #10)
> Hi Marcel, you started on this a while ago - do you want to continue or is
> it ok to change the QA contact? Patches apply now.

Go ahead if you want to QA this one..
Comment 12 Katrin Fischer 2015-10-18 12:22:00 UTC
Jonathan, there are some conflicts - can you please take a look and fix?

Applying: Bug 13345: Allow patrons to choose format on sending basket/list by email
Using index info to reconstruct a base tree...
M	C4/Biblio.pm
M	basket/basket.pl
M	basket/downloadcart.pl
M	basket/sendbasket.pl
M	koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasket.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-downloadcart.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-downloadshelf.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendbasket.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendbasketform.tt
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendshelfform.tt
M	opac/opac-downloadcart.pl
M	opac/opac-downloadshelf.pl
M	opac/opac-sendbasket.pl
M	opac/opac-sendshelf.pl
M	virtualshelves/downloadshelf.pl
M	virtualshelves/sendshelf.pl
Falling back to patching base and 3-way merge...
Auto-merging virtualshelves/sendshelf.pl
CONFLICT (content): Merge conflict in virtualshelves/sendshelf.pl
Auto-merging virtualshelves/downloadshelf.pl
Auto-merging opac/opac-sendshelf.pl
CONFLICT (content): Merge conflict in opac/opac-sendshelf.pl
Auto-merging opac/opac-sendbasket.pl
CONFLICT (content): Merge conflict in opac/opac-sendbasket.pl
Auto-merging opac/opac-downloadshelf.pl
Auto-merging opac/opac-downloadcart.pl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendshelfform.tt
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendbasketform.tt
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendbasket.tt
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-downloadshelf.tt
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-downloadcart.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasket.tt
Auto-merging basket/sendbasket.pl
CONFLICT (content): Merge conflict in basket/sendbasket.pl
Auto-merging basket/downloadcart.pl
Auto-merging basket/basket.pl
Auto-merging C4/Biblio.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 13345: Allow patrons to choose format on sending basket/list by email
The copy of the patch that failed is found in:
   /home/katrin/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".
Comment 13 Julian Maurice 2016-01-26 08:12:42 UTC
Created attachment 47297 [details] [review]
Bug 13345: Allow patrons to choose format on sending basket/list by email

It is already possible to download a basket(cart) or a list using
different format.
But by email, only iso2709 is possible.

This patch adds the ability to select a format for the file attached to
the email when sending the basket/list content.

Test plan:
1/ At the OPAC, add some records to your basket and send it by email
using different formats (RIS, BibTeX, MARC and the CSV profiles defined).
2/ At the OPAC, add some records to a list and send it by email using
the different formats.
3/ Redo step 1 at the intranet
4/ Redo step 2 at the intranet

Not that subtitles are not displayed in all these cases.

Note for QA: This patch refactors 3 main things:
1/ Template side: the dropdown list to select the format is now in an
include file (indeed, this list is used in 4 templates, this will make
easier further changes).
The include files are named 'download-export-available-formats.inc',
feel free to suggest something better.
The are used in:
[opac-]sendbasket.tt, [opac-]sendshelf.tt, [opac-]downloadbasket.tt and
[opac-]downloadshelf.tt.

2/ Build records data and the records file
This job was done in 4 scripts:
 - opac/opac-sendbasket.pl
 - opac/opac-sendshelf.pl
 - basket/sendbasket.pl
 - virtualshelves/sendshelf.pl
Now it's done in C4::Biblio::BuildBiblioDataForExport and tests are
provided in t/db_dependent/Biblio/BuildBiblioDataForExport.t
Not that a similar job is done in [opac-]downloadcart.pl and
[opac-]downloadshelf.pl

3/ Build the subject and body of the email sent
Same as before, the job was done at the same places.
Now it's done in Koha::Email->fill_body_for_sending (feel free to
suggestion a better name) and tests are provided in t/Koha_Email.t

Some tests for BuildBiblioDataForExport can fail if bug 13360 is not
pushed yet.

Sponsored-by: Mines Paristech
Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Comment 14 Julian Maurice 2016-01-26 08:13:01 UTC
Patch rebased on master
Comment 15 Katrin Fischer 2016-03-29 22:38:20 UTC
I am sorry, conflicting again:

CONFLICT (content): Merge conflict in virtualshelves/sendshelf.pl
Auto-merging opac/opac-sendshelf.pl
CONFLICT (content): Merge conflict in opac/opac-sendshelf.pl
Auto-merging opac/opac-sendbasket.pl
CONFLICT (content): Merge conflict in opac/opac-sendbasket.pl
Auto-merging opac/opac-downloadshelf.pl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendbasket.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/basket/sendbasket.tt
Auto-merging basket/sendbasket.pl
CONFLICT (content): Merge conflict in basket/sendbasket.pl

Will return to this asap when rebased.
Comment 16 Julian Maurice 2016-10-14 09:51:12 UTC
Created attachment 56499 [details] [review]
Bug 13345: Allow patrons to choose format on sending basket/list by email

It is already possible to download a basket(cart) or a list using
different format.
But by email, only iso2709 is possible.

This patch adds the ability to select a format for the file attached to
the email when sending the basket/list content.

Test plan:
1/ At the OPAC, add some records to your basket and send it by email
using different formats (RIS, BibTeX, MARC and the CSV profiles defined).
2/ At the OPAC, add some records to a list and send it by email using
the different formats.
3/ Redo step 1 at the intranet
4/ Redo step 2 at the intranet

Not that subtitles are not displayed in all these cases.

Note for QA: This patch refactors 3 main things:
1/ Template side: the dropdown list to select the format is now in an
include file (indeed, this list is used in 4 templates, this will make
easier further changes).
The include files are named 'download-export-available-formats.inc',
feel free to suggest something better.
The are used in:
[opac-]sendbasket.tt, [opac-]sendshelf.tt, [opac-]downloadbasket.tt and
[opac-]downloadshelf.tt.

2/ Build records data and the records file
This job was done in 4 scripts:
 - opac/opac-sendbasket.pl
 - opac/opac-sendshelf.pl
 - basket/sendbasket.pl
 - virtualshelves/sendshelf.pl
Now it's done in C4::Biblio::BuildBiblioDataForExport and tests are
provided in t/db_dependent/Biblio/BuildBiblioDataForExport.t
Not that a similar job is done in [opac-]downloadcart.pl and
[opac-]downloadshelf.pl

3/ Build the subject and body of the email sent
Same as before, the job was done at the same places.
Now it's done in Koha::Email->fill_body_for_sending (feel free to
suggestion a better name) and tests are provided in t/Koha_Email.t

Sponsored-by: Mines Paristech
Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Comment 17 Marcel de Rooy 2017-03-17 10:51:19 UTC
Created attachment 61210 [details] [review]
Bug 13345: Allow patrons to choose format on sending basket/list by email

It is already possible to download a basket(cart) or a list using
different format.
But by email, only iso2709 is possible.

This patch adds the ability to select a format for the file attached to
the email when sending the basket/list content.

Test plan:
1/ At the OPAC, add some records to your basket and send it by email
using different formats (RIS, BibTeX, MARC and the CSV profiles defined).
2/ At the OPAC, add some records to a list and send it by email using
the different formats.
3/ Redo step 1 at the intranet
4/ Redo step 2 at the intranet

Not that subtitles are not displayed in all these cases.

Note for QA: This patch refactors 3 main things:
1/ Template side: the dropdown list to select the format is now in an
include file (indeed, this list is used in 4 templates, this will make
easier further changes).
The include files are named 'download-export-available-formats.inc',
feel free to suggest something better.
The are used in:
[opac-]sendbasket.tt, [opac-]sendshelf.tt, [opac-]downloadbasket.tt and
[opac-]downloadshelf.tt.

2/ Build records data and the records file
This job was done in 4 scripts:
 - opac/opac-sendbasket.pl
 - opac/opac-sendshelf.pl
 - basket/sendbasket.pl
 - virtualshelves/sendshelf.pl
Now it's done in C4::Biblio::BuildBiblioDataForExport and tests are
provided in t/db_dependent/Biblio/BuildBiblioDataForExport.t
Not that a similar job is done in [opac-]downloadcart.pl and
[opac-]downloadshelf.pl

3/ Build the subject and body of the email sent
Same as before, the job was done at the same places.
Now it's done in Koha::Email->fill_body_for_sending (feel free to
suggestion a better name) and tests are provided in t/Koha_Email.t

Sponsored-by: Mines Paristech
Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Comment 18 Marcel de Rooy 2017-03-17 10:53:10 UTC
# Subtest: iso2709
    1..7
    ok 1 - BuildBiblioDataForExport should return a correct records file in iso2709
    ok 2
    ok 3 - 499 and 462
    ok 4
    ok 5 - 1 item
    ok 6 - 0 author
    ok 7 - bibionumber is correct

Please add test descriptions
Comment 19 Marcel de Rooy 2017-03-17 10:53:40 UTC
# Subtest: ris
    1..1
Use of uninitialized value in numeric ne (!=) at /usr/share/koha/masterclone/C4/Ris.pm line 834.
Use of uninitialized value in numeric ne (!=) at /usr/share/koha/masterclone/C4/Ris.pm line 834.
    not ok 1
    #   Failed test at t/db_dependent/Biblio/BuildBiblioDataForExport.t line 68.
    #          got: 'TY  - BOOK
    # AU  - Knuth,Donald Ervin
    # TI  - The art of computer programming
    # KW  - Computer programming
    # KW  - Computer algorithms
    # ER  -
    # '
    #     expected: 'TY  - BOOK
    # AU  - Knuth, Donald Ervin
    # TI  - The art of computer programming
    # KW  - Computer programming
    # KW  - Computer algorithms
    # ER  -
    # '
    # Looks like you failed 1 test of 1.
Comment 20 Marcel de Rooy 2017-03-17 10:54:41 UTC
# Subtest: bibtex
    1..1
    not ok 1
    #   Failed test at t/db_dependent/Biblio/BuildBiblioDataForExport.t line 77.
    #          got: '@book{1945,
    #   author = {Knuth, Donald Ervin},
    #   title = {The art of computer programming}
    # }
    # '
    #     expected: '@book{1945,
    #     author = {Knuth, Donald Ervin},
    #     title = {The art of computer programming}
    # }
    # '
    # Looks like you failed 1 test of 1.
Comment 21 Marcel de Rooy 2017-03-17 11:00:29 UTC
Can't locate object method "biblionumber" via package "41" (perhaps you forgot to load "41"?) at /usr/share/koha/masterclone/virtualshelves/sendshelf.pl line 76

Both opac-sendshelf and virtualshelves/sendshelf.pl contain this statement: 
    my $biblionumber = $content->biblionumber->biblionumber;
Is not correct.

Also please add your signoff to the original patch. And/or add follow-ups.
Comment 22 Jonathan Druart 2017-03-17 15:33:58 UTC
(In reply to Marcel de Rooy from comment #21)
> Can't locate object method "biblionumber" via package "41" (perhaps you
> forgot to load "41"?) at
> /usr/share/koha/masterclone/virtualshelves/sendshelf.pl line 76
> 
> Both opac-sendshelf and virtualshelves/sendshelf.pl contain this statement: 
>     my $biblionumber = $content->biblionumber->biblionumber;
> Is not correct.

Because of bug 17094.

About the test failures, I guess we will adjust the tests as it is certainly not this patch that make them fail, but it looks like a Koha regression then. I am pretty sure the tests passed when I wrote them.
Comment 23 Julian Maurice 2020-03-17 10:59:47 UTC
Created attachment 100886 [details] [review]
Bug 13345: Allow patrons to choose format on sending basket/list by email

It is already possible to download a basket(cart) or a list using
different format.
But by email, only iso2709 is possible.

This patch adds the ability to select a format for the file attached to
the email when sending the basket/list content.

Test plan:
1/ At the OPAC, add some records to your basket and send it by email
using different formats (RIS, BibTeX, MARC and the CSV profiles defined).
2/ At the OPAC, add some records to a list and send it by email using
the different formats.
3/ Redo step 1 at the intranet
4/ Redo step 2 at the intranet

Note that subtitles are not displayed in all these cases.

Note for QA: This patch refactors 3 main things:
1/ Template side: the dropdown list to select the format is now in an
include file (indeed, this list is used in 4 templates, this will make
easier further changes).
The include files are named 'download-export-available-formats.inc',
feel free to suggest something better.
The are used in:
[opac-]sendbasket.tt, [opac-]sendshelf.tt, [opac-]downloadbasket.tt and
[opac-]downloadshelf.tt.

2/ Build records data and the records file
This job was done in 4 scripts:
 - opac/opac-sendbasket.pl
 - opac/opac-sendshelf.pl
 - basket/sendbasket.pl
 - virtualshelves/sendshelf.pl
Now it's done in C4::Biblio::BuildBiblioDataForExport and tests are
provided in t/db_dependent/Biblio/BuildBiblioDataForExport.t
Not that a similar job is done in [opac-]downloadcart.pl and
[opac-]downloadshelf.pl

3/ Build the subject and body of the email sent
Same as before, the job was done at the same places.
Now it's done in Koha::Email->fill_body_for_sending (feel free to
suggestion a better name) and tests are provided in t/Koha_Email.t

Sponsored-by: Mines Paristech
Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Comment 24 Julian Maurice 2020-03-17 11:00:51 UTC
Patch rebased on master.
Comment 25 Kyle M Hall 2020-03-20 17:13:19 UTC
prove t/db_dependent/Biblio/BuildBiblioDataForExport.t
t/db_dependent/Biblio/BuildBiblioDataForExport.t .. 1/4 Use of uninitialized value in numeric ne (!=) at /kohadevbox/koha/C4/Ris.pm line 833.
Use of uninitialized value in numeric ne (!=) at /kohadevbox/koha/C4/Ris.pm line 833.

    #   Failed test at t/db_dependent/Biblio/BuildBiblioDataForExport.t line 68.
    #          got: 'TY  - BOOK
    # AU  - Knuth,Donald Ervin
    # TI  - The art of computer programming
    # KW  - Computer programming
    # KW  - Computer algorithms
    # ER  -
    # '
    #     expected: 'TY  - BOOK
    # AU  - Knuth, Donald Ervin
    # TI  - The art of computer programming
    # KW  - Computer programming
    # KW  - Computer algorithms
    # ER  -
    # '
    # Looks like you failed 1 test of 1.

#   Failed test 'ris'
#   at t/db_dependent/Biblio/BuildBiblioDataForExport.t line 69.

    #   Failed test at t/db_dependent/Biblio/BuildBiblioDataForExport.t line 77.
    #          got: '@book{440,
    # 	author = {Knuth, Donald Ervin},
    # 	title = {The art of computer programming}
    # }
    # '
    #     expected: '@book{440,
    #     author = {Knuth, Donald Ervin},
    #     title = {The art of computer programming}
    # }
    # '
    # Looks like you failed 1 test of 1.

#   Failed test 'bibtex'
#   at t/db_dependent/Biblio/BuildBiblioDataForExport.t line 82.
# Looks like you failed 2 tests of 4.
t/db_dependent/Biblio/BuildBiblioDataForExport.t .. Dubious, test returned 2 (wstat 512, 0x200)
Failed 2/4 subtests

Test Summary Report
-------------------
t/db_dependent/Biblio/BuildBiblioDataForExport.t (Wstat: 512 Tests: 4 Failed: 2)
  Failed tests:  3-4
  Non-zero exit status: 2
Files=1, Tests=4,  3 wallclock secs ( 0.02 usr  0.01 sys +  1.46 cusr  0.27 csys =  1.76 CPU)
Result: FAIL
Comment 26 Julian Maurice 2020-03-27 12:49:24 UTC
Created attachment 101998 [details] [review]
Bug 13345: Allow patrons to choose format on sending basket/list by email

It is already possible to download a basket(cart) or a list using
different format.
But by email, only iso2709 is possible.

This patch adds the ability to select a format for the file attached to
the email when sending the basket/list content.

Test plan:
1/ At the OPAC, add some records to your basket and send it by email
using different formats (RIS, BibTeX, MARC and the CSV profiles defined).
2/ At the OPAC, add some records to a list and send it by email using
the different formats.
3/ Redo step 1 at the intranet
4/ Redo step 2 at the intranet

Note that subtitles are not displayed in all these cases.

Note for QA: This patch refactors 3 main things:
1/ Template side: the dropdown list to select the format is now in an
include file (indeed, this list is used in 4 templates, this will make
easier further changes).
The include files are named 'download-export-available-formats.inc',
feel free to suggest something better.
The are used in:
[opac-]sendbasket.tt, [opac-]sendshelf.tt, [opac-]downloadbasket.tt and
[opac-]downloadshelf.tt.

2/ Build records data and the records file
This job was done in 4 scripts:
 - opac/opac-sendbasket.pl
 - opac/opac-sendshelf.pl
 - basket/sendbasket.pl
 - virtualshelves/sendshelf.pl
Now it's done in C4::Biblio::BuildBiblioDataForExport and tests are
provided in t/db_dependent/Biblio/BuildBiblioDataForExport.t
Not that a similar job is done in [opac-]downloadcart.pl and
[opac-]downloadshelf.pl

3/ Build the subject and body of the email sent
Same as before, the job was done at the same places.
Now it's done in Koha::Email->fill_body_for_sending (feel free to
suggestion a better name) and tests are provided in t/Koha_Email.t

Sponsored-by: Mines Paristech
Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Comment 27 Julian Maurice 2020-03-27 12:49:30 UTC
Created attachment 101999 [details] [review]
Bug 13345: Fix BuildBiblioDataForExport tests
Comment 28 Katrin Fischer 2020-04-13 16:22:41 UTC
This is quite an old patch that changes more files than you'd think at first glance. We got a sign-off from Nicole here, but it's from March 2015. Hope it's ok, but I'd like to ask for a new sign off here.

Tests pass and QA script too - I like to see the amount of 'red' (removed code) too.
Comment 29 Bernardo Gonzalez Kriegel 2020-04-22 00:52:47 UTC
Created attachment 103433 [details] [review]
Bug 13345: Allow patrons to choose format on sending basket/list by email

It is already possible to download a basket(cart) or a list using
different format.
But by email, only iso2709 is possible.

This patch adds the ability to select a format for the file attached to
the email when sending the basket/list content.

Test plan:
1/ At the OPAC, add some records to your basket and send it by email
using different formats (RIS, BibTeX, MARC and the CSV profiles defined).
2/ At the OPAC, add some records to a list and send it by email using
the different formats.
3/ Redo step 1 at the intranet
4/ Redo step 2 at the intranet

Note that subtitles are not displayed in all these cases.

Note for QA: This patch refactors 3 main things:
1/ Template side: the dropdown list to select the format is now in an
include file (indeed, this list is used in 4 templates, this will make
easier further changes).
The include files are named 'download-export-available-formats.inc',
feel free to suggest something better.
The are used in:
[opac-]sendbasket.tt, [opac-]sendshelf.tt, [opac-]downloadbasket.tt and
[opac-]downloadshelf.tt.

2/ Build records data and the records file
This job was done in 4 scripts:
 - opac/opac-sendbasket.pl
 - opac/opac-sendshelf.pl
 - basket/sendbasket.pl
 - virtualshelves/sendshelf.pl
Now it's done in C4::Biblio::BuildBiblioDataForExport and tests are
provided in t/db_dependent/Biblio/BuildBiblioDataForExport.t
Not that a similar job is done in [opac-]downloadcart.pl and
[opac-]downloadshelf.pl

3/ Build the subject and body of the email sent
Same as before, the job was done at the same places.
Now it's done in Koha::Email->fill_body_for_sending (feel free to
suggestion a better name) and tests are provided in t/Koha_Email.t

Sponsored-by: Mines Paristech
Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Test pass and mail sent, no errors.
But a follow-up to remove useless call to GetMember is needed for
email list to work.
Comment 30 Bernardo Gonzalez Kriegel 2020-04-22 00:52:52 UTC
Created attachment 103434 [details] [review]
Bug 13345: Fix BuildBiblioDataForExport tests

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment 31 Bernardo Gonzalez Kriegel 2020-04-22 00:53:30 UTC
Created attachment 103435 [details] [review]
Bug 13345: follow-up remove call to GetMember

This followup removes a forgotten call to GetMember
Comment 32 Bernardo Gonzalez Kriegel 2020-04-22 00:55:26 UTC
(In reply to Katrin Fischer from comment #28)
> This is quite an old patch that changes more files than you'd think at first
> glance. We got a sign-off from Nicole here, but it's from March 2015. Hope
> it's ok, but I'd like to ask for a new sign off here.
> 
> Tests pass and QA script too - I like to see the amount of 'red' (removed
> code) too.

Signed both patches, but sending a list by email fails because there is a forgotten call to GetMember, follow-up added
Comment 33 Séverine Queune 2021-04-01 08:12:54 UTC
Falling back to patching base and 3-way merge...
Auto-merging virtualshelves/sendshelf.pl
CONFLICT (content): Merge conflict in virtualshelves/sendshelf.pl
Auto-merging virtualshelves/downloadshelf.pl
CONFLICT (modify/delete): t/Koha_Email.t deleted in HEAD and modified in Bug 13345: Allow patrons to choose format on sending basket/list by email. Version Bug 13345: Allow patrons to choose format on sending basket/list by email of t/Koha_Email.t left in tree.
Auto-merging opac/opac-sendshelf.pl
CONFLICT (content): Merge conflict in opac/opac-sendshelf.pl
Auto-merging opac/opac-sendbasket.pl
CONFLICT (content): Merge conflict in opac/opac-sendbasket.pl
Auto-merging opac/opac-downloadshelf.pl
Auto-merging opac/opac-downloadcart.pl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendshelfform.tt
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-sendbasketform.tt
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-downloadshelf.tt
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-downloadcart.tt
Auto-merging basket/sendbasket.pl
CONFLICT (content): Merge conflict in basket/sendbasket.pl
Auto-merging basket/downloadcart.pl
Auto-merging basket/basket.pl
Auto-merging Koha/Email.pm
CONFLICT (content): Merge conflict in Koha/Email.pm
Auto-merging C4/Biblio.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 13345: Allow patrons to choose format on sending basket/list by email
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-13345-Allow-patrons-to-choose-format-on-sendin-QUTNYW.patch
Comment 34 Julian Maurice 2021-04-01 13:47:34 UTC
Created attachment 119078 [details] [review]
Bug 13345: Allow patrons to choose format on sending basket/list by email

It is already possible to download a basket(cart) or a list using
different format.
But by email, only iso2709 is possible.

This patch adds the ability to select a format for the file attached to
the email when sending the basket/list content.

Test plan:
1/ At the OPAC, add some records to your basket and send it by email
using different formats (RIS, BibTeX, MARC and the CSV profiles defined).
2/ At the OPAC, add some records to a list and send it by email using
the different formats.
3/ Redo step 1 at the intranet
4/ Redo step 2 at the intranet

Note that subtitles are not displayed in all these cases.

Note for QA: This patch refactors 2 main things:
1/ Template side: the dropdown list to select the format is now in an
include file (indeed, this list is used in 4 templates, this will make
easier further changes).
The include files are named 'download-export-available-formats.inc',
feel free to suggest something better.
The are used in:
[opac-]sendbasket.tt, [opac-]sendshelf.tt, [opac-]downloadbasket.tt and
[opac-]downloadshelf.tt.

2/ Build records data and the records file
This job was done in 4 scripts:
 - opac/opac-sendbasket.pl
 - opac/opac-sendshelf.pl
 - basket/sendbasket.pl
 - virtualshelves/sendshelf.pl
Now it's done in C4::Biblio::BuildBiblioDataForExport and tests are
provided in t/db_dependent/Biblio/BuildBiblioDataForExport.t
Note that a similar job is done in [opac-]downloadcart.pl and
[opac-]downloadshelf.pl

Sponsored-by: Mines Paristech
Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Test pass and mail sent, no errors.