Bug 21270 - "Not finding what you're looking" display needs to be fixed
Summary: "Not finding what you're looking" display needs to be fixed
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-24 15:45 UTC by Blou
Modified: 2020-01-15 22:04 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.01, 19.05.07, 18.11.13


Attachments
Bug 21270: "Not finding what you're looking for" displaying uselessly (1.85 KB, patch)
2018-08-24 15:51 UTC, Blou
Details | Diff | Splinter Review
Bug 21270: "Not finding what you're looking for" displaying uselessly (2.03 KB, patch)
2018-08-29 01:30 UTC, Dilan Johnpullé
Details | Diff | Splinter Review
Bug 21270: "Not finding what you're looking for" displaying uselessly (2.03 KB, patch)
2019-11-25 19:43 UTC, Blou
Details | Diff | Splinter Review
Bug 21270: Display "Make an Interlibrary loan request" even if "suggestions" is turned off (1.50 KB, patch)
2019-12-05 12:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21270: "Not finding what you're looking for" displaying uselessly (2.10 KB, patch)
2019-12-05 12:29 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 21270: Display "Make an Interlibrary loan request" even if "suggestions" is turned off (1.56 KB, patch)
2019-12-05 12:29 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 21270: Display "Make an Interlibrary loan request" even if "suggestions" is turned off (2.79 KB, patch)
2019-12-05 12:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21270: "Not finding what you're looking for" displaying uselessly (2.19 KB, patch)
2019-12-06 09:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21270: Display "Make an Interlibrary loan request" even if "suggestions" is turned off (2.88 KB, patch)
2019-12-06 09:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Blou 2018-08-24 15:45:42 UTC
When doing OPAC search, at the bottom you have an offer presented with "Not finding what you're looking for" to introduce Suggestions or ILL if enabled.

Bug 7317 modified the if-else and now if you enable ILLModule but not AnonSuggestions and are not logged, you have this little string sitting there, offering nothing to the user.

Just taunting them...
Comment 1 Blou 2018-08-24 15:51:07 UTC
Created attachment 78141 [details] [review]
Bug 21270: "Not finding what you're looking for" displaying uselessly

When doing OPAC search, at the bottom you have an offer presented with "Not finding what you're looking for" to introduce Suggestions or ILL if enabled.

Bug 7317 modified the if-else and now if you enable ILLModule but not AnonSuggestions and are not logged, you have this little string sitting there, offering nothing to the user.

Just taunting them...

TEST:
0) Set IllModule to true, AnonSuggestions to false.
1) Without login, just to an OPAC search.  At the bottom will be a nice string laughing at your failed search
2) Apply the patch.
3) Problem at 1) is fixed.
4) Login to validate that ILL link now appear.
5) Modify combination of IllModule and AnonSuggestions to validate links appearing.
Comment 2 Dilan Johnpullé 2018-08-29 01:30:01 UTC
Created attachment 78239 [details] [review]
Bug 21270: "Not finding what you're looking for" displaying uselessly

When doing OPAC search, at the bottom you have an offer presented with "Not finding what you're looking for" to introduce Suggestions or ILL if enabled.

Bug 7317 modified the if-else and now if you enable ILLModule but not AnonSuggestions and are not logged, you have this little string sitting there, offering nothing to the user.

Just taunting them...

TEST:
0) Set IllModule to true, AnonSuggestions to false.
1) Without login, just to an OPAC search.  At the bottom will be a nice string laughing at your failed search
2) Apply the patch.
3) Problem at 1) is fixed.
4) Login to validate that ILL link now appear.
5) Modify combination of IllModule and AnonSuggestions to validate links appearing.

Patch applies and functions as described.
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Comment 3 Katrin Fischer 2018-08-29 06:13:10 UTC
I don't feel like the suggested fix is correct. If you don't allow suggestions, but ILL is turned on, now nothing displays. I think as the hints are arranged as list items, there is no reason not to show ILL in this constellation:

- not logged in
- ill module enabled
- suggestions and anon suggestions not active
Comment 4 Blou 2018-08-29 12:12:53 UTC
(In reply to Katrin Fischer from comment #3)
> I don't feel like the suggested fix is correct. If you don't allow
> suggestions, but ILL is turned on, now nothing displays. I think as the
> hints are arranged as list items, there is no reason not to show ILL in this
> constellation:
> 
> - not logged in
> - ill module enabled
> - suggestions and anon suggestions not active

That is a different behavior.  I was just fixing the original intent, which clearly put ILL **UNDER** suggestion.  Without suggestion, no ILL.

I'm not changing ILL's behavior, that is a different ticket.  A whole one :)
They initially coded it so that it would not appear unless 'suggestion' was ON.  My fix doesn't alter that.

