Bug 11127 - OPAC suggestion: the delete link is active when no suggestion selected
Summary: OPAC suggestion: the delete link is active when no suggestion selected
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-24 09:40 UTC by Jonathan Druart
Modified: 2015-06-04 23:30 UTC (History)
5 users (show)

See Also:
GIT URL:
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 11127: OPAC suggestion: the delete link is active when no suggestion selected (2.02 KB, patch)
2013-10-24 09:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11127: OPAC suggestion: the delete link is active when no suggestion selected (1.72 KB, patch)
2014-03-04 15:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
[Signed-off] Bug 11127: OPAC suggestion: the delete link is active when no suggestion selected (1.91 KB, patch)
2014-03-04 16:53 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 11127: Define a specific message for suggestion (1.58 KB, patch)
2014-03-11 11:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11127: Define a specific message for suggestion (1.78 KB, patch)
2014-03-11 15:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11127: Define a specific message for suggestion (1.82 KB, patch)
2014-03-12 12:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11127: Define a specific message for suggestion (1.90 KB, patch)
2014-03-13 15:17 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
[PASSED QA] Bug 11127: OPAC suggestion: the delete link is active when no suggestion selected (2.01 KB, patch)
2014-03-13 15:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11127: Define a specific message for suggestion (1.95 KB, patch)
2014-03-13 15:20 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 Jonathan Druart 2013-10-24 09:40:31 UTC

    
Comment 1 Jonathan Druart 2013-10-24 09:44:28 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2013-10-24 09:46:06 UTC
Should I produce the same fix for others themes (bootstrap and ccsr)?
Comment 3 Owen Leonard 2013-10-24 13:59:49 UTC
Instead of adding the "disabled" attribute to an element which isn't really supposed to have it (according to the specification) I think it would be better to check for the existence of a checked checkbox:

  var checkedBoxes = $("input:checked");
  if ($(checkedBoxes).size() == 0) {
     alert(MSG_NO_RECORD_SELECTED);
     return false;
  }
Comment 4 Marcel de Rooy 2013-10-31 11:23:03 UTC
Jonathan:
If there are still suggestions, there is also the Delete selected button below.
If you click on that button without a selection, it still says: Selected sugg have been deleted.

For consistency, I would recommend adjusting that too.
Comment 5 Jonathan Druart 2013-10-31 13:27:51 UTC
Owen,
Why are there 2 ways to delete selected suggestions? Should not we keep only the button on the bottom?
Comment 6 Jonathan Druart 2013-11-13 14:42:01 UTC
ping Owen
Comment 7 Owen Leonard 2013-11-14 14:43:04 UTC
(In reply to Jonathan Druart from comment #6)

> ping Owen

Sorry, here I am!

(In reply to Jonathan Druart from comment #5)

> Why are there 2 ways to delete selected suggestions? Should not we keep only
> the button on the bottom?

I don't see any reason not to offer both options. We do the same on the lists page and the on the user account page with renewal options.
Comment 8 Jonathan Druart 2014-03-04 15:31:55 UTC Comment hidden (obsolete)
Comment 9 Marc Véron 2014-03-04 16:53:18 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2014-03-11 10:14:05 UTC
Hi Jonathan,

I like the idea and it works nicely :) The only thing I am a bit concerned about is reusing the message from the cart. I think right now they are always in the template even if the cart is turned off, but that might change at some point. Also the message might read a bit nicer as "No suggestion has been selected" - as item is also a bit confusing for translations.
Comment 11 Jonathan Druart 2014-03-11 11:07:43 UTC Comment hidden (obsolete)
Comment 12 Katrin Fischer 2014-03-11 11:44:49 UTC
Hi Jonathan, the alert doesn't come up for me now and I can't see the new message in the template. Maybe we could just add the string directly into the alert? There seems to be an example for this a bit below. I think the MSG.* mostly exist because the Javascript for those is in a separate .js file.
Comment 13 Owen Leonard 2014-03-11 13:00:10 UTC
The alert isn't coming up now because the MSG is defined in the prog include, but the template is only modified for the bootstrap theme.

I would probably just put the MSG definition in the template itself since it's only relevant to that one page.
Comment 14 Jonathan Druart 2014-03-11 15:35:06 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2014-03-11 15:35:39 UTC
(In reply to Owen Leonard from comment #13)
> The alert isn't coming up now because the MSG is defined in the prog
> include, but the template is only modified for the bootstrap theme.

Arg stupid mistake, sorry Katrin!
Comment 16 Katrin Fischer 2014-03-12 09:29:11 UTC
Hi Jonathan,

I am sorry, but after takig a look at the code, I think we need to move the line with the message out of the IF statement, or it will depend on cart and lists being turned on:
    [% IF Koha.Preference( 'opacbookbag' ) == 1 or Koha.Preference( 'virtualshelves' ) == 1 %]
Comment 17 Jonathan Druart 2014-03-12 12:10:18 UTC Comment hidden (obsolete)
Comment 18 Martin Renvoize (ashimema) 2014-03-13 15:17:48 UTC Comment hidden (obsolete)
Comment 19 Katrin Fischer 2014-03-13 15:20:41 UTC
Created attachment 26308 [details] [review]
[PASSED QA] Bug 11127: OPAC suggestion: the delete link is active when no suggestion selected

To reproduce:
1/ go on opac-suggestions.pl
2/ create a suggestion
3/ click on the delete link without checking any checkbox
The form is submitted with a message "The selected suggestions have been
deleted.".

Test plan:
0/ enable the bootstrap theme
1/ after applying the patch, click on delete without any checkbox
checked, a js alert should occur.
2/ check at least a cb and click on the delete link. The deletion should
work.

Followed test plan. Patch behaves as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Passes all tests and QA script, works as described.
Comment 20 Katrin Fischer 2014-03-13 15:20:51 UTC
Created attachment 26309 [details] [review]
[PASSED QA] Bug 11127: Define a specific message for suggestion

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 21 Galen Charlton 2014-03-13 16:51:11 UTC
Pushed to master.  Thanks, Jonathan!
Comment 22 Fridolin Somers 2014-06-12 08:10:28 UTC
Pushed to 3.14.x, will be in 3.14.08
Comment 23 Fridolin Somers 2014-06-12 08:13:40 UTC
Oups, the title of this bug was the one of Bug 11934 : "Replace given by if-else statements"
Comment 24 Fridolin Somers 2014-06-12 08:18:21 UTC
Sorry, I've add a odd behavior with my Lazarus Plugin.
I restore this bug from history.