|
Description
Blou
2018-08-24 15:45:42 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. 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> 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 (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... Moving to In Discussion. Katrin, could you reply on this one? I'v asked some others on their opinions as well - will get back to this. (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 loggedinusername == 1 ? Are you sure that this is correct? I am not ;) $user = $patron->firstname . ' ' . $patron->surname; $template->param( loggedinusername => $user ); 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> 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. Andrew, can you give your opinion on this one please? (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. Sounds reasonable to me (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 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 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> 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> 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 I was a bit premature with my signoff, sorry! I'll obsolete my signoff patches I just retested with the full test plan and all is fine, so I have kept my sign off patches QAing Intrigued, cant we do this simpler? 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> 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> (In reply to Marcel de Rooy from comment #21) > QAing Intrigued, cant we do this simpler? Probably only marginal ;) Nice work everyone! Pushed to master for 20.05 Pushed to 19.11.x for 19.11.02 Thanks joy backported to 19.05.x for 19.05.07 Backported to 18.11.x for 18.11.13 |