Bug 18555 - Create patron list from patron import
Summary: Create patron list from patron import
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Marc Véron
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-05-08 14:16 UTC by Marc Véron
Modified: 2017-08-10 19:38 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 18555: Create patron list from patron import (5.17 KB, patch)
2017-05-08 14:26 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18555: Create patron list from patron import (5.12 KB, patch)
2017-05-08 14:28 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18555: Create patron list from patron import (5.18 KB, patch)
2017-05-09 12:45 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18555: (QA-Followup) Remove duplicate code (1.10 KB, patch)
2017-08-08 08:08 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18555: Create patron list from patron import (5.25 KB, patch)
2017-08-10 19:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18555: (QA-Followup) Remove duplicate code (1.18 KB, patch)
2017-08-10 19:38 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 Marc Véron 2017-05-08 14:16:47 UTC
Create a patron list from imported patrons to be used for printing patron cards and other patron list related actions.
Comment 1 Marc Véron 2017-05-08 14:26:51 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2017-05-08 14:28:17 UTC
Created attachment 63242 [details] [review]
Bug 18555: Create patron list from patron import

Create a patron list from imported patrons to be used for printing patron
cards and other patron list related actions.

To test:
- Apply patch
- Go to Home > Tools > Import patrons
- Select a file to import and check new checkbox 'Create patron list'
- Import file
- Verify that in import results you see the time stamped name of the
  patronlist containig the imported patrons (if any patrons were imported)
- Go to Home > Tools > Patron lists
- Verify that the list shows up and contains the imported patrons
Comment 3 Josef Moravec 2017-05-09 12:45:07 UTC
Created attachment 63284 [details] [review]
[SIGNED-OFF] Bug 18555: Create patron list from patron import

Create a patron list from imported patrons to be used for printing patron
cards and other patron list related actions.

To test:
- Apply patch
- Go to Home > Tools > Import patrons
- Select a file to import and check new checkbox 'Create patron list'
- Import file
- Verify that in import results you see the time stamped name of the
  patronlist containig the imported patrons (if any patrons were imported)
- Go to Home > Tools > Patron lists
- Verify that the list shows up and contains the imported patrons

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 4 Marc Véron 2017-05-18 07:15:13 UTC
Still applies on current master
Comment 5 Nick Clemens 2017-07-31 16:10:56 UTC
Nice! A couple things:

1 - Duplicated code for testing the input and generating the list name - we should really only bother inside the check to create a list or not
2 - We should link from the results page to the newly created list
3 - We may want to optionally include overwritten borrowers in the list, they are sort of imported - I see use case in a school that overwrites patrons to update contact info/expiration and may generate new cards regularly -not blocker but would be nice
Comment 6 Marc Véron 2017-07-31 17:21:02 UTC
(In reply to Nick Clemens from comment #5)
> Nice! A couple things:
> 
> 1 - Duplicated code for testing the input and generating the list name - we
> should really only bother inside the check to create a list or not
> 2 - We should link from the results page to the newly created list
> 3 - We may want to optionally include overwritten borrowers in the list,
> they are sort of imported - I see use case in a school that overwrites
> patrons to update contact info/expiration and may generate new cards
> regularly -not blocker but would be nice

Hi Nick

Can you elaborate a little bit what you mean by 1) ?

2) and 3) could go to a new bug based on this one - I have not much time ATM.

Marc
Comment 7 Nick Clemens 2017-08-07 18:28:19 UTC
(In reply to Marc Véron from comment #6)
> (In reply to Nick Clemens from comment #5)
> Can you elaborate a little bit what you mean by 1) ?

109 	#create a patronlist
110 	my $createpatronlist = $input->param('createpatronlist') || 0;
111 	my $dt = dt_from_string();
112 	my $timestamp = $dt->ymd('-').' '.$dt->hms(':');
113 	my $patronlistname = $uploadborrowers . ' (' . $timestamp .')';

124 	    #create a patronlist
125 	    my $createpatronlist = $input->param('createpatronlist');
126 	    my $dt = dt_from_string();
127 	    my $timestamp = $dt->ymd('-').' '.$dt->hms(':');
128 	    my $patronlistname = $uploadborrowers. ' (' . $timestamp .')';

We do this twice - if createpatronlist isn't checked we don't need to do the rest of the setup

> 
> 2) and 3) could go to a new bug based on this one - I have not much time ATM.
I will try to provide a followup or open a new bug for these.
Comment 8 Marc Véron 2017-08-08 08:08:17 UTC
Created attachment 65614 [details] [review]
Bug 18555: (QA-Followup) Remove duplicate code

Remove duplicate code as of comment #7
Comment 9 Marc Véron 2017-08-08 08:09:49 UTC
Hope it's OK to set back to SO.
Comment 10 Jonathan Druart 2017-08-10 19:37:09 UTC
Created attachment 65852 [details] [review]
Bug 18555: Create patron list from patron import

Create a patron list from imported patrons to be used for printing patron
cards and other patron list related actions.

To test:
- Apply patch
- Go to Home > Tools > Import patrons
- Select a file to import and check new checkbox 'Create patron list'
- Import file
- Verify that in import results you see the time stamped name of the
  patronlist containig the imported patrons (if any patrons were imported)
- Go to Home > Tools > Patron lists
- Verify that the list shows up and contains the imported patrons

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Jonathan Druart 2017-08-10 19:37:58 UTC
(In reply to Marc Véron from comment #6)
> 2) and 3) could go to a new bug based on this one - I have not much time ATM.

Could you open them please?
Comment 12 Jonathan Druart 2017-08-10 19:38:10 UTC
Created attachment 65853 [details] [review]
Bug 18555: (QA-Followup) Remove duplicate code

Remove duplicate code as of comment #7

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2017-08-10 19:38:39 UTC
Pushed to master for 17.11, thanks to everybody involved!