Bug 31435 - "Configure this table" appears for non-configurable tables
Summary: "Configure this table" appears for non-configurable tables
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Martin Renvoize
QA Contact: Joonas Kylmälä
URL:
Keywords:
Depends on: 29723
Blocks: 33705
  Show dependency treegraph
 
Reported: 2022-08-22 07:13 UTC by Martin Renvoize
Modified: 2023-12-28 20:42 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00


Attachments
Bug 31435: Tie 'Configure' to table_settings (1.09 KB, patch)
2022-08-22 07:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31435: Tie 'Configure' to table_settings (1.84 KB, patch)
2022-08-22 07:23 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31435: Tie 'Configure' to table_settings (1.84 KB, patch)
2022-09-05 13:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31435: (follow-up) Make kohaTable calls consistent (3.08 KB, patch)
2022-09-05 13:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31435: (follow-up) Make KohaTable calls consistent (4.49 KB, patch)
2022-09-05 13:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31435: Tie 'Configure' to table_settings (1.84 KB, patch)
2022-09-07 07:20 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31435: (follow-up) Make kohaTable calls consistent (3.08 KB, patch)
2022-09-07 07:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31435: Tie 'Configure' to table_settings (2.11 KB, patch)
2022-09-07 09:31 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31435: (follow-up) Make kohaTable calls consistent (3.13 KB, patch)
2022-09-07 09:32 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31435: Tie 'Configure' to table_settings (2.25 KB, patch)
2022-09-07 15:53 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 31435: (follow-up) Make kohaTable calls consistent (3.28 KB, patch)
2022-09-07 15:53 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 31435: (QA follow-up) Don't pass explicitly default parameter values (1.83 KB, patch)
2022-09-07 15:53 UTC, Joonas Kylmälä
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2022-08-22 07:13:42 UTC
I missed something during QA of bug 29723. It appears that the button will appear regardless of whether table_settings is passed into the KohaTable/kohaTable instantiator.

This is a bug and the button leads to a server error.
Comment 1 Martin Renvoize 2022-08-22 07:14:36 UTC
Spotted by Owen when testing bug 31028

 * I get an error in the browser console when I click "Configure" on the datatable on the catalog concerns page.
Comment 2 Martin Renvoize 2022-08-22 07:17:23 UTC
Created attachment 139567 [details] [review]
Bug 31435: Tie 'Configure' to table_settings

This patch adds a check for table_settings to prevent the display of
'Configure this table' when table settings are not passed to the
datatable wrapper.
Comment 3 Martin Renvoize 2022-08-22 07:23:51 UTC
Created attachment 139570 [details] [review]
Bug 31435: Tie 'Configure' to table_settings

This patch adds a check for table_settings to prevent the display of
'Configure this table' when table settings are not passed to the
datatable wrapper.
Comment 4 Lucas Gass 2022-08-22 22:33:21 UTC
Is there a test plan? Maybe a list of tables we should check?
Comment 5 Martin Renvoize 2022-09-05 13:17:11 UTC
Created attachment 140209 [details] [review]
Bug 31435: Tie 'Configure' to table_settings

This patch adds a check for table_settings to prevent the display of
'Configure this table' when table settings are not passed to the
datatable wrapper.
Comment 6 Martin Renvoize 2022-09-05 13:17:15 UTC
Created attachment 140210 [details] [review]
Bug 31435: (follow-up) Make kohaTable calls consistent

Our calls to instantiate a kohaTable from JS were inconsistent in how
they pass table_settings where no table_settings exist.

This patch replaces empty arrays and empty hashrefs with a consistent
'null' being passed to ake it clear we don't have any settings to pass
yet.
Comment 7 Martin Renvoize 2022-09-05 13:17:18 UTC
Created attachment 140211 [details] [review]
Bug 31435: (follow-up) Make KohaTable calls consistent

Our calls to instantiate a KohaTable from were inconsistent in how
they pass table_settings where no table_settings exist.

This patch replaces empty arrays and empty hashrefs with a consistent
'null' being passed to ake it clear we don't have any settings to pass
yet.
Comment 8 Martin Renvoize 2022-09-05 13:19:02 UTC
OK.. so whilst trying to come up with a test plan I spotted a few inconsistencies in how we're instantiating our kohaTables and KohaTables.

