https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23727
--- Comment #40 from Marcel de Rooy <[email protected]> --- Generally, we are doing refactoring here and resolving a critical bug. We should not. To resolve a critical, I do not expect 876 insertions(+), 172 deletions. Test Plan: 1) Apply this patch 2) Add and edit course items, not the new checkboxes for enabling fields 3) Everything should function as before Rather thin test plan for 800+ lines ? As mentioned on IRC, would have been nice to get a librarian signoff (who knows the topic) for a patch set of this size (instead of the RM now). Glancing through the code: C4 CourseReserves + my $course = Koha::Courses->find( $course_id ); $course->{'instructors'} = $sth->fetchall_arrayref( {} ); => This is a weird way of mixing Koha Objects / DBIx and old school DBI. "the new storage columns store the original item value" The name field_storage is not clear to me right away. If you mean original value, we could think of a better name? No blocker. itype_enabled = 'no' `itype_enabled` tinyint(1) Came across that somewhere. Looks odd. + UPDATE course_items SET + itype = IF( itype_enabled = 'no', NULL, itype ), + ccode = IF( ccode_enabled = 'no', NULL, ccode ), + holdingbranch = IF( holdingbranch_enabled = 'no', NULL, holdingbranch ), + location = IF( location_enabled = 'no', NULL, location ) + WHERE enabled = 'no'; The course_items table has a field enabled (enum yes no). Does this field need to be there? If an item is enabled, it is in the table, right? When I remove it from the course, the record is gone. Interface for add_items. The meaning of the checkbox is not obvious. Conclusion: I should probably set it to Failed QA, which I wont. Since we are on a critical here and it seems to work now, I am passing QA. But note that this area needs more work. Please provide feedback to questions raised. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
