Bug 22914 - Add holds column to batch item delete to fix show/hide columns behaviour
Summary: Add holds column to batch item delete to fix show/hide columns behaviour
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 19490
Blocks:
  Show dependency treegraph
 
Reported: 2019-05-15 14:44 UTC by Lucas Gass
Modified: 2019-05-30 23:42 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.05.00, 18.11.06


Attachments
Bug 22914: Batch Item Delete column misconfigured when using show/hide tool (2.07 KB, patch)
2019-05-15 15:33 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 22914: Batch Item Delete column misconfigured when using show/hide tool (1.96 KB, patch)
2019-05-15 15:52 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 22742: ILS-DI - Fix datetime format returned by RenewLoan (1.30 KB, patch)
2019-05-15 16:52 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22742: ILS-DI - Fix datetime format returned by RenewLoan (1.36 KB, patch)
2019-05-15 17:01 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22914: Batch Item Delete column misconfigured when using show/hide tool (1.96 KB, patch)
2019-05-15 17:06 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 22914: Batch Item Delete column misconfigured when using show/hide tool (4.36 KB, patch)
2019-05-15 22:54 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 22914: Batch Item Delete column misconfigured when using show/hide tool (4.41 KB, patch)
2019-05-16 18:42 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 22914: Add holds column to batch item delete to fix show/hide columns behaviour (5.11 KB, patch)
2019-05-28 06:07 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass 2019-05-15 14:44:21 UTC
When using the batch delete tool the show/hide columns checkboxes are misconfigured. Instead of hiding/showing the proper column the column immediately to the right is instead hidden. This effects master and 18.11.x
Comment 1 Lucas Gass 2019-05-15 15:33:18 UTC
Created attachment 89785 [details] [review]
Bug 22914: Batch Item Delete column misconfigured when using show/hide tool

TEST PLAN:
1. Go to Tools > Batch item deletion and enter at least one barcode.
2. Try to hide a column that is showing 
3. Notice it hides the wrong column
4. apply patch
5. repeat steps 1 and 2.
6. Notice the correct column is being hidden
Comment 2 Lucas Gass 2019-05-15 15:52:40 UTC
Created attachment 89787 [details] [review]
Bug 22914: Batch Item Delete column misconfigured when using show/hide tool
Comment 3 Lucas Gass 2019-05-15 15:57:27 UTC
attachment 89787 [details] [review] fixes a bad filter, the test plan remains the same. 

TEST PLAN:
1. Go to Tools > Batch item deletion and enter at least one barcode.
2. Try to hide a column that is showing 
3. Notice it hides the wrong column
4. apply patch
5. repeat steps 1 and 2.
6. Notice the correct column is being hidden
Comment 4 ByWater Sandboxes 2019-05-15 16:52:44 UTC Comment hidden (obsolete)
Comment 5 ByWater Sandboxes 2019-05-15 17:01:12 UTC Comment hidden (obsolete)
Comment 6 Lucas Gass 2019-05-15 17:06:54 UTC
Created attachment 89790 [details] [review]
Bug 22914: Batch Item Delete column misconfigured when using show/hide tool
Comment 7 Jonathan Druart 2019-05-15 22:14:12 UTC
Applying: Bug 22914: Batch Item Delete column misconfigured when using show/hide tool
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-del.tt).
error: could not build fake ancestor
Comment 8 Jonathan Druart 2019-05-15 22:14:27 UTC
Please provide a patch for master, Lucas.
Comment 9 Lucas Gass 2019-05-15 22:54:58 UTC
Created attachment 89796 [details] [review]
Bug 22914: Batch Item Delete column misconfigured when using show/hide tool

TEST PLAN:
1. Go to Tools > Batch item deletion and enter at least one barcode.
2. Try to hide a column that is showing 
3. Notice it hides the wrong column
4. apply patch
5. repeat steps 1 and 2.
6. Notice the correct column is being hidden
Comment 10 Jonathan Druart 2019-05-15 23:53:31 UTC
Hi Lucas,
You are doing much more that what is advertised. A new column is added (the one added by bug 19490).

See also bug 8132.
Comment 11 Nick Clemens 2019-05-16 00:04:03 UTC
The changes on 19490 caused this, the two templates share a js file, and the data passed is the same.

Adding the column to the template brings things back into alignment, the data is already there, we just show it. We could leave out the whitespace changes, but I think they make it much easier to read

Ultimately the item display should be an include used by both templates, but adding the column now seems a better idea than adding logic to the js to act differently in the two pages
Comment 12 Jonathan Druart 2019-05-16 00:55:37 UTC
Yes Nick, it is exactly why I let this comment. With more info I wouldn't have needed to look after them :)

And also why I linked it with bug 8132. People seems to want this info (number of holds) on this page. I was asking to advertise it, like it was a new enhancement, not only a 'misalignment bug'.
Comment 13 Lucas Gass 2019-05-16 00:59:46 UTC
Thanks Jonathan. I will be more precise with advertisement in the future. :)
Comment 14 ByWater Sandboxes 2019-05-16 18:42:18 UTC
Created attachment 89869 [details] [review]
Bug 22914: Batch Item Delete column misconfigured when using show/hide tool

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>
Comment 15 Jonathan Druart 2019-05-16 19:03:03 UTC
(In reply to Lucas Gass from comment #13)
> Thanks Jonathan. I will be more precise with advertisement in the future. :)

Why in the future when you can start now? :)
Comment 16 Katrin Fischer 2019-05-28 06:01:35 UTC
This is a hard one, because I really want to pass it, but I feel it's not quite right yet. :(

As this is a bug report for a specific problem that also appears in 18.11, it is suited for backporting. But as it adds a new column, this is also an enhancement (with translation changes too), so it would not be backported. Also: because this only advertises the bug fix, we would not alert the libraries about the new feature in the release notes.

Lucas, would you be willing to please split this into 2 separate bugs? One for the fix and one for the new feature? The heavy reindentation also makes it hard to read, having that in a first patch and the actual fix in a second patch would also make my life easier!
Comment 17 Katrin Fischer 2019-05-28 06:03:50 UTC
Hm, seeing the discussion now. I still feel this puts us in a little bit of a hard place release notes wise :)
Comment 18 Katrin Fischer 2019-05-28 06:07:55 UTC
Created attachment 90139 [details] [review]
Bug 22914: Add holds column to batch item delete to fix show/hide columns behaviour

The changes on 19490 caused a bug, where the show/hide column feature hides the
wrong column on the items table. The two templates share a js file, and
the data passed is the same.

Adding the column to the template brings things back into alignment,
the data is already there, we just show it.

TEST PLAN:
1. Go to Tools > Batch item deletion and enter at least one barcode.
2. Try to hide a column that is showing
3. Notice it hides the wrong column
4. apply patch
5. repeat steps 1 and 2.
6. Notice the correct column is being hidden
7. Verify the new column for number of holds on the item works correctly
   for item and record level holds

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 19 Katrin Fischer 2019-05-28 06:08:57 UTC
I decided to pass this after fixing up the bug title and commit message. Lucas, please always include a description of the problem and the most current test plan in your commit messages! Thx!
Comment 20 Nick Clemens 2019-05-30 10:39:11 UTC
Awesome work all!

Pushed to master for 19.05
Comment 21 Martin Renvoize 2019-05-30 20:26:56 UTC
Pushed to 18.11.x for 18.11.06