Bug 21706 - Editing subfields in authority frameworks is broken (TT filter)
Summary: Editing subfields in authority frameworks is broken (TT filter)
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on: 13618 21526
Blocks:
  Show dependency treegraph
 
Reported: 2018-10-28 22:04 UTC by Katrin Fischer
Modified: 2019-10-14 19:56 UTC (History)
3 users (show)

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


Attachments
Bug 21706: Make authority framework subfield links work (TT filter) (2.65 KB, patch)
2018-10-29 13:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21706: (follow-up) remove unused template params (2.50 KB, patch)
2018-10-29 13:55 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21706: (follow-up) Fix subfield delete button (2.61 KB, patch)
2018-10-29 14:26 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21706: (QA follow-up) Fix tabs (2.67 KB, patch)
2018-10-31 14:30 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21706: Make authority framework subfield links work (TT filter) (2.73 KB, patch)
2018-10-31 17:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21706: (follow-up) remove unused template params (2.58 KB, patch)
2018-10-31 17:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21706: (follow-up) Fix subfield delete button (2.69 KB, patch)
2018-10-31 17:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21706: (QA follow-up) Fix tabs (2.75 KB, patch)
2018-10-31 17:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21706: Make authority framework subfield links work (TT filter) (2.78 KB, patch)
2018-11-06 20:12 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21706: (follow-up) remove unused template params (2.62 KB, patch)
2018-11-06 20:12 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21706: (follow-up) Fix subfield delete button (2.74 KB, patch)
2018-11-06 20:12 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21706: (QA follow-up) Fix tabs (2.80 KB, patch)
2018-11-06 20:12 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21706: Make authority framework subfield links work (TT filter) (2.83 KB, patch)
2018-11-12 19:46 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21706: (follow-up) remove unused template params (2.62 KB, patch)
2018-11-12 19:46 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21706: (follow-up) Fix subfield delete button (2.73 KB, patch)
2018-11-12 19:46 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21706: (QA follow-up) Fix tabs (2.80 KB, patch)
2018-11-12 19:46 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2018-10-28 22:04:37 UTC
Possibly the same problem as on bug 21704.
Comment 1 Tomás Cohen Arazi 2018-10-29 13:54:58 UTC
Created attachment 81500 [details] [review]
Bug 21706: Make authority framework subfield links work (TT filter)

This patch removes URL building from the controller
and does it on the templates, using the
right 'uri' filter for the query parameters.

To test:
- Go to Home > Administration > Athority types
- 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!
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi 2018-10-29 13:55:10 UTC
Created attachment 81501 [details] [review]
Bug 21706: (follow-up) remove unused template params

The original patch removed the need to add URLs to the template
variables. This patch removes that in the controller.

Bonus: fixes 'html' filter in URL replacing it by 'uri'.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 3 Tomás Cohen Arazi 2018-10-29 14:26:17 UTC
Created attachment 81506 [details] [review]
Bug 21706: (follow-up) Fix subfield delete button

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Katrin Fischer 2018-10-29 20:36:37 UTC
I can't add a new subfield with these patches. When clicking on "New" nothing happens. 

