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.
Created attachment 42522 [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
Created attachment 42592 [details] [review] [Signed-off] 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>
Created attachment 42639 [details] [review] Bug 14805: QA follow-up This patches remove a trailing whitespace and simplify 2 parts of code.
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.
(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.
Created attachment 42647 [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
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?
(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)?
Created attachment 42725 [details] [review] Bug 14805: Replace generic 'items' with the specific ones
(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.
Created attachment 42752 [details] [review] Bug 14805: Replace generic 'items' with the specific ones The case mechanism works, and it is okay for me. Signed-off-by: Joonas Kylmälä <j.kylmala@gmail.com>
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>
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>
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>
Created attachment 42765 [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>
Please rebase against master, urgently :-D
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>
(In reply to Tomás Cohen Arazi from comment #16) > Please rebase against master, urgently :-D Done! :)
Patches pushed to master. Thanks Joonas and Jonathan!