|
Description
Kyle M Hall (khall)
2016-01-08 16:59:25 UTC
Created attachment 46443 [details] [review] Bug 15530 - Add Unit Test Created attachment 46444 [details] [review] Bug 15530 - Editing a course item via a disabled course disables it even if it is on other enabled courses It appears that if the course item is edited by clicking the edit link from an active course, the course item will be set to enabled and the fields will be swapped, if the same course item is edited from a course that is *not* active, the course item will be set to *not* enabled, and the original fields will be swapped back in! The short term work-around is to only edit course items from an enabled course if the item has a course that is enabled. If all the courses it is on are disabled, it doesn't matter what course the item is edited from. Test Plan: 1) Create two courses, 1 enabled and 1 disabled 2) Add an item as a course reserve to both courses 3) Edit the course reserve data for the item via the enabled course 4) Note the course item is enabled ( easy way is to check the database ) 5) Edit the same course reserve data, but via the disabled course 6) Note the course item is now disabled even though it is part of an enabled course! 7) Apply this patch 8) Repeat steps 1 through 5 9) Note the course item is still enabled Created attachment 46445 [details] [review] Bug 15530 - Add Unit Test Created attachment 46446 [details] [review] Bug 15530 - Editing a course item via a disabled course disables it even if it is on other enabled courses It appears that if the course item is edited by clicking the edit link from an active course, the course item will be set to enabled and the fields will be swapped, if the same course item is edited from a course that is *not* active, the course item will be set to *not* enabled, and the original fields will be swapped back in! The short term work-around is to only edit course items from an enabled course if the item has a course that is enabled. If all the courses it is on are disabled, it doesn't matter what course the item is edited from. Test Plan: 1) Create two courses, 1 enabled and 1 disabled 2) Add an item as a course reserve to both courses 3) Edit the course reserve data for the item via the enabled course 4) Note the course item is enabled ( easy way is to check the database ) 5) Edit the same course reserve data, but via the disabled course 6) Note the course item is now disabled even though it is part of an enabled course! 7) Apply this patch 8) Repeat steps 1 through 5 9) Note the course item is still enabled Patch tested with a sandbox, by Margaret Holt <mholt@bastyr.edu> Created attachment 46615 [details] [review] Bug 15530 - Add Unit Test Signed-off-by: Margaret Holt <mholt@bastyr.edu> Created attachment 46616 [details] [review] Bug 15530 - Editing a course item via a disabled course disables it even if it is on other enabled courses It appears that if the course item is edited by clicking the edit link from an active course, the course item will be set to enabled and the fields will be swapped, if the same course item is edited from a course that is *not* active, the course item will be set to *not* enabled, and the original fields will be swapped back in! The short term work-around is to only edit course items from an enabled course if the item has a course that is enabled. If all the courses it is on are disabled, it doesn't matter what course the item is edited from. Test Plan: 1) Create two courses, 1 enabled and 1 disabled 2) Add an item as a course reserve to both courses 3) Edit the course reserve data for the item via the enabled course 4) Note the course item is enabled ( easy way is to check the database ) 5) Edit the same course reserve data, but via the disabled course 6) Note the course item is now disabled even though it is part of an enabled course! 7) Apply this patch 8) Repeat steps 1 through 5 9) Note the course item is still enabled Signed-off-by: Margaret Holt <mholt@bastyr.edu> Kyle, where is this enable/disable value used? (In reply to Jonathan Druart from comment #8) > Kyle, where is this enable/disable value used? "enabled" on a course item tells us if the fields have been swapped with the actual corresponding item fields. The incorrect assumption in the code was that the "enabled" value of the course an item was on would match the "enabled" value of the course itself. This is incorrect, as the value of "enabled" should be yes if *any* course the item is on is enabled. So if I understand correctly (not sure at all!), the course_items.enabled values could be retrieved from course_reserves.enabled? In this case, why don't we remove this column instead of trying to keep it up-to-date? (In reply to Jonathan Druart from comment #10) > So if I understand correctly (not sure at all!), the course_items.enabled > values could be retrieved from course_reserves.enabled? > In this case, why don't we remove this column instead of trying to keep it > up-to-date? No, that is not quite right, here is the mapping: courses <=> courses_reserves <=> course_items course_reserves is a many to many join table between courses and course_items, so that a given item can be on course reserve for many courses at once. The short answer is, if all courses a course_item is part of are disabled, then the course_item should be disabled. If *any* of the courses are enabled, the course_item should be enabled. The 'enabled' flag again tells us whether the itype, ccode, holdingbranch and/or location field have been swapped with the originals. Created attachment 47248 [details] [review] Bug 15530 - Add Unit Test Signed-off-by: Margaret Holt <mholt@bastyr.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 47249 [details] [review] Bug 15530 - Editing a course item via a disabled course disables it even if it is on other enabled courses It appears that if the course item is edited by clicking the edit link from an active course, the course item will be set to enabled and the fields will be swapped, if the same course item is edited from a course that is *not* active, the course item will be set to *not* enabled, and the original fields will be swapped back in! The short term work-around is to only edit course items from an enabled course if the item has a course that is enabled. If all the courses it is on are disabled, it doesn't matter what course the item is edited from. Test Plan: 1) Create two courses, 1 enabled and 1 disabled 2) Add an item as a course reserve to both courses 3) Edit the course reserve data for the item via the enabled course 4) Note the course item is enabled ( easy way is to check the database ) 5) Edit the same course reserve data, but via the disabled course 6) Note the course item is now disabled even though it is part of an enabled course! 7) Apply this patch 8) Repeat steps 1 through 5 9) Note the course item is still enabled Signed-off-by: Margaret Holt <mholt@bastyr.edu> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to Master - Should be in the May 2016 release. - Thanks! Patches pushed to 3.22.x, will be in 3.22.3 This patch has been pushed to 3.20.x, will be in 3.20.9. |