Bug 29257 - Redundant slash in the URL appear during item level hold placing
Summary: Redundant slash in the URL appear during item level hold placing
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Andrii Veremeienko
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 29259
  Show dependency treegraph
 
Reported: 2021-10-17 11:29 UTC by Andrii Veremeienko
Modified: 2021-10-30 22:25 UTC (History)
4 users (show)

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


Attachments
Bug 29257: Redundant slash in the URL appear during item level hold placing (1.69 KB, patch)
2021-10-17 11:47 UTC, Andrii Veremeienko
Details | Diff | Splinter Review
Bug 29257: Redundant slash in the URL appear during item level hold placing (1.69 KB, patch)
2021-10-17 13:12 UTC, Andrii Veremeienko
Details | Diff | Splinter Review
Bug 29257: Redundant slash in the URL appear during item level hold placing (1.82 KB, patch)
2021-10-17 14:49 UTC, Andrii Veremeienko
Details | Diff | Splinter Review
Bug 29257: Redundant slash in the URL appear during item level hold placing (1.87 KB, patch)
2021-10-17 17:42 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrii Veremeienko 2021-10-17 11:29:39 UTC
After placing hold user is redirected to a link containing a redundant slash in the end, which leads to unexpected consequences.

Steps to reproduce:

1. Search for an item;
2. Select single item, click 'Place hold';
3. Enter a patron card number;
4. Complete hold placing by clicking 'Place hold';
5. Note that after redirection to URL ended with slash (after biblionumber);

E.g.: request.pl?biblionumbers=233/
Comment 1 Andrii Veremeienko 2021-10-17 11:47:40 UTC Comment hidden (obsolete)
Comment 2 Andrii Veremeienko 2021-10-17 13:12:59 UTC Comment hidden (obsolete)
Comment 3 Andrii Veremeienko 2021-10-17 14:49:52 UTC
Created attachment 126411 [details] [review]
Bug 29257: Redundant slash in the URL appear during item level hold placing

The solution was to get rid of adding slash to $biblionumbers directly and
join slash only in place when it's needed.

How to test:
1. Search for an item;
2. Select single item, click 'Place hold';
3. Enter a patron card number;
4. Complete hold placing by clicking 'Place hold';
5. Note that after redirection to URL ended with slash (after biblionumber):
e.g.: `request.pl?biblionumbers=233/`
6. Apply the patch
7. Repeat steps 1-4
8 Note that after redirection to the page with hold list, URL doesn't contain slash after biblionumber parameter:
e.g.: `request.pl?biblionumbers=233
Comment 4 David Nind 2021-10-17 17:42:29 UTC
Created attachment 126412 [details] [review]
Bug 29257: Redundant slash in the URL appear during item level hold placing

The solution was to get rid of adding slash to $biblionumbers directly and
join slash only in place when it's needed.

How to test:
1. Search for an item;
2. Select single item, click 'Place hold';
3. Enter a patron card number;
4. Complete hold placing by clicking 'Place hold';
5. Note that after redirection to URL ended with slash (after biblionumber):
e.g.: `request.pl?biblionumbers=233/`
6. Apply the patch
7. Repeat steps 1-4
8 Note that after redirection to the page with hold list, URL doesn't contain slash after biblionumber parameter:
e.g.: `request.pl?biblionumbers=233

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Cook 2021-10-18 00:36:59 UTC
I haven't looked into this in-depth, but I'm wary of this one, especially in the context of item-specific multi-holds. 

I'm not going to look into it right now, but just sharing my thoughts. It would be worth expanding this test plan to different hold scenarios.
Comment 6 David Cook 2021-10-21 22:51:37 UTC
(In reply to David Cook from comment #5)
> I haven't looked into this in-depth, but I'm wary of this one, especially in
> the context of item-specific multi-holds. 
> 
> I'm not going to look into it right now, but just sharing my thoughts. It
> would be worth expanding this test plan to different hold scenarios.

I've taken a bit more of a look in light of bug 28782, and it looks the slashes are only important for the OPAC and not for the Staff Interface, so my objection withdrawn.
Comment 7 Joonas Kylmälä 2021-10-30 22:25:40 UTC
Just noting down here as well, if bug 28782 gets pushed then this bug is fixed as well.