Bug 10588 - opac-topissues first branch always selected in filters
Summary: opac-topissues first branch always selected in filters
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-15 10:54 UTC by Fridolin Somers
Modified: 2014-12-07 20:02 UTC (History)
3 users (show)

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


Attachments
Bug 10588 - opac-topissues first branch always selected in filters (4.87 KB, patch)
2013-07-16 09:53 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 10588 - opac-topissues first branch always selected in filters (4.92 KB, patch)
2013-07-17 12:28 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 10588 - opac-topissues first branch always selected in filters (4.98 KB, patch)
2013-07-18 03:27 UTC, Liz Rea
Details | Diff | Splinter Review
[PASSED QA] Bug 10588 - opac-topissues first branch always selected in filters (5.15 KB, patch)
2013-07-21 11:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 10588 - opac-topissues first branch always selected in filters (revised) (3.94 KB, patch)
2013-08-02 09:42 UTC, Fridolin Somers
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 10588 - opac-topissues first branch always selected in filters (3.98 KB, patch)
2013-08-05 06:03 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
[PASSED QA] Bug 10588 - opac-topissues first branch always selected in filters (4.08 KB, patch)
2013-08-25 19:04 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 Fridolin Somers 2013-07-15 10:54:31 UTC
In OPAC most popular items page:
With user is logged-in his branch should be selected.
When a branch is manually selected and form is submitted, this branch should be selected in results page.
When looking at HTML, you see that all options are selected in combobox o_O.
Comment 1 Fridolin Somers 2013-07-16 09:53:55 UTC Comment hidden (obsolete)
Comment 2 Fridolin Somers 2013-07-16 09:55:40 UTC
Code inspired by Bug 9928 which introduced the code "__ANY__".
Comment 3 Owen Leonard 2013-07-16 15:25:05 UTC
Does this depend on another patch? It doesn't apply on master.
Comment 4 Fridolin Somers 2013-07-17 12:28:07 UTC Comment hidden (obsolete)
Comment 5 Liz Rea 2013-07-18 03:27:03 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2013-07-21 11:46:47 UTC Comment hidden (obsolete)
Comment 7 Galen Charlton 2013-07-22 19:12:41 UTC
(In reply to Katrin Fischer from comment #6)
> I am not sure about the use of "__ANY__" but the behaviour of the
> page is improved by the patch.

I'm not sure about the "__ANY__" either.  Why not use the empty string as the value for the select, like the library selector on the masthead or the library selector on advanced search?

I know that C4/Suggestions.pm uses __ANY__, but that's the only place in the code that does, and I think that should be taken out as well.

Setting to in discussion.
Comment 8 Fridolin Somers 2013-08-02 09:42:54 UTC Comment hidden (obsolete)
Comment 9 Fridolin Somers 2013-08-02 09:50:18 UTC
(In reply to Galen Charlton from comment #7)
Indeed, I changed my mind about '__ANY__'.
Because the purpose of this patch is to distinguish form display and form post with a filter on all branches.
There is already a way to do this : use the hidden input named 'do_it' which is undef when form is displayed and '1' when form is posted.
This is used in a lot of pages and seems to me more clear code than using '__ANY__'.

I think '__ANY__' is only used in suggestions because there are suggestions with no branch and so there may be a filter with 3 cases : a branch selected, any branch and no branch. 

Setting to needs signoff.
Comment 10 Srdjan Jankovic 2013-08-05 06:03:35 UTC Comment hidden (obsolete)
Comment 11 Fridolin Somers 2013-08-07 15:17:10 UTC
Added link to Bug 9223 because the code of opac-top issues should use C4::Koha::GetSupportList.
Comment 12 Katrin Fischer 2013-08-25 19:04:54 UTC
Created attachment 20602 [details] [review]
[PASSED QA] Bug 10588 - opac-topissues first branch always selected in filters

In OPAC most popular items page:
With user is logged-in his branch should be selected.
When a branch is manually selected and form is submitted, this branch should be selected in results page.
When looking at HTML, you see that all options are selected in combobox.

This patch corrects by using the hidden input 'do_it' to distinguish form display and no branch filter selected.

Test plan :
- Make sure there are issues on records created in the past 3 month
- Go to OPAC not logged-in
- Click on "Most popular"
=> You get to page with results, "All libraries" is selected
- Select a branch and submit
=> You get to page with results, the branch is selected [testing opac-topissues.inc from]
- Select a branch and perform a search that does not have results (for example an item type never issued)
=> You get to page without results, the branch is selected [testing opac-topissues.tt form]
- Log-in with a user
- Click on "Most popular"
=> You get to page with results, user's branch is selected
- Select "All libraries" and submit
=> You get to page with results, "All libraries" is selected [testing opac-topissues.inc from]
- Select "All libraries" and perform a search that does not have results (for example an item type never issued)
=> You get to page without results, "All libraries" is selected [testing opac-topissues.tt form]

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works nicely and passes all tests.
Comment 13 Katrin Fischer 2013-08-25 19:06:25 UTC
I like this version a lot better, thx Fridolyn.
Comment 14 Galen Charlton 2013-09-04 17:15:35 UTC
Pushed to master.  Thanks, Fridolyn!
Comment 15 Tomás Cohen Arazi (tcohen) 2013-09-13 18:48:25 UTC
This patch has been pushed to 3.12.x, will be in 3.12.5.

Thanks Fridolyn!
Comment 16 Bernardo Gonzalez Kriegel 2013-10-13 13:27:16 UTC
Pushed to 3.10.x, will be in 3.10.12