Bug 23727 - Editing course reserve items is broken
Summary: Editing course reserve items is broken
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Course reserves (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Kyle M Hall
QA Contact: Katrin Fischer
URL:
Keywords: rel_20_05_target, RM_priority
Depends on:
Blocks: 22630
  Show dependency treegraph
 
Reported: 2019-10-03 09:33 UTC by Katrin Fischer
Modified: 2020-02-20 17:29 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
PDF highlighting the problematic behaviour (352.92 KB, application/pdf)
2019-11-06 12:31 UTC, Katrin Fischer
Details
Bug 23727: Add new columns (3.11 KB, patch)
2020-02-14 13:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23727: Update Schema (2.55 KB, patch)
2020-02-14 13:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23727: Add Koha Object(s) related to course reserves (10.84 KB, patch)
2020-02-14 13:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23727: Add new columns (6.47 KB, patch)
2020-02-19 20:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23727: Update Schema (3.20 KB, patch)
2020-02-19 20:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23727: Add Koha Object(s) related to course reserves (10.84 KB, patch)
2020-02-19 20:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23727: Editing course reserve items is broken (44.12 KB, patch)
2020-02-19 20:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23727: Add new columns (6.47 KB, patch)
2020-02-19 20:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23727: Update Schema (3.20 KB, patch)
2020-02-19 20:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23727: Add Koha Object(s) related to course reserves (10.84 KB, patch)
2020-02-19 20:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23727: Editing course reserve items is broken (44.12 KB, patch)
2020-02-19 20:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23727: Editing course reserve items is broken (44.04 KB, patch)
2020-02-19 20:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 23727: Add new columns (6.51 KB, patch)
2020-02-20 17:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23727: Update Schema (3.25 KB, patch)
2020-02-20 17:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23727: Add Koha Object(s) related to course reserves (10.88 KB, patch)
2020-02-20 17:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23727: Editing course reserve items is broken (44.08 KB, patch)
2020-02-20 17:06 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23727: Add new columns (6.51 KB, patch)
2020-02-20 17:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23727: Update Schema (3.25 KB, patch)
2020-02-20 17:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23727: Add Koha Object(s) related to course reserves (10.88 KB, patch)
2020-02-20 17:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23727: Editing course reserve items is broken (44.08 KB, patch)
2020-02-20 17:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23727: (QA follow-up) Fix script tags (1.56 KB, patch)
2020-02-20 17:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23727: (QA follow-up) Fix for boolean flags (1.03 KB, patch)
2020-02-20 17:18 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2019-10-03 09:33:49 UTC
Adding an item to course reserves and trying to edit any values in a second step does not work. Values are not saved and the table shows all values as "Unchanged".

To test:

- Create a new course
- Find an item, make sure it has a location set
- Add the item to the course, leave all values unchanged
- Edit the course item
- Change all the values
- Verify the table still says "Unchanged"
- Verify the table doesn't show the location
- Edit the item again
- Verify the changed values don't show
Comment 1 Jonathan Druart 2019-10-16 09:46:35 UTC
itype, ccode and holdingbranch from the course_items table can only be set on adding a reserve. At first glance I'd say it is the expected behavior.
Comment 2 Katrin Fischer 2019-10-16 10:31:04 UTC
I don't agree, the GUI suggests otherwiese: We have an "Edit" button. If you can't change those values, we should not be offering them.
Comment 3 Jonathan Druart 2019-10-16 10:50:17 UTC
It changes the values of the item.

I am just saying that it seems it always worked that way :)
Comment 4 Katrin Fischer 2019-11-06 12:31:45 UTC
Created attachment 95104 [details]
PDF highlighting the problematic behaviour
Comment 5 Katrin Fischer 2019-11-06 12:59:23 UTC
Updating to blocker, as this ends in data loss (original item values can't be restored)
Comment 6 Jonathan Druart 2020-02-14 10:11:02 UTC
Kyle, any ideas about how we could fix this correctly?
Comment 7 Kyle M Hall 2020-02-14 12:22:43 UTC
(In reply to Jonathan Druart from comment #6)
> Kyle, any ideas about how we could fix this correctly?

Something bad has happened to CourseReserves::_UpdateCourseItem. This function should check to see if the course is active. If it is inactive, it should edit the values in the course_items table, if it is active, it should edit the values in the items table. In master, it *always* edits the items table, and there is no way to ever update the course_items table no matter if the course is active or inactive!

This is appears to be a deeper more problematic issue than the stated one though. I think the solution for the "Unchanged" issue is to add a 'use/ignore' column for each of the swappable columns (itype_enabled, ccode_enabled, etc), along with a checkbox in the editor to enable them.

This would solves the original issue, that "Unchanged" as a value has been unreliable and weird. The module was first written without the expectation that a library would want an item column to be undef while *not* on course reserve, and have a value while *on* course reserve. Undef was used to signify the "Leave unchanged" status. It seems that after solving the more pressing issue above, moving the swap-ability indicator to it's own column would be the once-and-for-all solution to this problem.
Comment 8 Kyle M Hall 2020-02-14 12:25:14 UTC
I'm wondering if we should have the course items editor *only* update the course items table and *never* the items table. That would also mean disabling the ability to edit course items on active course reserve. This could make the system more understandable, though less flexible.
Comment 9 Kyle M Hall 2020-02-14 13:57:28 UTC
Created attachment 98943 [details] [review]
Bug 23727: Add new columns
Comment 10 Kyle M Hall 2020-02-14 13:57:38 UTC
Created attachment 98944 [details] [review]
Bug 23727: Update Schema
Comment 11 Kyle M Hall 2020-02-14 13:57:41 UTC
Created attachment 98945 [details] [review]
Bug 23727: Add Koha Object(s) related to course reserves
Comment 12 Kyle M Hall 2020-02-14 20:23:05 UTC
Follow work at:

https://github.com/bywatersolutions/bywater-koha-devel/commits/bug_23727
Comment 13 Kyle M Hall 2020-02-19 20:00:12 UTC
Created attachment 99269 [details] [review]
Bug 23727: Add new columns
Comment 14 Kyle M Hall 2020-02-19 20:00:20 UTC
Created attachment 99270 [details] [review]
Bug 23727: Update Schema
Comment 15 Kyle M Hall 2020-02-19 20:00:23 UTC
Created attachment 99271 [details] [review]
Bug 23727: Add Koha Object(s) related to course reserves
Comment 16 Kyle M Hall 2020-02-19 20:00:26 UTC
Created attachment 99272 [details] [review]
Bug 23727: Editing course reserve items is broken

Adding an item to course reserves and trying to edit any values in a second step does not work. Values are not saved and the table shows all values as "Unchanged".

This patch set adds two new sets of columns to the course_items table.

The first set determines if the specified column should be swapped or
not. The was previously 'implied' by the column being set to undef which
has been the root problem with that way of knowing if a column should
swap or not.

The second set of new columns are for storing the item field values
while the item is on course reserve. Previously, the column values
were swapped between the items table and the course_items table,
which leaves ambiguity as to what each value is. Now, the original
columns *always* store the value when the item is on course reserve,
and the new storage columns store the original item value while the
item is on reserve, and are NULL when an item is *not* on reserve.

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
Comment 17 Kyle M Hall 2020-02-19 20:17:37 UTC
Created attachment 99273 [details] [review]
Bug 23727: Add new columns
Comment 18 Kyle M Hall 2020-02-19 20:17:45 UTC
Created attachment 99274 [details] [review]
Bug 23727: Update Schema
Comment 19 Kyle M Hall 2020-02-19 20:17:49 UTC
Created attachment 99275 [details] [review]
Bug 23727: Add Koha Object(s) related to course reserves
Comment 20 Kyle M Hall 2020-02-19 20:17:52 UTC
Created attachment 99276 [details] [review]
Bug 23727: Editing course reserve items is broken

Adding an item to course reserves and trying to edit any values in a second step does not work. Values are not saved and the table shows all values as "Unchanged".

This patch set adds two new sets of columns to the course_items table.

The first set determines if the specified column should be swapped or
not. The was previously 'implied' by the column being set to undef which
has been the root problem with that way of knowing if a column should
swap or not.

The second set of new columns are for storing the item field values
while the item is on course reserve. Previously, the column values
were swapped between the items table and the course_items table,
which leaves ambiguity as to what each value is. Now, the original
columns *always* store the value when the item is on course reserve,
and the new storage columns store the original item value while the
item is on reserve, and are NULL when an item is *not* on reserve.

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
Comment 21 Kyle M Hall 2020-02-19 20:31:04 UTC
Created attachment 99277 [details] [review]
Bug 23727: Editing course reserve items is broken

Adding an item to course reserves and trying to edit any values in a second step does not work. Values are not saved and the table shows all values as "Unchanged".

This patch set adds two new sets of columns to the course_items table.

The first set determines if the specified column should be swapped or
not. The was previously 'implied' by the column being set to undef which
has been the root problem with that way of knowing if a column should
swap or not.

The second set of new columns are for storing the item field values
while the item is on course reserve. Previously, the column values
were swapped between the items table and the course_items table,
which leaves ambiguity as to what each value is. Now, the original
columns *always* store the value when the item is on course reserve,
and the new storage columns store the original item value while the
item is on reserve, and are NULL when an item is *not* on reserve.

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
Comment 22 Martin Renvoize 2020-02-20 16:49:06 UTC
Just testing this now.. so far so good
Comment 23 Martin Renvoize 2020-02-20 17:06:44 UTC
Created attachment 99327 [details] [review]
Bug 23727: Add new columns

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 24 Martin Renvoize 2020-02-20 17:06:47 UTC
Created attachment 99328 [details] [review]
Bug 23727: Update Schema

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 25 Martin Renvoize 2020-02-20 17:06:50 UTC
Created attachment 99329 [details] [review]
Bug 23727: Add Koha Object(s) related to course reserves

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize 2020-02-20 17:06:54 UTC
Created attachment 99330 [details] [review]
Bug 23727: Editing course reserve items is broken

Adding an item to course reserves and trying to edit any values in a second step does not work. Values are not saved and the table shows all values as "Unchanged".

This patch set adds two new sets of columns to the course_items table.

The first set determines if the specified column should be swapped or
not. The was previously 'implied' by the column being set to undef which
has been the root problem with that way of knowing if a column should
swap or not.

The second set of new columns are for storing the item field values
while the item is on course reserve. Previously, the column values
were swapped between the items table and the course_items table,
which leaves ambiguity as to what each value is. Now, the original
columns *always* store the value when the item is on course reserve,
and the new storage columns store the original item value while the
item is on reserve, and are NULL when an item is *not* on reserve.

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

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 27 Martin Renvoize 2020-02-20 17:17:46 UTC
Created attachment 99331 [details] [review]
Bug 23727: Add new columns

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 28 Martin Renvoize 2020-02-20 17:17:49 UTC
Created attachment 99332 [details] [review]
Bug 23727: Update Schema

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 29 Martin Renvoize 2020-02-20 17:17:52 UTC
Created attachment 99333 [details] [review]
Bug 23727: Add Koha Object(s) related to course reserves

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 30 Martin Renvoize 2020-02-20 17:17:56 UTC
Created attachment 99334 [details] [review]
Bug 23727: Editing course reserve items is broken

Adding an item to course reserves and trying to edit any values in a second step does not work. Values are not saved and the table shows all values as "Unchanged".

This patch set adds two new sets of columns to the course_items table.

The first set determines if the specified column should be swapped or
not. The was previously 'implied' by the column being set to undef which
has been the root problem with that way of knowing if a column should
swap or not.

The second set of new columns are for storing the item field values
while the item is on course reserve. Previously, the column values
were swapped between the items table and the course_items table,
which leaves ambiguity as to what each value is. Now, the original
columns *always* store the value when the item is on course reserve,
and the new storage columns store the original item value while the
item is on reserve, and are NULL when an item is *not* on reserve.

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

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 31 Martin Renvoize 2020-02-20 17:17:59 UTC
Created attachment 99335 [details] [review]
Bug 23727: (QA follow-up) Fix script tags
Comment 32 Martin Renvoize 2020-02-20 17:18:02 UTC
Created attachment 99336 [details] [review]
Bug 23727: (QA follow-up) Fix for boolean flags
Comment 33 Martin Renvoize 2020-02-20 17:18:27 UTC
Works as expected, Signing off.