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: RESOLVED DUPLICATE of bug 28782
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
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: 2022-01-28 13:50 UTC (History)
5 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 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.
Comment 8 Jonathan Druart 2021-12-02 10:18:13 UTC
(In reply to Joonas Kylmälä from comment #7)
> Just noting down here as well, if bug 28782 gets pushed then this bug is
> fixed as well.

Yes, I think we should favor bug 28782's solution over this one. Or we based it on top of this one to make this patch backportable.
Comment 9 Jonathan Druart 2022-01-28 13:50:46 UTC
Will be fixed globally on bug 28782.

*** This bug has been marked as a duplicate of bug 28782 ***