NOTE: that would be very easy to do, but then open a larger discussion beforehand, not on "display" but on ILL...
Comment 5 Marcel de Rooy 2018-08-31 06:05:47 UTC
Moving to In Discussion. Katrin, could you reply on this one?
Comment 6 Katrin Fischer 2018-08-31 07:25:00 UTC
I'v asked some others on their opinions as well - will get back to this.
Comment 7 Nick Clemens 2019-10-17 13:23:00 UTC
(In reply to Katrin Fischer from comment #3)
> I don't feel like the suggested fix is correct. If you don't allow
> suggestions, but ILL is turned on, now nothing displays. I think as the
> hints are arranged as list items, there is no reason not to show ILL in this
> constellation:
> 
> - not logged in
> - ill module enabled
> - suggestions and anon suggestions not active

But we actually don't show ILL in that situation, the logic to show ILL is:
[% IF Koha.Preference( 'ILLModule' ) == 1 && loggedinusername %]

So if not logged it it never shows, so the check for logged in username means the removal of the ILL check in the larger loop has no effect. I think this one can move forward
Comment 8 Marcel de Rooy 2019-10-18 12:24:24 UTC
loggedinusername == 1  ?

Are you sure that this is correct? I am not ;)

$user = $patron->firstname . ' ' . $patron->surname;
$template->param( loggedinusername   => $user );
Comment 9 Blou 2019-11-25 19:43:12 UTC
Created attachment 95807 [details] [review]
Bug 21270: "Not finding what you're looking for" displaying uselessly

When doing OPAC search, at the bottom you have an offer presented with "Not finding what you're looking for" to introduce Suggestions or ILL if enabled.

Bug 7317 modified the if-else and now if you enable ILLModule but not AnonSuggestions and are not logged, you have this little string sitting there, offering nothing to the user.

Just taunting them...

TEST:
0) Set IllModule to true, AnonSuggestions to false.
1) Without login, just to an OPAC search.  At the bottom will be a nice string laughing at your failed search
2) Apply the patch.
3) Problem at 1) is fixed.
4) Login to validate that ILL link now appear.
5) Modify combination of IllModule and AnonSuggestions to validate links appearing.

Patch applies and functions as described.
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Comment 10 Jonathan Druart 2019-12-05 11:22:42 UTC
Sorry, trying to make this moving forward but it's confusing me.

I am also expecting to see "Make an Interlibrary loan request" even if "suggestions" is turned off.
Comment 11 Jonathan Druart 2019-12-05 11:24:35 UTC
Andrew, can you give your opinion on this one please?
Comment 12 Jonathan Druart 2019-12-05 12:06:12 UTC
(In reply to Jonathan Druart from comment #10)
> Sorry, trying to make this moving forward but it's confusing me.
> 
> I am also expecting to see "Make an Interlibrary loan request" even if
> "suggestions" is turned off.

If we all agree on that, I am going to provide a patch.
Comment 13 Andrew Isherwood 2019-12-05 12:07:25 UTC
Sounds reasonable to me
Comment 14 Nick Clemens 2019-12-05 12:11:44 UTC
(In reply to Jonathan Druart from comment #12)
> (In reply to Jonathan Druart from comment #10)
> > Sorry, trying to make this moving forward but it's confusing me.
> > 
> > I am also expecting to see "Make an Interlibrary loan request" even if
> > "suggestions" is turned off.
> 
> If we all agree on that, I am going to provide a patch.

I agree and Katrin seems to agree above, I think this works
Comment 15 Jonathan Druart 2019-12-05 12:20:58 UTC
Created attachment 96034 [details] [review]
Bug 21270: Display "Make an Interlibrary loan request" even if "suggestions" is turned off

Test plan:
- Turn off "suggestions"
- Enable ILLModule
When logged in you should see the "Make an Interlibrary loan request" on
the opac search result if not result is displayed
Comment 16 Andrew Isherwood 2019-12-05 12:29:46 UTC
Created attachment 96035 [details] [review]
Bug 21270: "Not finding what you're looking for" displaying uselessly

When doing OPAC search, at the bottom you have an offer presented with "Not finding what you're looking for" to introduce Suggestions or ILL if enabled.

Bug 7317 modified the if-else and now if you enable ILLModule but not AnonSuggestions and are not logged, you have this little string sitting there, offering nothing to the user.

Just taunting them...

TEST:
0) Set IllModule to true, AnonSuggestions to false.
1) Without login, just to an OPAC search.  At the bottom will be a nice string laughing at your failed search
2) Apply the patch.
3) Problem at 1) is fixed.
4) Login to validate that ILL link now appear.
5) Modify combination of IllModule and AnonSuggestions to validate links appearing.

