Bug 21704 - Editing subfields in bibliographic frameworks is broken (TT filter)
Summary: Editing subfields in bibliographic frameworks is broken (TT filter)
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL:
Keywords: rel_18_11_candidate
Depends on: 13618
Blocks: 23153
  Show dependency treegraph
 
Reported: 2018-10-28 11:41 UTC by Tomás Cohen Arazi
Modified: 2019-06-19 05:00 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 21704: Edit framework subfield link is broken (TT filter) (8.01 KB, patch)
2018-10-28 12:44 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21704: (follow-up) Remove unused 'frameworkcode' template param (4.34 KB, patch)
2018-10-28 12:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21704: Edit framework subfield link is broken (TT filter) (8.12 KB, patch)
2018-10-28 20:33 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 21704: (follow-up) Remove unused 'frameworkcode' template param (4.45 KB, patch)
2018-10-28 20:34 UTC, Michal Denar
Details | Diff | Splinter Review
Bug 21704: Edit framework subfield link is broken (TT filter) (8.12 KB, patch)
2018-10-28 22:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21704: (follow-up) Remove unused 'frameworkcode' template param (4.50 KB, patch)
2018-10-28 22:09 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21704: (QA follow-up) Fix evil tabs (1.83 KB, patch)
2018-10-28 22:09 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2018-10-28 11:41:10 UTC
The subfield_link attribute is already escaped, so the uri filter breaks it. 

To test:
- Go to Home > Administration > MARC frameworks
- Pick any of them subfield (dropdown on the right, Subfields option)
=> FAIL: 'not found' error screen
Comment 1 Tomás Cohen Arazi 2018-10-28 11:44:14 UTC
I'll try to provide a fix tomorrow. Just a note for anyone beating me due to timezones: if we changed the template filter to (say) 'url' to fix this, we might be just hiding the fact that we shouldn't build the links in the .pl script. It should be done in the templates, using the 'uri' filter for the query parameters.
Comment 2 Tomás Cohen Arazi 2018-10-28 12:44:41 UTC
Created attachment 81405 [details] [review]
Bug 21704: Edit framework subfield link is broken (TT filter)

This patch removes URL building from the controller
(admin/marctagstructure.pl) and does it on the templates, using the
right 'uri' filter for the query parameters.

It also removes:
- No longer used attributes that are passed to the template
- Duplicated code in the templates

To test:
- Go to Home > Administration > MARC bibliographic framework
- On any of the listed frameworks, pick the 'MARC structure' action in
the dropdown
- On any subfield, hover on the actions dropdown
=> FAIL: The URLs are wrongly formed
- Click on the links
=> FAIL: You get errors because of the bad URL
- Apply this patch
- Reload the 'MARC structure' page
- On any subfield, hover on the actions dropdown
=> SUCCESS: LInks look correct
- Click on any of them
=> SUCCESS: They work!
- Use the 'Display only used tags/subfields' checkbox
=> SUCCESS: No behaviour change!
- Sign off :-D
Comment 3 Tomás Cohen Arazi 2018-10-28 12:59:37 UTC
Created attachment 81407 [details] [review]
Bug 21704: (follow-up) Remove unused 'frameworkcode' template param

This patch removes the 'frameworkcode' template parameter. Now the
'framework' object is passed, and everytime the frameworkcode is needed
on the template, framework.frameworkcode is called.

To test:
- Apply this patch
- Run:
  $ grep ' frameworkcode ' koha-tmpl/intranet-tmpl/prog/en/modules/admin/marctagstructure.tt
=> SUCCESS: There's no use of 'frameworkcode' in the template
- Sign off :-D
Comment 4 Michal Denar 2018-10-28 20:33:57 UTC
Created attachment 81428 [details] [review]
Bug 21704: Edit framework subfield link is broken (TT filter)

This patch removes URL building from the controller
(admin/marctagstructure.pl) and does it on the templates, using the
right 'uri' filter for the query parameters.

It also removes:
- No longer used attributes that are passed to the template
- Duplicated code in the templates

To test:
- Go to Home > Administration > MARC bibliographic framework
- On any of the listed frameworks, pick the 'MARC structure' action in
the dropdown
- On any subfield, hover on the actions dropdown
=> FAIL: The URLs are wrongly formed
- Click on the links
=> FAIL: You get errors because of the bad URL
- Apply this patch
- Reload the 'MARC structure' page
- On any subfield, hover on the actions dropdown
=> SUCCESS: LInks look correct
- Click on any of them
=> SUCCESS: They work!
- Use the 'Display only used tags/subfields' checkbox
=> SUCCESS: No behaviour change!
- Sign off :-D

Sponsored-by: Theke Solutions

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 5 Michal Denar 2018-10-28 20:34:01 UTC
Created attachment 81429 [details] [review]
Bug 21704: (follow-up) Remove unused 'frameworkcode' template param

This patch removes the 'frameworkcode' template parameter. Now the
'framework' object is passed, and everytime the frameworkcode is needed
on the template, framework.frameworkcode is called.

To test:
- Apply this patch
- Run:
  $ grep ' frameworkcode ' koha-tmpl/intranet-tmpl/prog/en/modules/admin/marctagstructure.tt
=> SUCCESS: There's no use of 'frameworkcode' in the template
- Sign off :-D

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 6 Katrin Fischer 2018-10-28 22:05:08 UTC
We need another patch for authority frameworks: bug 21706
Comment 7 Katrin Fischer 2018-10-28 22:09:32 UTC
Created attachment 81432 [details] [review]
Bug 21704: Edit framework subfield link is broken (TT filter)

This patch removes URL building from the controller
(admin/marctagstructure.pl) and does it on the templates, using the
right 'uri' filter for the query parameters.

It also removes:
- No longer used attributes that are passed to the template
- Duplicated code in the templates

To test:
- Go to Home > Administration > MARC bibliographic framework
- On any of the listed frameworks, pick the 'MARC structure' action in
the dropdown
- On any subfield, hover on the actions dropdown
=> FAIL: The URLs are wrongly formed
- Click on the links
=> FAIL: You get errors because of the bad URL
- Apply this patch
- Reload the 'MARC structure' page
- On any subfield, hover on the actions dropdown
=> SUCCESS: LInks look correct
- Click on any of them
=> SUCCESS: They work!
- Use the 'Display only used tags/subfields' checkbox
=> SUCCESS: No behaviour change!
- Sign off :-D

Sponsored-by: Theke Solutions

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 8 Katrin Fischer 2018-10-28 22:09:36 UTC
Created attachment 81433 [details] [review]
Bug 21704: (follow-up) Remove unused 'frameworkcode' template param

This patch removes the 'frameworkcode' template parameter. Now the
'framework' object is passed, and everytime the frameworkcode is needed
on the template, framework.frameworkcode is called.

To test:
- Apply this patch
- Run:
  $ grep ' frameworkcode ' koha-tmpl/intranet-tmpl/prog/en/modules/admin/marctagstructure.tt
=> SUCCESS: There's no use of 'frameworkcode' in the template
- Sign off :-D

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2018-10-28 22:09:40 UTC
Created attachment 81434 [details] [review]
Bug 21704: (QA follow-up) Fix evil tabs

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Nick Clemens 2018-10-29 01:17:13 UTC
Awesome work all!

Pushed to master for 18.11
Comment 11 Martin Renvoize 2018-11-08 15:26:10 UTC
Caused by bug 13618 not in 18.05.x series.