Bug 21757

Summary: Patron detail script (moremember.pl) cleanup
Product: Koha Reporter: Josef Moravec <josef.moravec>
Component: Architecture, internals, and plumbingAssignee: Josef Moravec <josef.moravec>
Status: CLOSED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: enhancement    
Priority: P5 - low CC: black23, david, martin.renvoize, nick, oleonard, sandboxes, severine.queune
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This enhancement tidies up the patron detail script (moremember.pl) and removes unused templates, uses objects as much as possible, and removes many template parameters.
Version(s) released in:
Circulation function:
Bug Depends on:    
Bug Blocks: 29889    
Attachments: Bug 21757: Clenup of moremember.pl and its templates
Bug 21757: Clenup of moremember.pl and its templates
Bug 21757: Clenup of moremember.pl and its templates
Bug 21757: Clenup of moremember.pl and its templates
Bug 21757: Clenup of moremember.pl and its templates
Bug 21757: Clenup of moremember.pl and its templates
Bug 21757: (folow-up) Restore patron brief detail page
Bug 21757: Clenup of moremember.pl and its templates
Bug 21757: (folow-up) Restore patron brief detail page
Bug 21757: (follow-up) Fix number of columns in checkouts table footer template
Bug 21757: Clenup of moremember.pl and its templates
Bug 21757: (folow-up) Restore patron brief detail page
Bug 21757: (follow-up) Fix number of columns in checkouts table footer template

Description Josef Moravec 2018-11-02 17:39:56 UTC
The moremember.pl script is too long ;)
Comment 1 Josef Moravec 2018-11-02 17:50:03 UTC Comment hidden (obsolete)
Comment 2 Séverine Queune 2019-02-27 14:08:47 UTC
Hi Josef, 
Here is the mail I've just received :

The sandbox you've requested is not ready.
Some problems occurred applying patches from bug 21757:
<h1>Something went wrong !</h1>Applying: Bug 21757: Clenup of moremember.pl and its templates
Using index info to reconstruct a base tree...
M       koha-tmpl/intranet-tmpl/prog/en/includes/checkouts-table-footer.inc
M       koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-print.tt
M       koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
M       members/moremember.pl
Falling back to patching base and 3-way merge...
Auto-merging members/moremember.pl
CONFLICT (content): Merge conflict in members/moremember.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
Removing koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-receipt.tt
CONFLICT (modify/delete): koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-print.tt deleted in Bug 21757: Clenup of moremember.pl and its templates and modified in 6533f32c3a7acdaaa6769e6039472045add847e2. Version 6533f32c3a7acdaaa6769e6039472045add847e2 of koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-print.tt left in tree.
Removing koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember-brief.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/checkouts-table-footer.inc
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/includes/checkouts-table-footer.inc
error: Failed to merge in the changes.
Patch failed at 0001 Bug 21757: Clenup of moremember.pl and its templates
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.
Bug 21757 - Patron detail script (moremember.pl) cleanup

81916 - Bug 21757: Clenup of moremember.pl and its templates

Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-21757-Clenup-of-morememberpl-and-its-templates-ZONSXB.patch .
Comment 3 Josef Moravec 2019-02-27 14:58:19 UTC
Created attachment 85782 [details] [review]
Bug 21757: Clenup of moremember.pl and its templates

This patch:
  - removes unused templates
  - use objects as much as possible
  - remove many template params

