Bug 13307 - Create a link to the last advanced search in search result page (OPAC)
Summary: Create a link to the last advanced search in search result page (OPAC)
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: simith.doliveira
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 14351
  Show dependency treegraph
 
Reported: 2014-11-20 13:59 UTC by simith.doliveira
Modified: 2016-12-05 21:22 UTC (History)
8 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Create a link to the last advanced search in search result page (OPAC) (17.51 KB, patch)
2014-11-20 14:45 UTC, simith.doliveira
Details | Diff | Splinter Review
Create a link to the last advanced search in search result page (OPAC) (14.48 KB, patch)
2014-11-21 20:21 UTC, simith.doliveira
Details | Diff | Splinter Review
Search boxes disappear when returning to advanced search (104.87 KB, image/png)
2014-12-01 17:15 UTC, Christopher Brannon
Details
Fix Patch- Search boxes disappear when returning to advanced search (1.30 KB, patch)
2014-12-03 14:55 UTC, simith.doliveira
Details | Diff | Splinter Review
Fix jquery.deseriable.min.js (4.55 KB, patch)
2014-12-12 19:10 UTC, simith.doliveira
Details | Diff | Splinter Review
Fix jquery.deseriable.min.js (4.55 KB, patch)
2014-12-12 19:10 UTC, simith.doliveira
Details | Diff | Splinter Review
[Signed-off] Create a link to the last advanced search in search result page (OPAC) (14.71 KB, patch)
2014-12-13 09:03 UTC, Marc Véron
Details | Diff | Splinter Review
[Signed-off] Fix Patch- Search boxes disappear when returning to advanced search (1.45 KB, patch)
2014-12-13 09:05 UTC, Marc Véron
Details | Diff | Splinter Review
[Signed-off] Fix jquery.deseriable.min.js (4.69 KB, patch)
2014-12-13 09:06 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Create a link to the last advanced search in search result page (OPAC) (14.73 KB, patch)
2014-12-19 17:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Fix Patch- Search boxes disappear when returning to advanced search (1.45 KB, patch)
2014-12-19 17:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Fix jquery.deseriable.min.js (4.69 KB, patch)
2014-12-19 17:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 13307 [QA Followup] (1.12 KB, patch)
2014-12-19 17:13 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description simith.doliveira 2014-11-20 13:59:45 UTC

    
Comment 1 simith.doliveira 2014-11-20 14:45:41 UTC Comment hidden (obsolete)
Comment 2 simith.doliveira 2014-11-20 14:46:11 UTC
This patch add a link in the search result page to the last advanced search if the last search is a advanced search.
If the last search is a simple search, no link is added.