I notice there is also a little error with the tabs on top, there is a gab between them and the form below.
Comment 5 Jonathan Druart 2018-10-29 22:57:32 UTC
See bug 21526 comment 19. There are plenty to fix.
Comment 6 Tomás Cohen Arazi 2018-10-30 16:04:51 UTC
(In reply to Katrin Fischer from comment #4)
> I can't add a new subfield with these patches. When clicking on "New"
> nothing happens. 
> 
> I notice there is also a little error with the tabs on top, there is a gab
> between them and the form below.

It works for me, Katrina.
Comment 7 Tomás Cohen Arazi 2018-10-30 16:06:08 UTC
(In reply to Jonathan Druart from comment #5)
> See bug 21526 comment 19. There are plenty to fix.

I got rid of the URL build step in the controller, it is now clear text, and the param's are URI-filtered as they should.
Comment 8 Katrin Fischer 2018-10-31 06:29:55 UTC
Hi Tomas, I retested, try this:

000 authority TOPIC_TERM 

It works for fields with more than one subfield or outside of 000 - some of that.
Comment 9 Tomás Cohen Arazi 2018-10-31 14:30:42 UTC
Created attachment 81729 [details] [review]
Bug 21706: (QA follow-up) Fix tabs

Not all href's should have parameters URI encoded. Because sometimes
they are just where for use by JavaScript libraries which fail in that
situations. For fixed-lenght fields we use @ as the subfield code and it
breaks the bootstrap tabs if we URI encode the '@'.

This fixes the tabs issues on fixed-lenght fields in the authority type
editing pages.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 10 Tomás Cohen Arazi 2018-10-31 14:40:27 UTC
(In reply to Katrin Fischer from comment #8)
> Hi Tomas, I retested, try this:
> 
> 000 authority TOPIC_TERM 
> 
> It works for fields with more than one subfield or outside of 000 - some of
> that.

Ok, I reproduced it in clean master. This was due to the TT filters too. It doesn't show on the bibliographic framworks edit only because they use a (bad) trick to use '_' instead of '@' when editing the subfield. I'm filing a separate bug for that.
Comment 11 Jonathan Druart 2018-10-31 17:56:30 UTC
Created attachment 81765 [details] [review]
Bug 21706: Make authority framework subfield links work (TT filter)

This patch removes URL building from the controller
and does it on the templates, using the
right 'uri' filter for the query parameters.

To test:
- Go to Home > Administration > Athority types
- 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!
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Jonathan Druart 2018-10-31 17:56:34 UTC
Created attachment 81766 [details] [review]
Bug 21706: (follow-up) remove unused template params

The original patch removed the need to add URLs to the template
variables. This patch removes that in the controller.

Bonus: fixes 'html' filter in URL replacing it by 'uri'.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2018-10-31 17:56:39 UTC
Created attachment 81767 [details] [review]
Bug 21706: (follow-up) Fix subfield delete button

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Jonathan Druart 2018-10-31 17:56:44 UTC
Created attachment 81768 [details] [review]
Bug 21706: (QA follow-up) Fix tabs

Not all href's should have parameters URI encoded. Because sometimes
they are just where for use by JavaScript libraries which fail in that
situations. For fixed-lenght fields we use @ as the subfield code and it
breaks the bootstrap tabs if we URI encode the '@'.

This fixes the tabs issues on fixed-lenght fields in the authority type
editing pages.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Jonathan Druart 2018-10-31 17:58:26 UTC
Note that html_entities is not needed at first glance, but already used in a similar place (marc frameworks). Also the href attribute are not prefixed by /cgi-bin/koha as we usually do but not important.
Comment 16 Jonathan Druart 2018-10-31 17:58:53 UTC
(In reply to Jonathan Druart from comment #15)
> Note that html_entities is not needed

=> Could be "html", that's what I meant.
Comment 17 Josef Moravec 2018-11-06 20:12:18 UTC Comment hidden (obsolete)
Comment 18 Josef Moravec 2018-11-06 20:12:23 UTC Comment hidden (obsolete)
Comment 19 Josef Moravec 2018-11-06 20:12:29 UTC Comment hidden (obsolete)
Comment 20 Josef Moravec 2018-11-06 20:12:35 UTC Comment hidden (obsolete)
Comment 21 Tomás Cohen Arazi 2018-11-12 19:46:03 UTC
Created attachment 82245 [details] [review]
Bug 21706: Make authority framework subfield links work (TT filter)

This patch removes URL building from the controller
and does it on the templates, using the
right 'uri' filter for the query parameters.

To test:
- Go to Home > Administration > Athority types
- 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!
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 22 Tomás Cohen Arazi 2018-11-12 19:46:08 UTC
Created attachment 82246 [details] [review]
Bug 21706: (follow-up) remove unused template params

The original patch removed the need to add URLs to the template
variables. This patch removes that in the controller.

Bonus: fixes 'html' filter in URL replacing it by 'uri'.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 23 Tomás Cohen Arazi 2018-11-12 19:46:16 UTC
Created attachment 82247 [details] [review]
Bug 21706: (follow-up) Fix subfield delete button

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 24 Tomás Cohen Arazi 2018-11-12 19:46:23 UTC
Created attachment 82248 [details] [review]
Bug 21706: (QA follow-up) Fix tabs

Not all href's should have parameters URI encoded. Because sometimes
they are just where for use by JavaScript libraries which fail in that
situations. For fixed-lenght fields we use @ as the subfield code and it
breaks the bootstrap tabs if we URI encode the '@'.

This fixes the tabs issues on fixed-lenght fields in the authority type
editing pages.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 25 Tomás Cohen Arazi 2018-11-12 19:47:10 UTC
Debased against 21526 to ease integration.
Comment 26 Tomás Cohen Arazi 2018-11-12 19:48:31 UTC
Pushed for 18.11. Thanks all!
Comment 27 Martin Renvoize 2018-11-16 14:22:55 UTC
Dependencies not in 18.05.x series.