Bug 30807 - Use patron-title.inc in patron payments pages
Summary: Use patron-title.inc in patron payments pages
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks: 30617
  Show dependency treegraph
 
Reported: 2022-05-19 11:52 UTC by Martin Renvoize
Modified: 2022-06-08 14:50 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement updates the templates for patron accounting - make a payment tab and payment pages (pay and write off options) to use the patron-title.inc include wherever patron names are referenced. This is used to format patron name names consistently, rather than a custom format each time the patron name is referenced. The patron name is now displayed as "Make a payment for firstname lastname (patron card number)" and "Pay charges for firstname lastname (patron card number)".
Version(s) released in:
22.11.00


Attachments
Bug 30807: Migrate to patron-title in pay and paycollect (5.80 KB, patch)
2022-05-19 11:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30807: Migrate to patron-title in pay and paycollect (5.85 KB, patch)
2022-05-23 18:45 UTC, David Nind
Details | Diff | Splinter Review
Bug 30807: Migrate to patron-title in pay and paycollect (5.92 KB, patch)
2022-05-31 07:28 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 Martin Renvoize 2022-05-19 11:52:52 UTC
We should use the patron-title include everwhere we display a patron title instead of templates mixing up formats.
Comment 1 Martin Renvoize 2022-05-19 11:57:21 UTC
Created attachment 135201 [details] [review]
Bug 30807: Migrate to patron-title in pay and paycollect

This patch updates the member-flags template to use the patron-title.inc
include wherever patron names are referenced.

Test plan
1) Navigate to a patron and click through to their accounts tab
2) Note how the patron name displays in the title, breadcrumb and
   headings
3) Click through to make a payment
4) Note how the patron name displays in the title, breadcrumb and
   headings
5) Apply the patch and reload the page
6) Confirm the patron name still appears in each location and is
   consistently formatted and linked as you would expect

Note: This patch also removes a superflous hidden title form element
that was simply passed to and from the controller but not actually used
in any way.
Comment 2 David Nind 2022-05-23 18:45:49 UTC
Created attachment 135285 [details] [review]
Bug 30807: Migrate to patron-title in pay and paycollect

This patch updates the member-flags template to use the patron-title.inc
include wherever patron names are referenced.

Test plan
1) Navigate to a patron and click through to their accounts tab
2) Note how the patron name displays in the title, breadcrumb and
   headings
3) Click through to make a payment
4) Note how the patron name displays in the title, breadcrumb and
   headings
5) Apply the patch and reload the page
6) Confirm the patron name still appears in each location and is
   consistently formatted and linked as you would expect

Note: This patch also removes a superflous hidden title form element
that was simply passed to and from the controller but not actually used
in any way.

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2022-05-23 19:07:57 UTC
The patch description needs updating - it currently says "...the member-flags template..." when it covers the patron payments pages. 8-)
Comment 4 David Nind 2022-05-23 22:58:12 UTC
Also, do we need to update the text for Accounting > Make a payment > Pay (or Write off) when a payment is made or a charge is written off? It currently says "fine", when I think we are now using "charge".

It currently says:
- For pay: "Pay an individual fine" as the title, breadcrumb, and block title after the Pay and Write off buttons, and "Collect fine payment for XXXX" in the browser tab title.
- For write off "Write off an individual fine" as the title, breadcrumb, and block title after the Pay and Write off buttons, and "Collect fine payment for XXXX" in the browser tab title.

If so, probably a separate bug... am happy to create.
Comment 5 Jonathan Druart 2022-05-31 07:27:40 UTC
Certainly a coding preference, but I personally prefer to explicitly pass the variable, even if not needed, like

[% INCLUDE 'patron-title.inc' patron=patron %]
instead of
[% INCLUDE 'patron-title.inc' %]

It prevent future cleaning to easily miss a variable that would be called from an include file.
Comment 6 Jonathan Druart 2022-05-31 07:28:07 UTC
Created attachment 135470 [details] [review]
Bug 30807: Migrate to patron-title in pay and paycollect

This patch updates the member-flags template to use the patron-title.inc
include wherever patron names are referenced.

Test plan
1) Navigate to a patron and click through to their accounts tab
2) Note how the patron name displays in the title, breadcrumb and
   headings
3) Click through to make a payment
4) Note how the patron name displays in the title, breadcrumb and
   headings
5) Apply the patch and reload the page
6) Confirm the patron name still appears in each location and is
   consistently formatted and linked as you would expect

Note: This patch also removes a superflous hidden title form element
that was simply passed to and from the controller but not actually used
in any way.

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Tomás Cohen Arazi 2022-06-08 14:50:59 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!