Sponsored-by: CCSR ( http://www.ccsr.qc.ca )

Testing:

I
- Open opac main page and execute a simple search
validate no link

II
- Open opac advanced search page, chose some options and execute a search.
validate a link
- Click on the link 
validate the advanced search page and the options. 

III
- Open opac advanced search page, chose some options and execute a search.
validate a link
- If it got some pages, click on next or a page, then click on the link 
validate the advanced search page and the options. 

IV
- Open opac advanced search page, click the button "more options", chose some options (add a field to search - bouton [+]) and execute a search.
validate a link
- Click on the link 
validate the advanced search page and the options. 

V
- Open opac advanced search page, chose some options and execute a search.
validate a link
- Execute a simple search 
validate no link.
Comment 3 Owen Leonard 2014-11-20 17:22:31 UTC
This is a nice feature and it seems to be working well. I have some suggestions for ways to make the patch conform better to some existing practices:

- <script> blocks should not be added to masthead.inc. In the bootstrap theme JavaScript is placed at the end of the document: opac-bottom.inc

- jQuery plugins which are included by OPAC pages should be minified. I think it's helpful to add both the minified and non-minified versions even if we only link to the minified version. But that's not an agreed-upon rule.

- The "Return to the last advanced search" link should be added to the DOM (or made visible) using JavaScript since the feature depends on JavaScript. Users without JS should not see the link.

I'm curious why you use the jQuery.cookie plugin in some code but not in the changes you made to masthead.inc?

Finally: This patch makes changes to .htaccess and it should not.
Comment 4 simith.doliveira 2014-11-21 20:21:49 UTC Comment hidden (obsolete)
Comment 5 simith.doliveira 2014-11-21 20:30:24 UTC
Concerning some suggestions/error :


- <script> blocks should not be added to masthead.inc. In the bootstrap theme JavaScript is placed at the end of the document: opac-bottom.inc

Done

- jQuery plugins which are included by OPAC pages should be minified. I think it's helpful to add both the minified and non-minified versions even if we only link to the minified version. But that's not an agreed-upon rule.

Done. But I included just the minifieds.

- The "Return to the last advanced search" link should be added to the DOM (or made visible) using JavaScript since the feature depends on JavaScript. Users without JS should not see the link.

The link is controlled by a cookie, this cookie est created by js, so, without js, no link.

I'm curious why you use the jQuery.cookie plugin in some code but not in the changes you made to masthead.inc?

Both use jQuery.cookie now.

Finally: This patch makes changes to .htaccess and it should not.

Corrected.
Comment 6 Christopher Brannon 2014-12-01 17:14:35 UTC
After clicking "Return to the last advanced search" link, the top portion of the advanced search page, with all search boxes, disappears.  Can't edit the original search terms!  I will include a screen shot.

Christopher
Comment 7 Christopher Brannon 2014-12-01 17:15:43 UTC Comment hidden (obsolete)
Comment 8 simith.doliveira 2014-12-03 14:55:19 UTC Comment hidden (obsolete)
Comment 9 Marc Véron 2014-12-05 09:21:15 UTC
I like this feature.

However I found the following:

1)
Search e.g. for Author = Goethe and Title = Faust
2)
The link back to the last advanced search appears as expected.
3)
After clicking the link, I get the advanced search page with
Keyword = Goethe and Keyword = Faust (instead of Author and Title)

Marc

PS: Applying patch did a 3-way-merge
Comment 10 simith.doliveira 2014-12-12 19:10:40 UTC Comment hidden (obsolete)
Comment 11 simith.doliveira 2014-12-12 19:10:54 UTC Comment hidden (obsolete)
Comment 12 simith.doliveira 2014-12-12 19:19:56 UTC
Error comment 9 fixed
Comment 13 Marc Véron 2014-12-13 09:03:02 UTC Comment hidden (obsolete)
Comment 14 Marc Véron 2014-12-13 09:05:13 UTC Comment hidden (obsolete)
Comment 15 Marc Véron 2014-12-13 09:06:50 UTC Comment hidden (obsolete)
Comment 16 Kyle M Hall 2014-12-19 17:13:27 UTC
Created attachment 34590 [details] [review]
[PASSED QA] Create a link to the last advanced search in search result page (OPAC)

http://bugs.koha-community.org/show_bug.cgi?id=13307

Followed test plan, works as expected (together with other patches)
Signed-off-by: Marc Veron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

http://bugs.koha-community.org/show_bug.cgi?id=13364
Comment 17 Kyle M Hall 2014-12-19 17:13:36 UTC
Created attachment 34591 [details] [review]
[PASSED QA] Fix Patch- Search boxes disappear when returning to advanced search

http://bugs.koha-community.org/show_bug.cgi?id=13307
Signed-off-by: Marc Veron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

http://bugs.koha-community.org/show_bug.cgi?id=13364
Comment 18 Kyle M Hall 2014-12-19 17:13:39 UTC
Created attachment 34592 [details] [review]
[PASSED QA] Fix jquery.deseriable.min.js

http://bugs.koha-community.org/show_bug.cgi?id=13307
Signed-off-by: Marc Veron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

http://bugs.koha-community.org/show_bug.cgi?id=13364
Comment 19 Kyle M Hall 2014-12-19 17:13:42 UTC
Created attachment 34593 [details] [review]
[PASSED QA] Bug 13307 [QA Followup]

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

http://bugs.koha-community.org/show_bug.cgi?id=13364
Comment 20 Tomás Cohen Arazi 2015-04-13 16:51:28 UTC
Enhancement pushed to master.

Thanks Simith!

Note: I ammended the commit messages. Please try to include bug number
and better descriptions of what the patches are for.
Comment 21 Katrin Fischer 2015-06-06 15:30:03 UTC
Hm, it looks like a given-when structure slipped in with this. The feature is experimental and should not be used, I filed bug 14351.