Bug 11804 - Merge circ-menu.tt and a circ-menu.inc
Summary: Merge circ-menu.tt and a circ-menu.inc
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Katrin Fischer
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-20 19:29 UTC by Galen Charlton
Modified: 2016-06-21 21:38 UTC (History)
7 users (show)

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


Attachments
Bug 11804: Remove unused circ-menu.tt (6.87 KB, patch)
2015-06-06 11:05 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11804: Remove unused circ-menu.tt (7.00 KB, patch)
2015-06-18 15:39 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 14409: Remove references to circ-menu.tt (1.88 KB, patch)
2015-06-18 16:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11804: Remove unused circ-menu.tt (7.06 KB, patch)
2015-06-18 16:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11804: Remove references to circ-menu.tt (1.76 KB, patch)
2015-06-18 16:20 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 Galen Charlton 2014-02-20 19:29:55 UTC
koha/koha-tmpl/intranet-tmpl/prog/en/includes/ contains both a circ-menu.tt and a circ-menu.inc.  circ-menu.tt contains the following comment:

[%# duplicates circ-menu.inc but assumes all borrower attributes are in a borrower variable rather than
in the global namespace %]

circ-menu.tt is used only by the following templates:

members/pay.tt
members/paycollect.tt

This is a blatant violation of the DRY principle, and some inconsistencies have appeared over the years:

* the purchase suggestions tab doesn't show up when using the fine payment page
* the logic for displaying the notices tab differs

The duplication should be removed, perhaps as part of a larger project to make all of the patron and circulation pages use the same routine to grab information about the current patron.
Comment 1 paxed 2014-04-15 10:47:26 UTC
Some additional differences have creeped into the files:

-use of INCLUDE 'patron-title.inc' vs. hard-coded patron title.
-IF-checking picture vs. checking has_picture.
-TT filtering borrowernumber with uri vs. not filtering.


Other stuff that could be improved:

-Hard-coded css styles should be moved into external stylesheets.
-patroncategory and patronlibrary do not separate field description from the field content, so impossible to style separately.
Comment 2 Katrin Fischer 2015-01-09 12:50:09 UTC
Good catch, thx for linking to the other bug paxed.
Comment 3 Marc Véron 2015-06-02 12:00:00 UTC
This bug seems to be resolved. Verifyed on current master:

koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt
line 230:
[% INCLUDE 'circ-menu.inc' %]

koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt
line 175:
[% INCLUDE 'circ-menu.inc' %]

git grep 'circ-menu.tt'
members/boraccount.pl:#workaround for Bug 4041 to get rid of duplicated code in circ-menu.tt, use circ-menu.inc instad
members/pay.pl:    #workaround for Bug 4041 to get rid of duplicated code in circ-menu.tt, use circ-menu.inc instad
members/paycollect.pl:#workaround for Bug 4041 to get rid of duplicated code in circ-menu.tt, use circ-menu.inc instad
(...)
Comment 4 Katrin Fischer 2015-06-06 11:01:56 UTC
Hi Mark,

I think you are right, but it looks like we didn't remove the file. I will submit a patch.
Comment 5 Katrin Fischer 2015-06-06 11:05:50 UTC Comment hidden (obsolete)
Comment 6 Bernardo Gonzalez Kriegel 2015-06-18 15:39:40 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2015-06-18 16:20:04 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2015-06-18 16:20:40 UTC
Created attachment 40319 [details] [review]
Bug 11804: Remove unused circ-menu.tt

The formerly used circ-menu.tt is no longer referenced in the
templates and can be removed.

To test:
- Verify all tabs in the patron account still work as
  they should.
- git grep circ-menu.tt

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
No problems on patron pages, no more circ-menu.tt
No errors

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 9 Jonathan Druart 2015-06-18 16:20:44 UTC
Created attachment 40320 [details] [review]
Bug 11804: Remove references to circ-menu.tt

Signed-off-by: Jonathan Druart <jonathan.druart@koha-community.org>
Comment 10 Tomás Cohen Arazi 2015-06-22 20:09:11 UTC
Patches pushed to master.

Thanks Katrina and Jonathan!
Comment 11 Chris Cormack 2015-07-09 22:02:53 UTC
Pushed to 3.20.x will be in 3.20.2