Bug 19231 - No warning of number of attached items when deleting a course
Summary: No warning of number of attached items when deleting a course
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Course reserves (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-31 22:01 UTC by Aleisha Amohia
Modified: 2017-10-27 19:11 UTC (History)
2 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19231: Add number of attached items to confirm message when deleting course (2.68 KB, patch)
2017-08-31 22:06 UTC, Aleisha Amohia
Details | Diff | Splinter Review
showing expected warning pop up (113.32 KB, image/png)
2017-09-12 01:07 UTC, Aleisha Amohia
Details
Bug 19231: Add number of attached items to confirm message when deleting course (2.75 KB, patch)
2017-09-12 06:24 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 19231: Add number of attached items to confirm message when deleting course (2.81 KB, patch)
2017-10-27 14:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19231: No need to pass the size to the template (1.93 KB, patch)
2017-10-27 18:30 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2017-08-31 22:01:52 UTC
There is a confirm message when attempting to delete a course that has items in it, but it should also warn about the number of items in the course. If a course has many items in it they may decide not to delete it, or realise they are deleting the wrong one.
Comment 1 Aleisha Amohia 2017-08-31 22:06:14 UTC
Created attachment 66679 [details] [review]
Bug 19231: Add number of attached items to confirm message when deleting course

To test:
1) Ensure UseCourseReserves is enabled
2) Go to Course Reserves and create 3 new courses
3) Add one item to a course, add two items to another course, and add no
items to the third course
4) Delete the course with no items. Confirm the error message shows up.
No need to say how many attached items there are here (because there are
none).
5) Delete the course with one item. Confirm the error message shows and
warns of the one attached item and makes grammatical sense
6) Delete the course with two items. Confirm the error message shows and
shows the correct number of attached items and makes grammatical sense
7) Confirm clicking 'Cancel' and 'OK' does what is expected

Sponsored-by: Catalyst IT
Comment 2 Claire Gravely 2017-09-07 12:17:30 UTC
Hi Aleisha,
I only got the normal warning message, but didn't get any information about the number of items attached.
Comment 3 Dominic Pichette 2017-09-08 20:42:14 UTC
I followed the steps and go the same results as Claire did. No error messages, only the normal warning were shown, none specified the items attached to the course.
Comment 4 Aleisha Amohia 2017-09-12 01:07:32 UTC
Created attachment 67069 [details]
showing expected warning pop up

(In reply to Claire Gravely from comment #2)
> Hi Aleisha,
> I only got the normal warning message, but didn't get any information about
> the number of items attached.

(In reply to Dominic Pichette from comment #3)
> I followed the steps and go the same results as Claire did. No error
> messages, only the normal warning were shown, none specified the items
> attached to the course.

Thank you both for testing, sorry I wasn't clear in my test plan. The normal warning pop up should be what shows, not an error message, that was my mistake. However, for courses that have more than one item in course, there should be an extra sentence in the warning, such as in the screenshot attached. I have tested and it is showing for me. Could you please test again?
Comment 5 Claire Gravely 2017-09-12 06:24:35 UTC
Created attachment 67071 [details] [review]
Bug 19231: Add number of attached items to confirm message when deleting course

To test:
1) Ensure UseCourseReserves is enabled
2) Go to Course Reserves and create 3 new courses
3) Add one item to a course, add two items to another course, and add no
items to the third course
4) Delete the course with no items. Confirm the error message shows up.
No need to say how many attached items there are here (because there are
none).
5) Delete the course with one item. Confirm the error message shows and
warns of the one attached item and makes grammatical sense
6) Delete the course with two items. Confirm the error message shows and
shows the correct number of attached items and makes grammatical sense
7) Confirm clicking 'Cancel' and 'OK' does what is expected

Sponsored-by: Catalyst IT
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>

Looks good!
Comment 6 Kyle M Hall 2017-10-27 14:41:41 UTC
Created attachment 68726 [details] [review]
Bug 19231: Add number of attached items to confirm message when deleting course

To test:
1) Ensure UseCourseReserves is enabled
2) Go to Course Reserves and create 3 new courses
3) Add one item to a course, add two items to another course, and add no
items to the third course
4) Delete the course with no items. Confirm the error message shows up.
No need to say how many attached items there are here (because there are
none).
5) Delete the course with one item. Confirm the error message shows and
warns of the one attached item and makes grammatical sense
6) Delete the course with two items. Confirm the error message shows and
shows the correct number of attached items and makes grammatical sense
7) Confirm clicking 'Cancel' and 'OK' does what is expected

Sponsored-by: Catalyst IT
Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>

Looks good!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Jonathan Druart 2017-10-27 18:30:21 UTC
Created attachment 68783 [details] [review]
Bug 19231: No need to pass the size to the template

We have the array already, we just need to call .size on it
Comment 8 Jonathan Druart 2017-10-27 19:11:45 UTC
Pushed to master for 17.11, thanks to everybody involved!