[Koha-bugs] [Bug 23727] Editing course reserve items is broken

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23727

--- Comment #40 from Marcel de Rooy <m.de.rooy at rijksmuseum.nl> ---
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.