Bug 18789 - Send a Koha::Patron object to the templates
Summary: Send a Koha::Patron object to the templates
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Josef Moravec
URL:
Whiteboard:
Keywords:
Depends on: 18403
Blocks: 12159 20223 20226 20792 20249 20822
  Show dependency treegraph
 
Reported: 2017-06-12 16:40 UTC by Jonathan Druart
Modified: 2018-05-28 18:11 UTC (History)
3 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 18789: Send Koha::Patron object to the templates (219.30 KB, patch)
2017-06-12 16:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Pass a Koha::Patron object from returns.pl (16.86 KB, patch)
2017-06-12 16:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->image from the templates (18.83 KB, patch)
2017-06-12 16:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: add tests for is_adult and is_child (3.84 KB, patch)
2017-06-12 16:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->is_child where needed (7.88 KB, patch)
2017-06-12 16:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->is_adult where needed (10.60 KB, patch)
2017-06-12 16:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Remove useless IS_ADULT variable (845 bytes, patch)
2017-06-12 16:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Remove adultborrower from the pay* scripts (1.76 KB, patch)
2017-06-12 16:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Update other occurences where is->child should be used (7.70 KB, patch)
2017-06-12 16:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: (follow-up)Send Koha::Patron object to the templates (1.36 KB, patch)
2017-06-12 17:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Send Koha::Patron object to the templates (215.14 KB, patch)
2018-02-07 18:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Pass a Koha::Patron object from returns.pl (16.53 KB, patch)
2018-02-07 18:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->image from the templates (18.63 KB, patch)
2018-02-07 18:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: add tests for is_adult and is_child (3.70 KB, patch)
2018-02-07 18:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->is_child where needed (7.55 KB, patch)
2018-02-07 18:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->is_adult where needed (10.57 KB, patch)
2018-02-07 18:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Remove useless IS_ADULT variable (845 bytes, patch)
2018-02-07 18:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Remove adultborrower from the pay* scripts (1.79 KB, patch)
2018-02-07 18:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Update other occurences where is->child should be used (7.80 KB, patch)
2018-02-07 18:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: (follow-up)Send Koha::Patron object to the templates (1.36 KB, patch)
2018-02-07 18:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: (follow-up) Use Koha::Patron->image from the templates (697 bytes, patch)
2018-02-07 18:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Send Koha::Patron object to the templates (214.74 KB, patch)
2018-02-12 17:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18789: Pass a Koha::Patron object from returns.pl (16.57 KB, patch)
2018-02-12 17:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->image from the templates (18.55 KB, patch)
2018-02-12 17:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18789: add tests for is_adult and is_child (3.74 KB, patch)
2018-02-12 17:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->is_child where needed (7.54 KB, patch)
2018-02-12 17:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->is_adult where needed (10.53 KB, patch)
2018-02-12 17:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18789: Remove useless IS_ADULT variable (895 bytes, patch)
2018-02-12 17:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18789: Remove adultborrower from the pay* scripts (1.83 KB, patch)
2018-02-12 17:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18789: Update other occurences where is->child should be used (7.80 KB, patch)
2018-02-12 17:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18789: (follow-up)Send Koha::Patron object to the templates (1.41 KB, patch)
2018-02-12 17:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18789: (follow-up) Use Koha::Patron->image from the templates (746 bytes, patch)
2018-02-12 17:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18789: Fix number of unit tests (689 bytes, patch)
2018-02-12 17:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 18789: Send Koha::Patron object to the templates (209.09 KB, patch)
2018-02-15 18:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Pass a Koha::Patron object from returns.pl (16.58 KB, patch)
2018-02-15 18:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->image from the templates (18.49 KB, patch)
2018-02-15 18:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: add tests for is_adult and is_child (3.76 KB, patch)
2018-02-15 18:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->is_child where needed (6.70 KB, patch)
2018-02-15 18:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->is_adult where needed (10.60 KB, patch)
2018-02-15 18:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Remove useless IS_ADULT variable (901 bytes, patch)
2018-02-15 18:49 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Remove adultborrower from the pay* scripts (1.85 KB, patch)
2018-02-15 18:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Update other occurences where is->child should be used (7.86 KB, patch)
2018-02-15 18:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: (follow-up)Send Koha::Patron object to the templates (1.42 KB, patch)
2018-02-15 18:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: (follow-up) Use Koha::Patron->image from the templates (752 bytes, patch)
2018-02-15 18:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: add tests for is_adult and is_chil (668 bytes, patch)
2018-02-15 18:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Use the patron variable in circulation.[pl|tt] (15.71 KB, patch)
2018-02-15 18:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Send Koha::Patron object to the templates (208.69 KB, patch)
2018-02-16 11:12 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: Pass a Koha::Patron object from returns.pl (16.63 KB, patch)
2018-02-16 11:12 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->image from the templates (18.41 KB, patch)
2018-02-16 11:12 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: Add tests for is_adult and is_child (3.80 KB, patch)
2018-02-16 11:12 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->is_child where needed (6.69 KB, patch)
2018-02-16 11:12 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: Use Koha::Patron->is_adult where needed (10.55 KB, patch)
2018-02-16 11:12 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: Remove useless IS_ADULT variable (950 bytes, patch)
2018-02-16 11:12 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: Remove adultborrower from the pay* scripts (1.89 KB, patch)
2018-02-16 11:12 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: Update other occurences where is_child should be used (7.85 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: (follow-up) Send Koha::Patron object to the templates (1.47 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: (follow-up) Use Koha::Patron->image from the templates (801 bytes, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: Add tests for is_adult and is_child (717 bytes, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: Use the patron variable in circulation.[pl|tt] (15.74 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: (QA follow-up) Fix tab char (1.55 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: (QA follow-up) Fix borrowernumber in menu item links (1.93 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: (QA follow-up) Fix showing the name of basket creator on booksellers page (1.55 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: (QA follow-up) Use first_valid_email_address on Tranfers to receive page (2.07 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: (QA follow-up) Fix loading saved address data to edit patron form (1.06 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: (QA follow-up) Fix showing patron name when checking in (2.94 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: (QA follow-up) Fix showing cardnumber when creating a hold (1.50 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: (QA follow-up) Use patron object in batch checkout template (4.69 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: (QA follow-up) Use is_child in circulation.pl (1.06 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: (QA follow-up) Update holdshistory.pl, fix is_child, is_adult, patron->image, clear template params (1.46 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: (QA follow-up) Fix viewlog when accessed via circulation (2.57 KB, patch)
2018-02-16 11:13 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18789: Add POD for is_adult and is_child (1.02 KB, patch)
2018-02-16 16:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Fix perlcritic issue - housebound.pl (1.05 KB, patch)
2018-02-16 17:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: (follow-up) Fix page title when creating a new patron (1.86 KB, patch)
2018-02-16 18:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: (follow-up) Fix place hold page (1.39 KB, patch)
2018-03-26 19:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: (follow-up) Pass a Koha::Patron object from returns.pl (2.88 KB, patch)
2018-04-09 19:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18789: Fix borrowernumber on members/files.tt (2.46 KB, patch)
2018-05-10 17:43 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2017-06-12 16:40:02 UTC
The simplify and make uniform the code, the controller scripts should send a Koha::Patron object to the templates and not all attributes of a patron.

That will make the code much more easier to maintain and will be less error-prone.
Comment 1 Jonathan Druart 2017-06-12 16:59:20 UTC
Created attachment 64211 [details] [review]
Bug 18789: Send Koha::Patron object to the templates

In order to simplify and make uniform the code, the controller scripts send
a Koha::Patron object to the templates instead of all attributes of a patron.

That will make the code much more easier to maintain and will be less
error-prone.

The variable "patron" sent to the templates is supposed to represent the
patron the librarian is editing the detail.

In the members module and some scripts of the circulation module, the
patron's detail are sent one by one to the template. That leads to
frustration from developpers (making sure everything is passed from all
scripts) and to regression (we got tone of bugs in the last year because
of this way to do).
With this patch set it will be easy access patron's detail, passing only
1 variable from the controllers.

Test plan:
Play with the patron and circulation module and make sur the detail of
the patron you are editing/seeing info are correctly displayed.
Comment 2 Jonathan Druart 2017-06-12 16:59:24 UTC
Created attachment 64212 [details] [review]
Bug 18789: Pass a Koha::Patron object from returns.pl

This is a specific patch for returns.pl

Test plan:
Do some checkin for a patron and make sure the info are correctly
displayed (and info from someone that has placed a hold on this item)
Comment 3 Jonathan Druart 2017-06-12 16:59:28 UTC
Created attachment 64213 [details] [review]
Bug 18789: Use Koha::Patron->image from the templates

Now that we have the 'patron' variable in all our templates, we can call
Koha::Patron->image and do the check from the templates.

Test plan:
On the different pages of the patron module, you should see the default
image if there is no image attached or the one that has been attached
(see pref patronimages)
Comment 4 Jonathan Druart 2017-06-12 16:59:31 UTC
Created attachment 64214 [details] [review]
Bug 18789: add tests for is_adult and is_child

This patch add 2 methods to the Koha::Patron module: is_adult and
is_child.
That way the business logic is at only one place and covered by tests.
Comment 5 Jonathan Druart 2017-06-12 16:59:35 UTC
Created attachment 64215 [details] [review]
Bug 18789: Use Koha::Patron->is_child where needed

Test plan:
When you are on a page related to a child you should see the "Update
child to adult patron" button
Comment 6 Jonathan Druart 2017-06-12 16:59:38 UTC
Created attachment 64216 [details] [review]
Bug 18789: Use Koha::Patron->is_adult where needed

Test plan:
When editing adult's info you must see the "Add child" button
Comment 7 Jonathan Druart 2017-06-12 16:59:41 UTC
Created attachment 64218 [details] [review]
Bug 18789: Remove useless IS_ADULT variable

Test plan:
  git grep IS_ADULT
must not return any results
Comment 8 Jonathan Druart 2017-06-12 16:59:45 UTC
Created attachment 64219 [details] [review]
Bug 18789: Remove adultborrower from the pay* scripts

This flag is not used in the templates
Comment 9 Jonathan Druart 2017-06-12 16:59:48 UTC
Created attachment 64221 [details] [review]
Bug 18789: Update other occurences where is->child should be used

Trivial changes here, just do a code review on the QA step
Comment 10 Jonathan Druart 2017-06-12 17:20:51 UTC
Created attachment 64229 [details] [review]
Bug 18789: (follow-up)Send Koha::Patron object to the templates
Comment 11 Aleisha Amohia 2017-08-21 00:32:26 UTC
Patch did not apply:

$ git bz apply 18789
Bug 18789 - Send a Koha::Patron object to the templates

64211 - Bug 18789: Send Koha::Patron object to the templates
64212 - Bug 18789: Pass a Koha::Patron object from returns.pl
64213 - Bug 18789: Use Koha::Patron->image from the templates
64214 - Bug 18789: add tests for is_adult and is_child
64215 - Bug 18789: Use Koha::Patron->is_child where needed
64216 - Bug 18789: Use Koha::Patron->is_adult where needed
64218 - Bug 18789: Remove useless IS_ADULT variable
64219 - Bug 18789: Remove adultborrower from the pay* scripts
64221 - Bug 18789: Update other occurences where is->child should be used
64229 - Bug 18789: (follow-up)Send Koha::Patron object to the templates

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 18789: Send Koha::Patron object to the templates
fatal: sha1 information is lacking or useless (acqui/booksellers.pl).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 18789: Send Koha::Patron object to the templates
Comment 12 Jonathan Druart 2017-08-22 15:41:15 UTC
You need to apply the dependencies first.
Comment 13 Jonathan Druart 2018-02-07 18:36:19 UTC
Created attachment 71301 [details] [review]
Bug 18789: Send Koha::Patron object to the templates

In order to simplify and make uniform the code, the controller scripts send
a Koha::Patron object to the templates instead of all attributes of a patron.

That will make the code much more easier to maintain and will be less
error-prone.

The variable "patron" sent to the templates is supposed to represent the
patron the librarian is editing the detail.

In the members module and some scripts of the circulation module, the
patron's detail are sent one by one to the template. That leads to
frustration from developpers (making sure everything is passed from all
scripts) and to regression (we got tone of bugs in the last year because
of this way to do).
With this patch set it will be easy access patron's detail, passing only
1 variable from the controllers.

Test plan:
Play with the patron and circulation module and make sur the detail of
the patron you are editing/seeing info are correctly displayed.
Comment 14 Jonathan Druart 2018-02-07 18:36:24 UTC
Created attachment 71302 [details] [review]
Bug 18789: Pass a Koha::Patron object from returns.pl

This is a specific patch for returns.pl

Test plan:
Do some checkin for a patron and make sure the info are correctly
displayed (and info from someone that has placed a hold on this item)
Comment 15 Jonathan Druart 2018-02-07 18:36:28 UTC
Created attachment 71303 [details] [review]
Bug 18789: Use Koha::Patron->image from the templates

Now that we have the 'patron' variable in all our templates, we can call
Koha::Patron->image and do the check from the templates.

Test plan:
On the different pages of the patron module, you should see the default
image if there is no image attached or the one that has been attached
(see pref patronimages)
Comment 16 Jonathan Druart 2018-02-07 18:36:33 UTC
Created attachment 71304 [details] [review]
Bug 18789: add tests for is_adult and is_child

This patch add 2 methods to the Koha::Patron module: is_adult and
is_child.
That way the business logic is at only one place and covered by tests.
Comment 17 Jonathan Druart 2018-02-07 18:36:37 UTC
Created attachment 71305 [details] [review]
Bug 18789: Use Koha::Patron->is_child where needed

Test plan:
When you are on a page related to a child you should see the "Update
child to adult patron" button
Comment 18 Jonathan Druart 2018-02-07 18:36:41 UTC
Created attachment 71306 [details] [review]
Bug 18789: Use Koha::Patron->is_adult where needed

Test plan:
When editing adult's info you must see the "Add child" button
Comment 19 Jonathan Druart 2018-02-07 18:36:44 UTC
Created attachment 71307 [details] [review]
Bug 18789: Remove useless IS_ADULT variable

Test plan:
  git grep IS_ADULT
must not return any results
Comment 20 Jonathan Druart 2018-02-07 18:36:48 UTC
Created attachment 71308 [details] [review]
Bug 18789: Remove adultborrower from the pay* scripts

This flag is not used in the templates
Comment 21 Jonathan Druart 2018-02-07 18:36:52 UTC
Created attachment 71309 [details] [review]
Bug 18789: Update other occurences where is->child should be used

Trivial changes here, just do a code review on the QA step
Comment 22 Jonathan Druart 2018-02-07 18:36:55 UTC
Created attachment 71310 [details] [review]
Bug 18789: (follow-up)Send Koha::Patron object to the templates
Comment 23 Jonathan Druart 2018-02-07 18:38:20 UTC
This is very hard to rebase and will introduce a lot of issues if not pushed quickly.
Comment 24 Jonathan Druart 2018-02-07 18:54:49 UTC
Created attachment 71311 [details] [review]
Bug 18789: (follow-up) Use Koha::Patron->image from the templates
Comment 25 Kyle M Hall 2018-02-12 17:54:22 UTC
Created attachment 71485 [details] [review]
Bug 18789: Send Koha::Patron object to the templates

In order to simplify and make uniform the code, the controller scripts send
a Koha::Patron object to the templates instead of all attributes of a patron.

That will make the code much more easier to maintain and will be less
error-prone.

The variable "patron" sent to the templates is supposed to represent the
patron the librarian is editing the detail.

In the members module and some scripts of the circulation module, the
patron's detail are sent one by one to the template. That leads to
frustration from developpers (making sure everything is passed from all
scripts) and to regression (we got tone of bugs in the last year because
of this way to do).
With this patch set it will be easy access patron's detail, passing only
1 variable from the controllers.

Test plan:
Play with the patron and circulation module and make sur the detail of
the patron you are editing/seeing info are correctly displayed.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 26 Kyle M Hall 2018-02-12 17:54:29 UTC
Created attachment 71486 [details] [review]
Bug 18789: Pass a Koha::Patron object from returns.pl

This is a specific patch for returns.pl

Test plan:
Do some checkin for a patron and make sure the info are correctly
displayed (and info from someone that has placed a hold on this item)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 27 Kyle M Hall 2018-02-12 17:54:32 UTC
Created attachment 71487 [details] [review]
Bug 18789: Use Koha::Patron->image from the templates

Now that we have the 'patron' variable in all our templates, we can call
Koha::Patron->image and do the check from the templates.

Test plan:
On the different pages of the patron module, you should see the default
image if there is no image attached or the one that has been attached
(see pref patronimages)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 28 Kyle M Hall 2018-02-12 17:54:34 UTC
Created attachment 71488 [details] [review]
Bug 18789: add tests for is_adult and is_child

This patch add 2 methods to the Koha::Patron module: is_adult and
is_child.
That way the business logic is at only one place and covered by tests.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 29 Kyle M Hall 2018-02-12 17:54:37 UTC
Created attachment 71489 [details] [review]
Bug 18789: Use Koha::Patron->is_child where needed

Test plan:
When you are on a page related to a child you should see the "Update
child to adult patron" button

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 30 Kyle M Hall 2018-02-12 17:54:40 UTC
Created attachment 71490 [details] [review]
Bug 18789: Use Koha::Patron->is_adult where needed

Test plan:
When editing adult's info you must see the "Add child" button

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 31 Kyle M Hall 2018-02-12 17:54:42 UTC
Created attachment 71491 [details] [review]
Bug 18789: Remove useless IS_ADULT variable

Test plan:
  git grep IS_ADULT
must not return any results

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 32 Kyle M Hall 2018-02-12 17:54:45 UTC
Created attachment 71492 [details] [review]
Bug 18789: Remove adultborrower from the pay* scripts

This flag is not used in the templates

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 33 Kyle M Hall 2018-02-12 17:54:48 UTC
Created attachment 71493 [details] [review]
Bug 18789: Update other occurences where is->child should be used

Trivial changes here, just do a code review on the QA step

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 34 Kyle M Hall 2018-02-12 17:54:51 UTC
Created attachment 71494 [details] [review]
Bug 18789: (follow-up)Send Koha::Patron object to the templates

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 35 Kyle M Hall 2018-02-12 17:54:53 UTC
Created attachment 71495 [details] [review]
Bug 18789: (follow-up) Use Koha::Patron->image from the templates

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 36 Kyle M Hall 2018-02-12 17:54:56 UTC
Created attachment 71496 [details] [review]
Bug 18789: Fix number of unit tests

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 37 Jonathan Druart 2018-02-15 18:49:12 UTC
Created attachment 71679 [details] [review]
Bug 18789: Send Koha::Patron object to the templates

In order to simplify and make uniform the code, the controller scripts send
a Koha::Patron object to the templates instead of all attributes of a patron.

That will make the code much more easier to maintain and will be less
error-prone.

The variable "patron" sent to the templates is supposed to represent the
patron the librarian is editing the detail.

In the members module and some scripts of the circulation module, the
patron's detail are sent one by one to the template. That leads to
frustration from developpers (making sure everything is passed from all
scripts) and to regression (we got tone of bugs in the last year because
of this way to do).
With this patch set it will be easy access patron's detail, passing only
1 variable from the controllers.

Test plan:
Play with the patron and circulation module and make sur the detail of
the patron you are editing/seeing info are correctly displayed.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 38 Jonathan Druart 2018-02-15 18:49:18 UTC
Created attachment 71680 [details] [review]
Bug 18789: Pass a Koha::Patron object from returns.pl

This is a specific patch for returns.pl

Test plan:
Do some checkin for a patron and make sure the info are correctly
displayed (and info from someone that has placed a hold on this item)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 39 Jonathan Druart 2018-02-15 18:49:26 UTC
Created attachment 71681 [details] [review]
Bug 18789: Use Koha::Patron->image from the templates

Now that we have the 'patron' variable in all our templates, we can call
Koha::Patron->image and do the check from the templates.

Test plan:
On the different pages of the patron module, you should see the default
image if there is no image attached or the one that has been attached
(see pref patronimages)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 40 Jonathan Druart 2018-02-15 18:49:33 UTC
Created attachment 71682 [details] [review]
Bug 18789: add tests for is_adult and is_child

This patch add 2 methods to the Koha::Patron module: is_adult and
is_child.
That way the business logic is at only one place and covered by tests.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 41 Jonathan Druart 2018-02-15 18:49:43 UTC
Created attachment 71683 [details] [review]
Bug 18789: Use Koha::Patron->is_child where needed

Test plan:
When you are on a page related to a child you should see the "Update
child to adult patron" button

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 42 Jonathan Druart 2018-02-15 18:49:50 UTC
Created attachment 71684 [details] [review]
Bug 18789: Use Koha::Patron->is_adult where needed

Test plan:
When editing adult's info you must see the "Add child" button

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 43 Jonathan Druart 2018-02-15 18:49:57 UTC
Created attachment 71685 [details] [review]
Bug 18789: Remove useless IS_ADULT variable

Test plan:
  git grep IS_ADULT
must not return any results

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 44 Jonathan Druart 2018-02-15 18:50:04 UTC
Created attachment 71686 [details] [review]
Bug 18789: Remove adultborrower from the pay* scripts

This flag is not used in the templates

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 45 Jonathan Druart 2018-02-15 18:50:10 UTC
Created attachment 71687 [details] [review]
Bug 18789: Update other occurences where is->child should be used

Trivial changes here, just do a code review on the QA step

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 46 Jonathan Druart 2018-02-15 18:50:17 UTC
Created attachment 71688 [details] [review]
Bug 18789: (follow-up)Send Koha::Patron object to the templates

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 47 Jonathan Druart 2018-02-15 18:50:22 UTC
Created attachment 71689 [details] [review]
Bug 18789: (follow-up) Use Koha::Patron->image from the templates

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 48 Jonathan Druart 2018-02-15 18:50:27 UTC
Created attachment 71690 [details] [review]
Bug 18789: add tests for is_adult and is_chil
Comment 49 Jonathan Druart 2018-02-15 18:50:31 UTC
Created attachment 71691 [details] [review]
Bug 18789: Use the patron variable in circulation.[pl|tt]
Comment 50 Josef Moravec 2018-02-16 11:12:33 UTC
Created attachment 71713 [details] [review]
Bug 18789: Send Koha::Patron object to the templates

In order to simplify and make uniform the code, the controller scripts send
a Koha::Patron object to the templates instead of all attributes of a patron.

That will make the code much more easier to maintain and will be less
error-prone.

The variable "patron" sent to the templates is supposed to represent the
patron the librarian is editing the detail.

In the members module and some scripts of the circulation module, the
patron's detail are sent one by one to the template. That leads to
frustration from developpers (making sure everything is passed from all
scripts) and to regression (we got tone of bugs in the last year because
of this way to do).
With this patch set it will be easy access patron's detail, passing only
1 variable from the controllers.

Test plan:
Play with the patron and circulation module and make sur the detail of
the patron you are editing/seeing info are correctly displayed.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 51 Josef Moravec 2018-02-16 11:12:37 UTC
Created attachment 71714 [details] [review]
Bug 18789: Pass a Koha::Patron object from returns.pl

This is a specific patch for returns.pl

Test plan:
Do some checkin for a patron and make sure the info are correctly
displayed (and info from someone that has placed a hold on this item)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 52 Josef Moravec 2018-02-16 11:12:40 UTC
Created attachment 71715 [details] [review]
Bug 18789: Use Koha::Patron->image from the templates

Now that we have the 'patron' variable in all our templates, we can call
Koha::Patron->image and do the check from the templates.

Test plan:
On the different pages of the patron module, you should see the default
image if there is no image attached or the one that has been attached
(see pref patronimages)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 53 Josef Moravec 2018-02-16 11:12:44 UTC
Created attachment 71716 [details] [review]
Bug 18789: Add tests for is_adult and is_child

This patch add 2 methods to the Koha::Patron module: is_adult and
is_child.
That way the business logic is at only one place and covered by tests.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 54 Josef Moravec 2018-02-16 11:12:48 UTC
Created attachment 71717 [details] [review]
Bug 18789: Use Koha::Patron->is_child where needed

Test plan:
When you are on a page related to a child you should see the "Update
child to adult patron" button

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 55 Josef Moravec 2018-02-16 11:12:51 UTC
Created attachment 71718 [details] [review]
Bug 18789: Use Koha::Patron->is_adult where needed

Test plan:
When editing adult's info you must see the "Add child" button

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 56 Josef Moravec 2018-02-16 11:12:55 UTC
Created attachment 71719 [details] [review]
Bug 18789: Remove useless IS_ADULT variable

Test plan:
  git grep IS_ADULT
must not return any results

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 57 Josef Moravec 2018-02-16 11:12:58 UTC
Created attachment 71720 [details] [review]
Bug 18789: Remove adultborrower from the pay* scripts

This flag is not used in the templates

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 58 Josef Moravec 2018-02-16 11:13:02 UTC
Created attachment 71721 [details] [review]
Bug 18789: Update other occurences where is_child should be used

Trivial changes here, just do a code review on the QA step

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 59 Josef Moravec 2018-02-16 11:13:05 UTC
Created attachment 71722 [details] [review]
Bug 18789: (follow-up) Send Koha::Patron object to the templates

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 60 Josef Moravec 2018-02-16 11:13:09 UTC
Created attachment 71723 [details] [review]
Bug 18789: (follow-up) Use Koha::Patron->image from the templates

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 61 Josef Moravec 2018-02-16 11:13:12 UTC
Created attachment 71724 [details] [review]
Bug 18789: Add tests for is_adult and is_child

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 62 Josef Moravec 2018-02-16 11:13:16 UTC
Created attachment 71725 [details] [review]
Bug 18789: Use the patron variable in circulation.[pl|tt]

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 63 Josef Moravec 2018-02-16 11:13:19 UTC
Created attachment 71726 [details] [review]
Bug 18789: (QA follow-up) Fix tab char

Test plan:

run koha qa tools, should be green

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 64 Josef Moravec 2018-02-16 11:13:23 UTC
Created attachment 71727 [details] [review]
Bug 18789: (QA follow-up) Fix borrowernumber in menu item links

Test plan:
Go to patrons routing list
Try to click Holds history or ILL requests menu link
--> without page blank page
--> with page desired page

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 65 Josef Moravec 2018-02-16 11:13:27 UTC
Created attachment 71728 [details] [review]
Bug 18789: (QA follow-up) Fix showing the name of basket creator on booksellers page

Test plan:
1) Go to Acquisition and search for a vendor, you are presented with vendors and their baskets
2) Look to Created by column
--> without patch only firstname is shown
--> with patch you should see full name

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 66 Josef Moravec 2018-02-16 11:13:30 UTC
Created attachment 71729 [details] [review]
Bug 18789: (QA follow-up) Use first_valid_email_address on Tranfers to receive page

For consistancy with waitng reserves

Test plan:
1) Have a hold for patron which need to be transfered, inititate
transfer and go to target library
2) Go to Circulation -> transfers to receive page and you should see any
first valid email for tha patron in column "On hold for"

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 67 Josef Moravec 2018-02-16 11:13:34 UTC
Created attachment 71730 [details] [review]
Bug 18789: (QA follow-up) Fix loading saved address data to edit patron form

Test plan:
1) Have patron with address filled in
2) Edit the patron
-- without this patch the fields for address are blank
-- with patch the fields are filled with actual data

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 68 Josef Moravec 2018-02-16 11:13:37 UTC
Created attachment 71731 [details] [review]
Bug 18789: (QA follow-up) Fix showing patron name when checking in

Test plan:
1) Have a patron with more then one checkouts
2) Check in one of the checkouts
--> without patch patron name is not shown in checked-in items table
--> with patch the patron is shown with name, category, number of
checkouts and both links are working

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 69 Josef Moravec 2018-02-16 11:13:41 UTC
Created attachment 71732 [details] [review]
Bug 18789: (QA follow-up) Fix showing cardnumber when creating a hold

Test plan:
1) Try to create a hold request for a patron
2) On place hold form on the first line is shown the patron name
--> without patch the cardnumber is not shown
--> with patch there is cardnumber right after the patron name

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 70 Josef Moravec 2018-02-16 11:13:45 UTC
Created attachment 71733 [details] [review]
Bug 18789: (QA follow-up) Use patron object in batch checkout template

1) Enable batch checkouts in system preferences and set it on for some
patron categories
2) Try to batch checkout for some patron with one of these categories
--> without patch the batch checkout page is even not shown
--> with patch batch checking out is working as expected

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 71 Josef Moravec 2018-02-16 11:13:48 UTC
Created attachment 71734 [details] [review]
Bug 18789: (QA follow-up) Use is_child in circulation.pl

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 72 Josef Moravec 2018-02-16 11:13:52 UTC
Created attachment 71735 [details] [review]
Bug 18789: (QA follow-up) Update holdshistory.pl, fix is_child, is_adult, patron->image, clear template params

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 73 Josef Moravec 2018-02-16 11:13:55 UTC
Created attachment 71736 [details] [review]
Bug 18789: (QA follow-up) Fix viewlog when accessed via circulation

Test plan:
Go to any patron and then to Modification log of this patron
-- without patch you will not see the left side menu and patron information
-- with patch menu and patron information is here, and should be fully
working (links)

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 74 Josef Moravec 2018-02-16 11:16:39 UTC
Good job Jonathan, I just added some small fixes.

I think that memberentry.pl and memberentrygen.tt will need some refactoring too, but I am for to do it on another report, so passing qa.
Comment 75 Jonathan Druart 2018-02-16 16:57:34 UTC
Created attachment 71827 [details] [review]
Bug 18789: Add POD for is_adult and is_child
Comment 76 Jonathan Druart 2018-02-16 17:06:36 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 77 Jonathan Druart 2018-02-16 17:28:18 UTC
Created attachment 71838 [details] [review]
Bug 18789: Fix perlcritic issue - housebound.pl

"$branch" is declared but not used at line 65, column 1.  Unused
variables clutter code and make it harder to read.  (Severity: 3)
Comment 78 Jonathan Druart 2018-02-16 18:04:32 UTC
Created attachment 71841 [details] [review]
Bug 18789: (follow-up) Fix page title when creating a new patron

This has been caught by selenium test, the category name must be
displayed when we are creating a new patron, and so does not depends on
the "patron" variable
Comment 79 Nick Clemens 2018-03-23 01:05:44 UTC
Enhancement, not backported for 17.11
Comment 80 Jonathan Druart 2018-03-26 19:59:27 UTC
Created attachment 73293 [details] [review]
Bug 18789: (follow-up) Fix place hold page

This patch fixes very weird behaviours introduced by
  commit 0ab22e1c7c6b6e8d14a4fb7478ab2a3d42da347d
  Bug 18789: Send Koha::Patron object to the templates

The patron variable was only set when no action was defined.
This patch restores the feature the easiest way possible (less changes)
even if it is not the best one.
Comment 81 Jonathan Druart 2018-03-26 20:01:22 UTC
Last patch pushed to master
Comment 82 Jonathan Druart 2018-04-09 19:00:00 UTC
Created attachment 73890 [details] [review]
Bug 18789: (follow-up) Pass a Koha::Patron object from returns.pl
Comment 83 Jonathan Druart 2018-04-09 19:16:04 UTC
Last patch pushed to master
Comment 84 Jonathan Druart 2018-05-10 17:43:05 UTC
Created attachment 75237 [details] [review]
Bug 18789: Fix borrowernumber on members/files.tt
Comment 85 Jonathan Druart 2018-05-10 17:43:48 UTC
(In reply to Jonathan Druart from comment #84)
> Created attachment 75237 [details] [review] [review]
> Bug 18789: Fix borrowernumber on members/files.tt

Pushed to master!