|
Description
Fridolin Somers
2013-07-15 10:54:31 UTC
Created attachment 19690 [details] [review] Bug 10588 - opac-topissues first branch always selected in filters See test plan in commit message Code inspired by Bug 9928 which introduced the code "__ANY__". Does this depend on another patch? It doesn't apply on master. Created attachment 19714 [details] [review] Bug 10588 - opac-topissues first branch always selected in filters I rebased on master. There was a conflict with Bug 10590 patch. I should apply on a today fresh master. Created attachment 19744 [details] [review] 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 code "__ANY__" 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: Liz Rea <liz@catalyst.net.nz> Tests work as described - much improved! Created attachment 19837 [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 code "__ANY__" 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: Liz Rea <liz@catalyst.net.nz> Tests work as described - much improved! Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> I am not sure about the use of "__ANY__" but the behaviour of the page is improved by the patch. (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. Created attachment 20055 [details] [review] Bug 10588 - opac-topissues first branch always selected in filters (revised) Revised patch, same test plan (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. Created attachment 20103 [details] [review] [SIGNED-OFF] 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> Added link to Bug 9223 because the code of opac-top issues should use C4::Koha::GetSupportList. 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. I like this version a lot better, thx Fridolyn. Pushed to master. Thanks, Fridolyn! This patch has been pushed to 3.12.x, will be in 3.12.5. Thanks Fridolyn! Pushed to 3.10.x, will be in 3.10.12 |