Bug 14805 - Allow the deletion of multiple patron card batches
Summary: Allow the deletion of multiple patron card batches
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Label/patron card printing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Joonas Kylmälä
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-11 08:24 UTC by Joonas Kylmälä
Modified: 2016-06-21 21:40 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 14805: Allow the user to delete multiple batches (7.70 KB, patch)
2015-09-14 13:34 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
[Signed-off] Bug 14805: Allow the user to delete multiple batches (7.88 KB, patch)
2015-09-16 12:51 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14805: QA follow-up (3.14 KB, patch)
2015-09-17 08:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14805: (follow-up) fix a bug in previous code and change msg string (2.55 KB, patch)
2015-09-17 11:44 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 14805: Replace generic 'items' with the specific ones (1.75 KB, patch)
2015-09-21 14:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14805: Replace generic 'items' with the specific ones (1.89 KB, patch)
2015-09-22 07:33 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 14805: Allow the user to delete multiple batches (7.94 KB, patch)
2015-09-22 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14805: QA follow-up (3.21 KB, patch)
2015-09-22 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14805: (follow-up) fix a bug in previous code and change msg string (2.62 KB, patch)
2015-09-22 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14805: Replace generic 'items' with the specific ones (1.96 KB, patch)
2015-09-22 09:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14805: Replace generic 'items' with the specific ones (1.96 KB, patch)
2015-11-04 11:02 UTC, Joonas Kylmälä
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Joonas Kylmälä 2015-09-11 08:24:57 UTC
In koha/patroncards/manage.pl?card_element=batch you cannot select multiple batches for deletion. We don't want the user to go through them one by one if the user wants to delete all the batches.
Comment 1 Joonas Kylmälä 2015-09-14 13:34:19 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2015-09-16 12:51:40 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2015-09-17 08:56:27 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-09-17 08:57:35 UTC
Work quite well, but when trying to remove more than 1 profile, the message is "Are you sure you want to delete X batches?", should be 'profiles'.
I haven't tried, but it is certainly the same for templates and layouts.
Comment 5 Joonas Kylmälä 2015-09-17 09:47:20 UTC
(In reply to Jonathan Druart from comment #4)
> Work quite well, but when trying to remove more than 1 profile, the message
> is "Are you sure you want to delete X batches?", should be 'profiles'.
> I haven't tried, but it is certainly the same for templates and layouts.

I see. I'll do a follow-up for that.
Comment 6 Joonas Kylmälä 2015-09-17 11:44:19 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2015-09-17 13:02:06 UTC
Comment on attachment 42647 [details] [review]
Bug 14805: (follow-up) fix a bug in previous code and change msg string

Review of attachment 42647 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/manage.tt
@@ +21,4 @@
>  
>                  var msg;
>                  if (element_ids.length > 1) {
> +                    msg = _("Are you sure you want to delete %s items?").format(element_ids.length, element_ids.length);

Is not possible to call translate_card_element here?
Comment 8 Joonas Kylmälä 2015-09-18 07:17:38 UTC
(In reply to Jonathan Druart from comment #7)
> Comment on attachment 42647 [details] [review] [review]
> Bug 14805: (follow-up) fix a bug in previous code and change msg string
> 
> Review of attachment 42647 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/manage.tt
> @@ +21,4 @@
> >  
> >                  var msg;
> >                  if (element_ids.length > 1) {
> > +                    msg = _("Are you sure you want to delete %s items?").format(element_ids.length, element_ids.length);
> 
> Is not possible to call translate_card_element here?

One way would be by adding if statements for all the different translate_card_elements. I don't know how we could otherwise conjugate the words. Any better ideas than the if statements (or case statements)?
Comment 9 Jonathan Druart 2015-09-21 14:57:55 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2015-09-21 14:58:41 UTC
(In reply to Joonas Kylmälä from comment #8)
> (In reply to Jonathan Druart from comment #7)
> > Comment on attachment 42647 [details] [review] [review] [review]
> > Bug 14805: (follow-up) fix a bug in previous code and change msg string
> > 
> > Review of attachment 42647 [details] [review] [review] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/manage.tt
> > @@ +21,4 @@
> > >  
> > >                  var msg;
> > >                  if (element_ids.length > 1) {
> > > +                    msg = _("Are you sure you want to delete %s items?").format(element_ids.length, element_ids.length);
> > 
> > Is not possible to call translate_card_element here?
> 
> One way would be by adding if statements for all the different
> translate_card_elements. I don't know how we could otherwise conjugate the
> words. Any better ideas than the if statements (or case statements)?

Have a look at my last patch and please sign it off if it works correctly.
Comment 11 Joonas Kylmälä 2015-09-22 07:33:47 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2015-09-22 09:05:29 UTC
Created attachment 42762 [details] [review]
Bug 14805: Allow the user to delete multiple batches

Allows the user to delete multiple patron card batches.

Test plan:

1. Add first some 20 batches
2. Go to the edit page of some X batch by first selecting from the
   batch list the batch you want to edit and then after that by clicking
   edit.
3. Delete one batch by selecting one batch and then clicking delete.
4. Select 0 batches and click delete, notice you cannot delete.
5. Select 0 batches and click edit, notice you cannot edit.
6. Select 2 batches and click edit, notice you cannot edit.
7. Apply patch
8. Check that steps from 2-6 work like they worked before.
   (and add more batches so that you have ~20 of them)
9. Select 2 batches and click delete. Make sure they are deleted.
10. Try to figure out some anomalies this patch might have caused.

Sponsored-by: Vaara-kirjastot

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2015-09-22 09:05:32 UTC
Created attachment 42763 [details] [review]
Bug 14805: QA follow-up

This patches remove a trailing whitespace and simplify 2 parts of code.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 14 Jonathan Druart 2015-09-22 09:05:35 UTC
Created attachment 42764 [details] [review]
Bug 14805: (follow-up) fix a bug in previous code and change msg string

Previously the code didn't work if there was only one batch available for selection.
This fixes that now. Also there was a bug with wrong string showing now it's
changed to a general one, "items". I choosed the string "items" because there was
difficulties adding the plural part of the word. One cannot add just s to the end
of the word batch like you can in the case of profile.

Sponsored-by: Vaara-kirjastot

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Jonathan Druart 2015-09-22 09:05:38 UTC Comment hidden (obsolete)
Comment 16 Tomás Cohen Arazi 2015-11-02 15:27:30 UTC
Please rebase against master, urgently :-D
Comment 17 Joonas Kylmälä 2015-11-04 11:02:03 UTC
Created attachment 44358 [details] [review]
Bug 14805: Replace generic 'items' with the specific ones

Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 18 Joonas Kylmälä 2015-11-04 11:03:39 UTC
(In reply to Tomás Cohen Arazi from comment #16)
> Please rebase against master, urgently :-D

Done! :)
Comment 19 Tomás Cohen Arazi 2015-11-04 14:31:18 UTC
Patches pushed to master.

Thanks Joonas and Jonathan!