Test plan:
1) Apply the patch
2) Play with patron detail page and try to broke it anyhow ;)
- messaging preferences
- enhanced attributes
- guarantors and guarantees
- fines
- messages
- checkouts
- overdues
- use different date formats, price formats and address formats
- ... there is many thinks you could try with this one page ;)
Comment 4 Josef Moravec 2019-02-27 15:04:28 UTC
(In reply to Séverine Queune from comment #2)
> Hi Josef, 
> Here is the mail I've just received :
> 
> The sandbox you've requested is not ready.

Thanks for testing! Patch is rebased now.
Comment 5 Michal Denar 2019-04-14 18:59:03 UTC
Hi Josef,
can you rebase this patch on master?

Thank you.
Comment 6 Josef Moravec 2019-04-15 14:25:37 UTC
Created attachment 88007 [details] [review]
Bug 21757: Clenup of moremember.pl and its templates

This patch:
  - removes unused templates
  - use objects as much as possible
  - remove many template params

Test plan:
1) Apply the patch
2) Play with patron detail page and try to broke it anyhow ;)
- messaging preferences
- enhanced attributes
- guarantors and guarantees
- fines
- messages
- checkouts
- overdues
- use different date formats, price formats and address formats
- ... there is many thinks you could try with this one page ;)
Comment 7 Josef Moravec 2019-04-15 14:26:14 UTC
(In reply to Michal Denar from comment #5)
> Hi Josef,
> can you rebase this patch on master?
> 
> Thank you.

Rebased, thanks for testing
Comment 8 Michal Denar 2019-04-15 19:38:01 UTC
Josef,
I've tried to broke patron page.

I found just one problem.
Print summary give error:
Template process failed: file error - /home/vagrant/kohaclone/koha-tmpl/intranet-tmpl//en/modules/members/moremember-print.tt: not found at /home/vagrant/kohaclone/C4/Templates.pm line 122
Comment 9 Séverine Queune 2019-04-16 10:01:22 UTC
Hi Josef,
There is also the alert "Patron's card has been reported lost." when "Lost card" is checked that has disappeared.
Comment 10 Josef Moravec 2019-04-16 11:16:56 UTC
Created attachment 88058 [details] [review]
Bug 21757: Clenup of moremember.pl and its templates

This patch:
  - removes unused templates
  - use objects as much as possible
  - remove many template params

Test plan:
1) Apply the patch
2) Play with patron detail page and try to broke it anyhow ;)
- messaging preferences
- enhanced attributes
- guarantors and guarantees
- fines
- messages
- checkouts
- overdues
- use different date formats, price formats and address formats
- ... there is many thinks you could try with this one page ;)
Comment 11 Josef Moravec 2019-04-16 11:19:15 UTC
(In reply to Michal Denar from comment #8)
> Josef,
> I've tried to broke patron page.
> 
> I found just one problem.
> Print summary give error:
> Template process failed: file error -
> /home/vagrant/kohaclone/koha-tmpl/intranet-tmpl//en/modules/members/
> moremember-print.tt: not found at /home/vagrant/kohaclone/C4/Templates.pm
> line 122

(In reply to Séverine Queune from comment #9)
> Hi Josef,
> There is also the alert "Patron's card has been reported lost." when "Lost
> card" is checked that has disappeared.

Thanks for testing Michal and Séverine,

I addressed both of the issues, so ready for another round ;)
Comment 12 Biblibre Sandboxes 2019-04-16 11:53:10 UTC
Patch tested with a sandbox, by Séverine QUEUNE <severine.queune@bulac.fr>
Comment 13 Biblibre Sandboxes 2019-04-16 11:53:31 UTC
Created attachment 88060 [details] [review]
Bug 21757: Clenup of moremember.pl and its templates

This patch:
  - removes unused templates
  - use objects as much as possible
  - remove many template params

Test plan:
1) Apply the patch
2) Play with patron detail page and try to broke it anyhow ;)
- messaging preferences
- enhanced attributes
- guarantors and guarantees
- fines
- messages
- checkouts
- overdues
- use different date formats, price formats and address formats
- ... there is many thinks you could try with this one page ;)

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 14 Katrin Fischer 2019-04-17 05:41:18 UTC
Can you please rebase?
Comment 15 Josef Moravec 2019-04-17 06:36:10 UTC
Created attachment 88146 [details] [review]
Bug 21757: Clenup of moremember.pl and its templates

This patch:
  - removes unused templates
  - use objects as much as possible
  - remove many template params

