Bug 19230

I think it would be better to assign a $course_id variable and reuse it.
Something like:

modified: course_reserves/mod_course.pl
@ mod_course.pl:41 @ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 );

 my $action = $cgi->param('action') || '';
+my $course_id = $cgi->param('course_id');

 if ( $action eq 'del' ) {
-    DelCourse( $cgi->param('course_id') );
+    DelCourse( $course_id );
     print $cgi->redirect("/cgi-bin/koha/course_reserves/course-reserves.pl");
 } else {
     my %params;

-    $params{'course_id'} = $cgi->param('course_id')
-      if ( $cgi->param('course_id') );
+    $params{'course_id'} = $course_id;
     $params{'department'}     = $cgi->param('department');
     $params{'course_number'}  = $cgi->param('course_number');
     $params{'section'}        = $cgi->param('section');
@ mod_course.pl:60 @ if ( $action eq 'del' ) {
     $params{'students_count'} = $cgi->param('students_count');
     $params{'enabled'}        = ( $cgi->param('enabled') eq 'on' ) ? 'yes' : 'no';

-    my $course_id = ModCourse(%params);
+    my $new_course_id = ModCourse(%params);

     my @instructors = $cgi->multi_param('instructors');
     ModCourseInstructors(
         mode        => 'replace',
         cardnumbers => \@instructors,
-        course_id   => $course_id
+        course_id   => $new_course_id
     );
-    print $cgi->redirect("/cgi-bin/koha/course_reserves/course-details.pl?course_id=$course_id");
+    print $cgi->redirect("/cgi-bin/koha/course_reserves/course-details.pl?course_id=$new_course_id");
 }

Would it make more sense?

Created attachment 69946 [details] [review]
Bug 19230: Preventing warn when deleting course

To test:
1) Ensure UseCourseReserves is enabled
2) Go to Course Reserves, create a course
3) Delete course
4) Notice warn
5) Apply patch
6) Create a course again and delete it
7) Notice no warn

Sponsored-by: Catalyst IT

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

(In reply to Jonathan Druart from comment #3)

> Would it make more sense?

hi Jonathan,
Yes I believe your fix makes more sense so I have implemented in the next patch.

When I was testing, I also found a new warn when looking at a course with NO course reserves in it (course_reserves/course-details.pl)
Argument "" isn't numeric in numeric gt (>) at /home/vagrant/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt line 219.

Also, when deleting a course with no course reserves, the confirm message is not triggered.

I've added a fix for this also in my next patch.

Created attachment 69985 [details] [review]
Bug 19230: Preventing warn when deleting course

To test:
1) Ensure UseCourseReserves is enabled
2) Go to Course Reserves, create a course
3) When you are taken to the course page, you should notice the
'Argument "" isn't numeric in numeric gt' warn
4) Delete course
5) Notice warn, and that no confirm message popped up
6) Apply patch
7) Create a course again. Confirm the 'Argument "" isn't numeric in
numeric gt' warn did not show
8) Delete the course
9) Notice no warn and the confirm message pops up

Sponsored-by: Catalyst IT

Created attachment 70780 [details] [review]
Bug 19230: Preventing warn when deleting course

To test:
1) Ensure UseCourseReserves is enabled
2) Go to Course Reserves, create a course
3) When you are taken to the course page, you should notice the
'Argument "" isn't numeric in numeric gt' warn
4) Delete course
5) Notice warn, and that no confirm message popped up
6) Apply patch
7) Create a course again. Confirm the 'Argument "" isn't numeric in
numeric gt' warn did not show
8) Delete the course
9) Notice no warn and the confirm message pops up

Sponsored-by: Catalyst IT
Signed-off-by: Roch D'Amour <roch.damour@inlibro.com>

Created attachment 70933 [details] [review]
Bug 19230: Preventing warn when deleting course

To test:
1) Ensure UseCourseReserves is enabled
2) Go to Course Reserves, create a course
3) When you are taken to the course page, you should notice the
'Argument "" isn't numeric in numeric gt' warn
4) Delete course
5) Notice warn, and that no confirm message popped up
6) Apply patch
7) Create a course again. Confirm the 'Argument "" isn't numeric in
numeric gt' warn did not show
8) Delete the course
9) Notice no warn and the confirm message pops up

Sponsored-by: Catalyst IT
Signed-off-by: Roch D'Amour <roch.damour@inlibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Created attachment 71067 [details] [review]
Bug 19230: (follow-up) Preventing warn when deleting course

Simplify 'unless' statement

Pushed to master for 18.05, thanks to everybody involved!

Awesome work all!

This patch has been pushed to 17.11.x and will be in 17.11.03