Patch applies and functions as described.
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Comment 17 Andrew Isherwood 2019-12-05 12:29:49 UTC
Created attachment 96036 [details] [review]
Bug 21270: Display "Make an Interlibrary loan request" even if "suggestions" is turned off

Test plan:
- Turn off "suggestions"
- Enable ILLModule
When logged in you should see the "Make an Interlibrary loan request" on
the opac search result if not result is displayed

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Comment 18 Jonathan Druart 2019-12-05 12:30:57 UTC
Created attachment 96037 [details] [review]
Bug 21270: Display "Make an Interlibrary loan request" even if "suggestions" is turned off

Test plan:
- Turn off "suggestions"
- Turn on "ILLModule"
=> When logged in you should see the "Make an Interlibrary loan request"

- Turn on "suggestions"
- Turn off "ILLModule"
=> When logged in you should see the "Make a purchase suggestion"

- Turn on "suggestions"
- Turn on "ILLModule"
=> When logged in you should see both links.

- Turn off "suggestions"
- Turn off "ILLModule"
=> You should not see the "Not finding what you're looking for?" block

- Turn on "suggestions"
- Turn on AnonSuggestions
=> You should see the "Make a purchase suggestion" even when logged out
Comment 19 Andrew Isherwood 2019-12-05 12:32:44 UTC
I was a bit premature with my signoff, sorry! I'll obsolete my signoff patches
Comment 20 Andrew Isherwood 2019-12-05 13:26:31 UTC
I just retested with the full test plan and all is fine, so I have kept my sign off patches
Comment 21 Marcel de Rooy 2019-12-06 09:27:46 UTC
QAing Intrigued, cant we do this simpler?
Comment 22 Marcel de Rooy 2019-12-06 09:46:52 UTC
Created attachment 96059 [details] [review]
Bug 21270: "Not finding what you're looking for" displaying uselessly

When doing OPAC search, at the bottom you have an offer presented with "Not finding what you're looking for" to introduce Suggestions or ILL if enabled.

Bug 7317 modified the if-else and now if you enable ILLModule but not AnonSuggestions and are not logged, you have this little string sitting there, offering nothing to the user.

Just taunting them...

TEST:
0) Set IllModule to true, AnonSuggestions to false.
1) Without login, just to an OPAC search.  At the bottom will be a nice string laughing at your failed search
2) Apply the patch.
3) Problem at 1) is fixed.
4) Login to validate that ILL link now appear.
5) Modify combination of IllModule and AnonSuggestions to validate links appearing.

Patch applies and functions as described.
Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 23 Marcel de Rooy 2019-12-06 09:46:57 UTC
Created attachment 96060 [details] [review]
Bug 21270: Display "Make an Interlibrary loan request" even if "suggestions" is turned off

Test plan:
- Turn off "suggestions"
- Turn on "ILLModule"
=> When logged in you should see the "Make an Interlibrary loan request"

- Turn on "suggestions"
- Turn off "ILLModule"
=> When logged in you should see the "Make a purchase suggestion"

- Turn on "suggestions"
- Turn on "ILLModule"
=> When logged in you should see both links.

- Turn off "suggestions"
- Turn off "ILLModule"
=> You should not see the "Not finding what you're looking for?" block

- Turn on "suggestions"
- Turn on AnonSuggestions
=> You should see the "Make a purchase suggestion" even when logged out

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 24 Marcel de Rooy 2019-12-06 09:47:25 UTC
(In reply to Marcel de Rooy from comment #21)
> QAing Intrigued, cant we do this simpler?

Probably only marginal ;)
Comment 25 Martin Renvoize 2019-12-13 15:31:50 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 26 Joy Nelson 2019-12-31 18:00:46 UTC
Pushed to 19.11.x for 19.11.02

Thanks
joy
Comment 27 Lucas Gass 2020-01-06 22:59:30 UTC
backported to 19.05.x for 19.05.07
Comment 28 Hayley Mapley 2020-01-10 01:05:02 UTC
Backported to 18.11.x for 18.11.13