Test plan:
1) Apply the patch
2) Play with patron detail page and try to broke it anyhow ;)
- messaging preferences
- enhanced attributes
- guarantors and guarantees
- fines
- messages
- checkouts
- overdues
- use different date formats, price formats and address formats
- ... there is many thinks you could try with this one page ;)

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 16 Josef Moravec 2019-04-17 06:36:44 UTC
(In reply to Katrin Fischer from comment #14)
> Can you please rebase?

done ;)
Comment 17 Katrin Fischer 2019-04-18 05:43:22 UTC
This changes the beahaviour of the patron duplicate message as the template for displaying the data of the duplicate candidate patron is removed. I am not sure about this change and it's not described in the commit messsage.

Can you please take a look?
Comment 18 Josef Moravec 2019-04-18 08:42:31 UTC
Created attachment 88268 [details] [review]
Bug 21757: (folow-up) Restore patron brief detail page

Test plan
1) Try to create patron with same name/datefobirth to trigger the
duplicate patron check
2) Click on "View existing record" link on "Duplicate patron record?"
alert message
3) You should see the patron data, but withou toolbar, menu, and so on.
Comment 19 Katrin Fischer 2019-04-24 06:21:45 UTC
Hi Josef,
the duplicate dialog now works as expected, but I have issues loading the checkouts table (datatable errors, endless loading). Also the tabs are broken. Please check!
Comment 20 Josef Moravec 2019-04-24 08:03:08 UTC
Created attachment 88595 [details] [review]
Bug 21757: Clenup of moremember.pl and its templates

This patch:
  - removes unused templates
  - use objects as much as possible
  - remove many template params

Test plan:
1) Apply the patch
2) Play with patron detail page and try to broke it anyhow ;)
- messaging preferences
- enhanced attributes
- guarantors and guarantees
- fines
- messages
- checkouts
- overdues
- use different date formats, price formats and address formats
- ... there is many thinks you could try with this one page ;)

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Comment 21 Josef Moravec 2019-04-24 08:03:12 UTC
Created attachment 88596 [details] [review]
Bug 21757: (folow-up) Restore patron brief detail page

Test plan
1) Try to create patron with same name/datefobirth to trigger the
duplicate patron check
2) Click on "View existing record" link on "Duplicate patron record?"
alert message
3) You should see the patron data, but withou toolbar, menu, and so on.
Comment 22 Josef Moravec 2019-04-24 08:03:15 UTC
Created attachment 88597 [details] [review]
Bug 21757: (follow-up) Fix number of columns in checkouts table footer template
Comment 23 Josef Moravec 2019-04-24 08:04:11 UTC
(In reply to Katrin Fischer from comment #19)
> Hi Josef,
> the duplicate dialog now works as expected, but I have issues loading the
> checkouts table (datatable errors, endless loading). Also the tabs are
> broken. Please check!

Thanks for testing, fixed in follow-up.
Comment 24 Katrin Fischer 2019-04-24 20:30:19 UTC
Created attachment 88643 [details] [review]
Bug 21757: Clenup of moremember.pl and its templates

This patch:
  - removes unused templates
  - use objects as much as possible
  - remove many template params

Test plan:
1) Apply the patch
2) Play with patron detail page and try to broke it anyhow ;)
- messaging preferences
- enhanced attributes
- guarantors and guarantees
- fines
- messages
- checkouts
- overdues
- use different date formats, price formats and address formats
- ... there is many thinks you could try with this one page ;)

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Katrin Fischer 2019-04-24 20:30:22 UTC
Created attachment 88644 [details] [review]
Bug 21757: (folow-up) Restore patron brief detail page

Test plan
1) Try to create patron with same name/datefobirth to trigger the
duplicate patron check
2) Click on "View existing record" link on "Duplicate patron record?"
alert message
3) You should see the patron data, but withou toolbar, menu, and so on.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Katrin Fischer 2019-04-24 20:30:26 UTC
Created attachment 88645 [details] [review]
Bug 21757: (follow-up) Fix number of columns in checkouts table footer template

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 27 Nick Clemens (kidclamp) 2019-04-29 16:20:39 UTC
Awesome work all!

Pushed to master for 19.05
Comment 28 Martin Renvoize (ashimema) 2019-05-08 14:03:39 UTC
Enhancement will not be backported for 18.11.x series