Bug 21777 - Checkouts table in circulation is out of alignment
Summary: Checkouts table in circulation is out of alignment
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Target Milestone: ---
Assignee: Nick Clemens
QA Contact: Tomás Cohen Arazi
URL:
Whiteboard:
Keywords:
Depends on: 21562 21775
Blocks: 22020
  Show dependency treegraph
 
Reported: 2018-11-06 21:04 UTC by Nick Clemens
Modified: 2018-12-19 19:01 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 21777: Fix circulation table alignment (3.56 KB, patch)
2018-11-07 12:20 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 21777: Fix circulation table alignment (3.63 KB, patch)
2018-11-07 13:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21777: Add a regression test (4.17 KB, patch)
2018-11-07 17:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21777: Fix circulation table alignment (2.68 KB, patch)
2018-11-12 17:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21777: Add a regression test (4.21 KB, patch)
2018-11-13 15:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21777: Fix circulation table alignment (2.71 KB, patch)
2018-11-13 15:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21777: (QA follow-up) Fix colspan in the footer (1.17 KB, patch)
2018-11-13 18:17 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2018-11-06 21:04:22 UTC
On bug 21562 a scret sorting column was added, however, we didn't add the column on the table, only on the rows fetch by ajax checkouts

Also we need a DB update to default the column to hidden and cannot be toggled by default
Comment 1 Nick Clemens 2018-11-07 12:20:14 UTC
Created attachment 82021 [details] [review]
Bug 21777: Fix circulation table alignment

TO test:
1 - Check out an item to a patron
2 - Note the columns are misaligned
3 - Apply patch
4 - Updatedatabase
5 - Reload the patron
6 - Note the table is fixed
Comment 2 Jonathan Druart 2018-11-07 13:23:33 UTC
Created attachment 82023 [details] [review]
Bug 21777: Fix circulation table alignment

TO test:
1 - Check out an item to a patron
2 - Note the columns are misaligned
3 - Apply patch
4 - Updatedatabase
5 - Reload the patron
6 - Note the table is fixed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 3 Jonathan Druart 2018-11-07 17:24:16 UTC
Created attachment 82043 [details] [review]
Bug 21777: Add a regression test

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 4 Jonathan Druart 2018-11-07 17:26:22 UTC
(In reply to Jonathan Druart from comment #3)
> Created attachment 82043 [details] [review] [review]
> Bug 21777: Add a regression test
> 
> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

These tests will apply for master on top of bug 21775.
Comment 5 Julian Maurice 2018-11-09 13:39:17 UTC
Works as expected, no complaints from qa-test-tools.
But is the DB update really necessary ? It works without it.
Comment 6 Katrin Fischer 2018-11-09 13:42:38 UTC
I think the goal was to hide the hidden date column - but as it's preset in Yaml it probably works without?
Comment 7 Julian Maurice 2018-11-09 13:45:06 UTC
(In reply to Katrin Fischer from comment #6)
> I think the goal was to hide the hidden date column - but as it's preset in
> Yaml it probably works without?

It certainly works without ;)
That's why I'm thinking the DB update is superfluous and can be removed.
Comment 8 Jonathan Druart 2018-11-09 13:49:36 UTC
(In reply to Julian Maurice from comment #7)
> (In reply to Katrin Fischer from comment #6)
> > I think the goal was to hide the hidden date column - but as it's preset in
> > Yaml it probably works without?
> 
> It certainly works without ;)
> That's why I'm thinking the DB update is superfluous and can be removed.

Sure? I tried without IIRC.
- Do not apply the patch
- admin/columns_settings.pl > Tick some checkboxes for this table
- apply the patch, do not execute the update DB
- go to the admin and confirm that you see the new column
Comment 9 Julian Maurice 2018-11-09 13:59:53 UTC
Here's what I tried:
1) Turn off memcached
2) DELETE FROM columns_settings
3) git checkout master && restart starman

I can reproduce the bug and the column checkout_on_unformatted is not in admin page

4) Check some boxes and save, now columns_settings is filled with a lot of lines (but no checkout_on_unformatted)
5) git checkout qa/21777 && restart starman
6) Refresh admin page: checkout_on_unformatted appears both boxes checked and not modifiable, checkout_on_unformatted is not in columns_settings
7) Go to circulation page, the bug is gone
Comment 10 Jonathan Druart 2018-11-12 14:06:43 UTC
Nick do you agree with removing the atomic update?
Comment 11 Nick Clemens 2018-11-12 15:18:38 UTC
(In reply to Jonathan Druart from comment #10)
> Nick do you agree with removing the atomic update?

Yes, I copied the update from another big and thought it was needed. If not it can be removed
Comment 12 Jonathan Druart 2018-11-12 17:54:17 UTC
Created attachment 82231 [details] [review]
Bug 21777: Fix circulation table alignment

TO test:
1 - Check out an item to a patron
2 - Note the columns are misaligned
3 - Apply patch
4 - Updatedatabase
5 - Reload the patron
6 - Note the table is fixed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Jonathan Druart 2018-11-12 17:54:54 UTC
(In reply to Nick Clemens from comment #11)
> (In reply to Jonathan Druart from comment #10)
> > Nick do you agree with removing the atomic update?
> 
> Yes, I copied the update from another big and thought it was needed. If not
> it can be removed

Patch amended, file removed.
Comment 14 Tomás Cohen Arazi 2018-11-13 15:01:31 UTC
Created attachment 82287 [details] [review]
Bug 21777: Add a regression test

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Tomás Cohen Arazi 2018-11-13 15:01:39 UTC
Created attachment 82288 [details] [review]
Bug 21777: Fix circulation table alignment

TO test:
1 - Check out an item to a patron
2 - Note the columns are misaligned
3 - Apply patch
4 - Updatedatabase
5 - Reload the patron
6 - Note the table is fixed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Tomás Cohen Arazi 2018-11-13 18:17:28 UTC
Created attachment 82320 [details] [review]
Bug 21777: (QA follow-up) Fix colspan in the footer

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 17 Nick Clemens 2018-11-13 21:15:55 UTC
Awesome work all!

Pushed to master for 18.11
Comment 18 Fridolin SOMERS 2018-11-26 06:44:05 UTC
Pushed to 18.05.x for 18.05.06
Comment 19 Fridolin SOMERS 2018-11-28 10:19:01 UTC
Depends on Bug 21562 not in 17.11.x