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
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
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>
Created attachment 82043 [details] [review] Bug 21777: Add a regression test Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(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.
Works as expected, no complaints from qa-test-tools. But is the DB update really necessary ? It works without it.
I think the goal was to hide the hidden date column - but as it's preset in Yaml it probably works without?
(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.
(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
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
Nick do you agree with removing the atomic update?
(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
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>
(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.
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>
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>
Created attachment 82320 [details] [review] Bug 21777: (QA follow-up) Fix colspan in the footer Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Awesome work all! Pushed to master for 18.11
Pushed to 18.05.x for 18.05.06
Depends on Bug 21562 not in 17.11.x