Instead of not passing anything, or passing an explicit 'null', we were passing empty arrays and empty hashrefs in some places.. my follow-ups correct that for consistency.

In short, the tables affected by the two follow-up patches will all have displayed the 'Configure' link even when there's nothing to configure... that's the test plan ;P
Comment 9 Joonas Kylmälä 2022-09-05 18:39:19 UTC
In the second patch the file koha-tmpl/intranet-tmpl/prog/en/modules/tools/quotes.tt has undef instead of null.

Not sure I agree with the last patch, it is unnecessary information exposed to the programmer, since there is a default, no settings, then I don't see why it would have to be marked explictly, we don't supply the other KohaTable add_filters parameter either. I'd suggest removing the last patch.
Comment 10 Martin Renvoize 2022-09-07 07:18:50 UTC
In retrospect, I agree with you Joonas.. I wrote the patch as a way to record that I'd gone through all the templates that call KohaTable.. but sending nothing is perfectly OK.. it's sending and empty hash or empty array that's an issue.
Comment 11 Martin Renvoize 2022-09-07 07:20:59 UTC
Created attachment 140260 [details] [review]
Bug 31435: Tie 'Configure' to table_settings

This patch adds a check for table_settings to prevent the display of
'Configure this table' when table settings are not passed to the
datatable wrapper.
Comment 12 Martin Renvoize 2022-09-07 07:21:02 UTC
Created attachment 140261 [details] [review]
Bug 31435: (follow-up) Make kohaTable calls consistent

Our calls to instantiate a kohaTable from JS were inconsistent in how
they pass table_settings where no table_settings exist.

This patch replaces empty arrays and empty hashrefs with a consistent
'null' being passed to ake it clear we don't have any settings to pass
yet.
Comment 13 Katrin Fischer 2022-09-07 09:28:53 UTC
*cough* test plan *cough* ;)
Comment 14 Katrin Fischer 2022-09-07 09:31:59 UTC
Created attachment 140265 [details] [review]
Bug 31435: Tie 'Configure' to table_settings

This patch adds a check for table_settings to prevent the display of
'Configure this table' when table settings are not passed to the
datatable wrapper.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Suggested test plan:
* Without patch:
* Go to Administration > SMTP servers
* Verify the configure button shows, but doesn't work
* Apply patch
* Reload the SMTP servers page
* The configure button will no longer show
Comment 15 Katrin Fischer 2022-09-07 09:32:03 UTC
Created attachment 140266 [details] [review]
Bug 31435: (follow-up) Make kohaTable calls consistent

Our calls to instantiate a kohaTable from JS were inconsistent in how
they pass table_settings where no table_settings exist.

This patch replaces empty arrays and empty hashrefs with a consistent
'null' being passed to ake it clear we don't have any settings to pass
yet.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 16 Joonas Kylmälä 2022-09-07 15:53:31 UTC
Created attachment 140318 [details] [review]
Bug 31435: Tie 'Configure' to table_settings

This patch adds a check for table_settings to prevent the display of
'Configure this table' when table settings are not passed to the
datatable wrapper.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Suggested test plan:
* Without patch:
* Go to Administration > SMTP servers
* Verify the configure button shows, but doesn't work
* Apply patch
* Reload the SMTP servers page
* The configure button will no longer show

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 17 Joonas Kylmälä 2022-09-07 15:53:35 UTC
Created attachment 140319 [details] [review]
Bug 31435: (follow-up) Make kohaTable calls consistent

Our calls to instantiate a kohaTable from JS were inconsistent in how
they pass table_settings where no table_settings exist.

This patch replaces empty arrays and empty hashrefs with a consistent
'null' being passed to ake it clear we don't have any settings to pass
yet.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 18 Joonas Kylmälä 2022-09-07 15:53:39 UTC
Created attachment 140320 [details] [review]
Bug 31435: (QA follow-up) Don't pass explicitly default parameter values

For kohaTable these are the default values.

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 19 Tomás Cohen Arazi 2022-09-07 20:21:47 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!