We should really make bug 22276 work globally rather than having a bug per table to add this feature.
Created attachment 149444 [details] [review] Bug 33484: Make saveState column handling global This patch updates the handling for saveState added in bug 22276 to work globally for all KohaTables that pass a table_settings hash.
Created attachment 149445 [details] [review] Bug 33484: Revert template change introduced in bug 22276 With the globalisation of the stateSave handling introduced by bug 22276 we can remove the localised changes to the template in branches.tt
To Test. 1) Confirm the test plan for bug 22276 still passes. 2) Check other instances where KohaTable is used and table_settings (i.e. you can choose what columns to display) is passed.. all should now save and restore state upon navigating back to them I'm not 100% sure about the original patch.. I feel it does more than expected in so much as saveState also saves and restores pagination, search filters etc etc. Secondly.. whilst I do this for the KohaTable (found in column_settings.inc).. I haven't added it to the DT REST wrapper.. or Vue components.. we should really do all three... but I wanted to start getting feedback first.
Created attachment 149447 [details] [review] Bug 33484: Make saveState column handling global This patch updates the handling for saveState added in bug 22276 to work globally for all KohaTables that pass a table_settings hash.
Created attachment 149448 [details] [review] Bug 33484: Revert template change introduced in bug 22276 With the globalisation of the stateSave handling introduced by bug 22276 we can remove the localised changes to the template in branches.tt
Created attachment 149449 [details] [review] Bug 33484: Revert template change on suggestion.tt This reverts commit 2b0693dbfd6aa2dd2c7c91428b23aba717ccc0af as it's no longer required once we globalise the logic as we do in this patchset
This works on some pages, but not on others. I have a meeting in a few minutes and so can't test them all at the moment, but this is what I've found so far: Pages that work: pendingreserves.pl suggestions.pl Pages that don't retain the save state: branches.pl members-home.pl (table id: memberresults)
Update: The tables that aren't retaining the save state are generated with the REST API wrapper from datatables.js, not the KohaTable wrapper from column_settings.inc. All the tables I've tested that use the KohaTable wrapper do retain the save state as intended. I think I do need to set this in its current state to Failed QA for causing a regression on branches.tt, though. As for the fact that saveState saves pagination and filters, there are certainly times when that's less desirable, but there are times when it is desirable too. Part of what spurred me to create bug 33426 was that our selectors want that behavior on the purchase suggestion table. Also, to be fair, the same could be said for saving the column configuration - there are definitely times when staff hide certain columns to make a page print more nicely, but don't necessarily need that state to persist. That being said...there are definitely some tables where saving the search filters is almost never going to be desirable. For example, on holdings_table in detail.pl, because the search filter on one bib will carry over to a different bib where that search filter may not even make sense. Not sure what the right answer is here... Go back to adding saveState only to certain tables individually, where it's actually needed? Add it globally but make it configurable (which would add some scope creep)?
Hi Emily, thanks for your comments! I hope I won't get myself into trouble for this... but could it be a flag we set on the column configuration page? But that would bring us back to having a different behaviour for column controlled vs. column uncontrolled tables...
Would it be possible to have it save as part of the saveState an option for keeping or clearing filters and have it clear filters by default, but someone could turn it on for a table if they wanted? Lisette
I've got a patch for the AJAX datatables now, and I'm working through making it all configurable through the table settings page. I'll need a hand working through the Vue table component, but I don't imagine it'll be too honerous. I'll throw the patches up as soon as I've got the configuration patch working.
Created attachment 149925 [details] [review] Bug 33484: Make saveState column handling global This patch updates the handling for saveState added in bug 22276 to work globally for all KohaTables that pass a table_settings hash.
Created attachment 149926 [details] [review] Bug 33484: Revert template change introduced in bug 22276 With the globalisation of the stateSave handling introduced by bug 22276 we can remove the localised changes to the template in branches.tt
Created attachment 149927 [details] [review] Bug 33484: Revert template change on suggestion.tt This reverts commit 2b0693dbfd6aa2dd2c7c91428b23aba717ccc0af as it's no longer required once we globalise the logic as we do in this patchset
Created attachment 149928 [details] [review] Bug 33484: Add support to kohaTable (AJAX Tables)
Created attachment 149929 [details] [review] Bug 33484: Add save state option to table settings
Created attachment 149930 [details] [review] Bug 33484: Schema Update
OK.. we now have this applying to KohaTable and kohaTable components (<KohaTable isn't yet covered). We also have the requested switches so one can enable/disable the filter state saving from the admin table settings page. Feel free to test and give feedback.
PS, you'll need to run the DB update ;P
None of the koha tables show anymore and the console is showing these errors/warnings: caught SyntaxError: Unexpected token '<<' jquery-3.6.0.min_22.1200016.js:2 jQuery.Deferred exception: $(...).kohaTable is not a function TypeError: $(...).kohaTable is not a function at HTMLDocument.<anonymous> (https://staff-save2.sandbox.bywatersolutions.com/cgi-bin/koha/admin/branches.pl:1632:45) at e (https://staff-save2.sandbox.bywatersolutions.com/intranet-tmpl/lib/jquery/jquery-3.6.0.min_22.1200016.js:2:30038) at t (https://staff-save2.sandbox.bywatersolutions.com/intranet-tmpl/lib/jquery/jquery-3.6.0.min_22.1200016.js:2:30340) undefined S.Deferred.exceptionHook @ jquery-3.6.0.min_22.1200016.js:2 jquery-3.6.0.min_22.1200016.js:2 Uncaught TypeError: $(...).kohaTable is not a function at HTMLDocument.<anonymous> (branches.pl:1632:45) at e (jquery-3.6.0.min_22.1200016.js:2:30038) at t (jquery-3.6.0.min_22.1200016.js:2:30340) Lisette
I can't replicate that I'm afraid.. I just freshly applied the bug and it's all working for me.
Created attachment 149974 [details] [review] Bug 33484: Make saveState column handling global This patch updates the handling for saveState added in bug 22276 to work globally for all KohaTables that pass a table_settings hash.
Created attachment 149975 [details] [review] Bug 33484: Revert template change introduced in bug 22276 With the globalisation of the stateSave handling introduced by bug 22276 we can remove the localised changes to the template in branches.tt
Created attachment 149976 [details] [review] Bug 33484: Revert template change on suggestion.tt This reverts commit 2b0693dbfd6aa2dd2c7c91428b23aba717ccc0af as it's no longer required once we globalise the logic as we do in this patchset
Created attachment 149977 [details] [review] Bug 33484: Add support to kohaTable (AJAX Tables)
Created attachment 149978 [details] [review] Bug 33484: Add save state option to table settings
Created attachment 149979 [details] [review] Bug 33484: Schema Update
Re-attached the patches, but nothings changed in them.. The message you report looks like a merge issue, but I can't see any such markers in the patches.
I can't reproduce the error on KTD either. Lisette, can you give more information about how to reproduce it? I am noticing so far a few other unexpected behaviors so far, though: - Holds to Pull saves the client-side state regardless of the configuration on the admin table settings page - Holds Queue appears to be working correctly for the most part. If you disable state saving from the admin table settings page, the table will continue to save the client-side state for the rest of the session, but will respect the configuration if you start a new session. (I'm not sure if that's an error or if it's the expected behavior.) - memberresultst and branches.pl (both of which use the kohaTable API wrapper rather than the KohaTable wrapper) appear to be working correctly for the most part. However, if the admin table settings hide a column by default, that column stays hidden even when state saving is enabled. Other aspects of client-side state saving (hiding columns that are shown by default, entries per page) work, though. Also, this was the case for branches.pl in master, too, so it wasn't introduced by this patch. I really like where this is going - but I think it's going to need more thorough/systematic testing than what I've done so far to catch any anomalies.
I'm no longer getting that error, but it's still not working. Likely a problem with my KTD. I'll try and sort that out before testing again.
I wrote up a test plan to keep myself organized while testing different tables and states...sharing in case someone else gets to it before me and would like to use it. (also open to feedback :) ) Considerations: - test a variety of tables - must include both tables that use KohaTable (column_settings.inc) and kohaTable (datatables.js) For reference: tables that use kohaTable (based on a git grep): - patron search (patrons_table) - acqui/parcel.tt (pending_orders) - admin/background_jobs.tt (table_jobs) - admin/branches.tt (libraries) - admin/cities.tt (table_cities) - admin/identity_provider_domains.tt (identity_provider_domains) - admin/identity_providers.tt (identity_providers) - admin/search_filters.tt (search_filters_table) - admin/smtp_servers.tt (smtp_servers) - catalogue/detail.tt (table_concerns) - catalogue/detail.tt (bundle_table) - cataloguing/concerns.tt (table_concerns) - pos/register.tt (table_cashups) - tools/quotes.tt (quotes) Test plan per table: 1. Edit the config to hide some columns and disable "save state" 2. Test that the "clean" page matches the config settings 3. Make some client-side changes a. Hide one or more showing columns b. Show one or more hidden columns c. Change number of results to display d. Add a search filter 4. Navigate away from that page and back 5. Confirm that the state was not saved and the page matches the config settings 6. Navigate away from that page 7. Edit the config to enable "save state" 8. Return to the page and confirm that it matches the config settings 9. Make some client-side changes a. Hide one or more showing columns b. Show one or more hidden columns c. Change number of results to display d. Add a search filter 10. Navigate away from the page and back 11. Confirm that all of the changes were retained
Taking another stab at testing this. I'm still getting a few tables (holds to pull and saved SQL reports, at least) that will continue to save the local configuration indefinitely if a local save state was ever created, even if it was later disabled in the configuration. I couldn't figure out why it happens with some tables and not others, but it did clear up when I tried moving the block that clears the local save state (if configured not to save) to occur before trying to load the local save state: Lines ~21-53 (columns_settings.inc): + if ( table_settings.hasOwnProperty('default_save_state') && table_settings['default_save_state'] === 0 ) { + localStorage.removeItem(table_key); + } + // enable stateSave dt_parameters['stateSave'] = true; dt_parameters['stateSaveCallback'] = function( settings, data ) { localStorage.setItem( table_key, JSON.stringify(data) ) } dt_parameters['stateLoadCallback'] = function(settings) { return JSON.parse( localStorage.getItem(table_key) ) } // merge stateSave for column settings var local_settings = localStorage.getItem(table_key); var system_settings = table_settings['columns']; var columns_settings = get_columns_saved_state(local_settings, system_settings); $(columns_settings).each( function() { var named_id = $( 'thead th[data-colname="' + this.columnname + '"]', selector ).index( selector + ' th' ); var used_id = dt_parameters.bKohaColumnsUseNames ? named_id : counter; if ( used_id == -1 ) return; if ( this['is_hidden'] == "1" ) { hidden_ids.push( used_id ); } if ( this['cannot_be_toggled'] == "0" ) { included_ids.push( used_id ); } counter++; }); - if ( table_settings.hasOwnProperty('default_save_state') && table_settings['default_save_state'] === 0 ) { - localStorage.removeItem(table_key); - }
*** Bug 35569 has been marked as a duplicate of this bug. ***
*** Bug 16881 has been marked as a duplicate of this bug. ***
Patches rebased, adjusted and improved. Everything is on a remote branch: I had a go, here is a summary of the situation: 1. We need 2 flags in the table settings: one to store the "configuration" (number of entries, column visibility, etc.) of the table, and one to store the previous search. With the new patch set we now have: tables_settings.default_save_state tables_settings.default_save_state_search The existing "stateSave" occurrences shown that: * Bug 33426 ("Add client storage of user-selected DataTables configuration to suggestion.tt") => We want to keep everything (search+config) for the suggestions table * Bug 22276 ("Add client storage of user-selected DataTables configuration") => The aim of this patch was the premise of this 33484 one. We only wanted to make the selection of the column persistent. On the library list view. * On the pending and received orders page (not sure since when we have them, at least bug 20212) => We want to keep everything 2 problems here, the first one is that the the pending orders table (#pending_orders) didn't have the table settings (now done with patch "Add table settings to pending order"). The other is that the orders received table (#receivedt) does not use KohaTable or kohaTable, but DataTables directly. As there is no column visibility or filters stateSave is enable directly on this table. We should not do that anymore, this is an exception! * Bug 21852 ("Add more columns and column configuration to overdues report") We want to keep everything Those 3 tables that want "everything" to be kept have the default_save_state_search flag set to 1 in the yml config file. 2. Code is copy-pasted between columns_settings.inc and datatables.js, but at least it does not appear a 3rd time for the KohaTable vue component. Good enough for now IMO. 3. In order to fix bug 27467 with this change the last patches actually save the state all the time. So that we can copy the link for sharing purpose. 4. The state is not restored correctly when the share link is used. On complex table (patron search for instance) there is a shift with the table headers, and it get worst when columns are hidden. Not sure it is fixable. Yes the patches will be moved out of this bug report it is too hard to deal with. 5. We may want to have a syspref to turn off the whole stateSave feature (in case of performance issue? Or privacy?) Please test and provide early feedback.
Pushed new patches, ready for testing. We have different tables to test: * kohaTable (datatables.js) ** with filters - catalogue detail, holding tables - main patron search - item search (I didn't test this one) - cities ** without filters - acqui/edifactmsgs (didn't test, not familiar with edifact) - admin/branches.pl * KohaTable (columns_settings.inc) ** with filters - only reports/itemslost.tt ** without filters - catalogue detail page, tab "Comments" ** with search form - Overdue report (might need adjustments) - others? * Vue tables (using kohaTable but differ a bit) The "Copy conditions" buttons should only appear if table settings are defined (ie. there is an entry in the columns_settings.yml file). For each table you can try to: * Play with the two new options from the settings: - "Save configuration state on page change": will save the column visibility, length and order (ON by default) - "Save search state on page change": will save the search (OFF by default) * See if the state is correctly restored and not, depending on the settings. * Use the "Copy conditions" button and reuse it to confirm that is restore the table properly. Note that it will ofc erase the localStorage entry with this state.
If none of the above makes sense and you don't understand what I am talking about, please let me know. If you are interested in testing this patch but I need more details, please let me know. If you are interested in this feature, please test ASAP, it is not a trivial one, and the rebases might be tricky.
*** Bug 27467 has been marked as a duplicate of this bug. ***
Maybe we could make the bug description a bit less technical to draw more attention. Maybe something like "Remember user's table settings for all tables"?
Going further: * "clear state" button * "Do not keep state for this page" - we could store in localstorage => any librarians can decide what to do.
I'm interested in testing this patch, but I have'nt ever tested a patch from a remote branch before, so I need to figure out how to do that first. I'm also curious would this fix an issue with certain tables, where if a column is hidden by default in table settings, it still briefly flashes when the table first loads before it is hidden? Specifically this happens in the 'checkedintable'
(In reply to Brendan Lawlor from comment #41) > I'm interested in testing this patch, but I have'nt ever tested a patch from > a remote branch before, so I need to figure out how to do that first. % git remote add joubu https://gitlab.com/joubu/koha.git % git fetch joubu bug_33484 > I'm also curious would this fix an issue with certain tables, where if a > column is hidden by default in table settings, it still briefly flashes when > the table first loads before it is hidden? Specifically this happens in the > 'checkedintable' Nope, but I can have a look at this issue later.
(In reply to Jonathan Druart from comment #42) > (In reply to Brendan Lawlor from comment #41) > > I'm also curious would this fix an issue with certain tables, where if a > > column is hidden by default in table settings, it still briefly flashes when > > the table first loads before it is hidden? Specifically this happens in the > > 'checkedintable' > > Nope, but I can have a look at this issue later. The flash actually is not there if the state is loaded from the URL or localStorage. So yes it's kind of fixed.
Thank you Joubu for this very important work. My 2 cents: 1) QA script has several FAIL 2) t/db_dependent/TablesSettings.t syntax error -> is missing a comma on line 196. Tests fail after correcting that. 3) atomicupdate does not follow skeleton.pl Output 4) Testing suggestions table at /cgi-bin/koha/suggestion/suggestion.pl: - Created 2 suggestions with random title each - Did a search keyword for 'staff' - Clicked 'Copy conditions' - Opened new tab, pasted -> BOOM Error 500 with the following error: DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'DataTables_acqui_suggestions_suggestions_state' in 'where clause' at /kohadevbox/koha/Koha/Objects.pm line 403 at /usr/share/perl5/DBIx/Class/Exception.pm line 77 Non-blockers / too hard for now: 1) "Copy conditions" button does not consider left-side filters on the patron search (and I suspect the same for ILL requests even though I have not tested, it works pretty much the same way i.e. left-side filters live outside of the datatable). 2) "Copy conditions" button does not exist on the items search table but that's expected as that table is very specific, and has this functionality specifically built for it. Obs: - Visiting a different page and returning to the suggestions table has kept the 'staff' keyword state, respecting the default_save_state_search, I'm assuming. - Visiting a different page and returning to the cities table has not kept the keyword state, respecting the default_save_state_search of false. Changing this value and testing again works as expected, i.e. it keeps the keyword state. - ERM Agreements table seems to be working as intended. - I suggest a new label for "Copy conditions". The item search button that does the same thing has "Copy shareable link" which I think it's much better/obvious to what it does. - "Save configuration state on page change" is this about columns configuration display only? If so, I suggest renaming this label to "Keep columns visibility state on page change". - Likewise "Save search state on page change:" I would suggest "Keep search state on page change" instead. We're "keeping" the state between page hops, no actual "saving" is happening.
(In reply to Pedro Amorim from comment #44) > Thank you Joubu for this very important work. My 2 cents: > > 1) QA script has several FAIL Fixed. > 2) t/db_dependent/TablesSettings.t syntax error -> is missing a comma on > line 196. Tests fail after correcting that. Fixed. > 3) atomicupdate does not follow skeleton.pl Output Please clarify, I am using the usual `say $out`, what is in skeleton. There is no clear definition for the uses of the other success/failure. The output for 24.11 is not consistent at all for the moment, that's a discussion for somewhere else. > 4) Testing suggestions table at /cgi-bin/koha/suggestion/suggestion.pl: > - Created 2 suggestions with random title each > - Did a search keyword for 'staff' > - Clicked 'Copy conditions' > - Opened new tab, pasted -> BOOM Error 500 with the following error: > DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st > execute failed: Unknown column > 'DataTables_acqui_suggestions_suggestions_state' in 'where clause' at > /kohadevbox/koha/Koha/Objects.pm line 403 > at /usr/share/perl5/DBIx/Class/Exception.pm line 77 Those ugly scripts, erk. Fixed. ugly fix ofc... There is more to do to make it work on this table however. > Non-blockers / too hard for now: > 1) "Copy conditions" button does not consider left-side filters on the > patron search (and I suspect the same for ILL requests even though I have > not tested, it works pretty much the same way i.e. left-side filters live > outside of the datatable). I will write something to exclude some tables from the first iteration. Especially the ones with "additional filters" (ie. when some filters are outside of the table). > - I suggest a new label for "Copy conditions". The item search button that > does the same thing has "Copy shareable link" which I think it's much > better/obvious to what it does. Correct, done. I've also added a tooltip "Copied!" when the button is clicked (like on the item search). > - "Save configuration state on page change" is this about columns > configuration display only? If so, I suggest renaming this label to "Keep > columns visibility state on page change". There is a tooltip, it's the column visibility, number of results, sort order. > - Likewise "Save search state on page change:" I would suggest "Keep search > state on page change" instead. We're "keeping" the state between page hops, > no actual "saving" is happening. It's actually saving in the localStorage. But you are correct, for the end user it's more about keeping/restoring. I will adjust when we agree on a wording for the 2 entries. Changes from today: git range-diff joubu/old/20241017-bug_33484~27..joubu/old/20241017-bug_33484 joubu/bug_33484~32..joubu/bug_33484
I have pushed some patches to restore properly the external filters on the patron searches (and catalogue concerns). Also fixed the suggestions page, however the external filters are not taken into account (but they are in current main and nobody complains).
I have pushed some patches to restore properly the external filters on the patron searches (and catalogue concerns). Also fixed the suggestions page, however the external filters are not taken into account (but they are not in current main and nobody complains).
Obs (other bugs, not here, if you agree): 1) Libraries search does not search on address1 (or address2/address3) despite it being shown. 2) ILL requests table needs to adopt external_filter_nodes to have external filters included in the "Copy shareable link". 3) qa script complains about console.logs in datatables.js and columns_settings.inc but I don't consider these blockers Blocker: t/db_dependent/selenium/patrons_search.t .. # Failed test 'Searching in standard brings back correct results' # at t/db_dependent/selenium/patrons_search.t line 285. # got: 'No entries to show (filtered from 80 total entries)' # expected: 'Showing 1 to 15 of 26 entries (filtered from 80 total entries)' # Failed test 'Filtering on library works in combination with main search' # at t/db_dependent/selenium/patrons_search.t line 289. # got: 'No entries to show (filtered from 80 total entries)' # expected: 'Showing 1 to 15 of 25 entries (filtered from 80 total entries)' # Looks like you planned 7 tests but ran 1. # Failed test 'remember_search' # at t/db_dependent/selenium/patrons_search.t line 407. # Looks like you planned 28 tests but ran 27. # Looks like you failed 3 tests of 27 run.
(In reply to Pedro Amorim from comment #48) > Obs (other bugs, not here, if you agree): > 1) Libraries search does not search on address1 (or address2/address3) > despite it being shown. Yes, it deserves its own bug. > 2) ILL requests table needs to adopt external_filter_nodes to have external > filters included in the "Copy shareable link". Should be done here. Almost done, there is only the value for "Backend" and "Status" dropdown lists that are not restored. It seems that both are fetched in AJAX and I guess there is a race between DT that is loading its state (and selecting the options) and those requests. I've tried to move the code (populateStatusFilter) before the kohaTable init but it still does not work. Could you have look maybe? See commits Bug 33484: External filter into state - support flatpickr Bug 33484: External filter into state - ill requests > 3) qa script complains about console.logs in datatables.js and > columns_settings.inc but I don't consider these blockers console.log("Tag '%s' not supported yet for DT state".format(tag_name)) They are there on purpose. False positives. > Blocker: > t/db_dependent/selenium/patrons_search.t .. > # Failed test 'Searching in standard brings back correct results' > # at t/db_dependent/selenium/patrons_search.t line 285. > # got: 'No entries to show (filtered from 80 total entries)' > # expected: 'Showing 1 to 15 of 26 entries (filtered from 80 total > entries)' > > # Failed test 'Filtering on library works in combination with main > search' > # at t/db_dependent/selenium/patrons_search.t line 289. > # got: 'No entries to show (filtered from 80 total entries)' > # expected: 'Showing 1 to 15 of 25 entries (filtered from 80 total > entries)' > # Looks like you planned 7 tests but ran 1. > > # Failed test 'remember_search' > # at t/db_dependent/selenium/patrons_search.t line 407. > # Looks like you planned 28 tests but ran 27. > # Looks like you failed 3 tests of 27 run. Fixed! Fixed!
(In reply to Jonathan Druart from comment #49) > Should be done here. > Almost done, there is only the value for "Backend" and "Status" dropdown > lists that are not restored. > It seems that both are fetched in AJAX and I guess there is a race between > DT that is loading its state (and selecting the options) and those requests. > I've tried to move the code (populateStatusFilter) before the kohaTable init > but it still does not work. Could you have look maybe? I've done some testing on the ILL table and none of the left filters are copied by the "Copy shareable link" button. I was expecting backend+status to not work but none are working, i.e. none get copied over. The built-in tiny search input on the table works as expected when copying the shareable link. I'm happy to follow-up with work on this for the ILL table specifically but if that's the case I'd suggest moving that to a different bug and not have it block this one. Up to you. Additional info: I'm running the latest commit from the branch: ae15be2d3f63d0ea3cf0eaeadc50c35d83b1f740 With bug 35570 applied on top for faster ILL testing.
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
(In reply to Pedro Amorim from comment #51) > Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Added to the remote branch, thanks!
(In reply to Jonathan Druart from comment #52) > (In reply to Pedro Amorim from comment #51) > > Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> > > Added to the remote branch, thanks! 4f8d1b5ca00 (HEAD -> bug_33484, gitlab/bug_33484) Bug 33484: (follow-up) Fix category select f2b268b978f Bug 33484: Fix selenium/patrons_search.t dac59ead863 Bug 33484: External filter into state - ill requests 2814b1e998a Bug 33484: External filter into state - support flatpickr 5ae78fe19c6 Bug 33484: External filter into state - catalogue concerns a40fb3ab004 Bug 33484: External filter into state - support checkboxes 995e10fdba9 Bug 33484: External filter into state - patron search 0464ba6a917 Bug 33484: Store external filters into state ef16a39424f Bug 33484: Fix for suggestions cfac4b84e55 Bug 33484: Allow several tables using the same table settings - columns_settings 0121c20b395 Bug 33484: Add 'Copied!' tooltip when the button is clicked 9897e254a2c Bug 33484: Rename 'Copy conditions' => 'Copy shareable link' 7a02ce991a0 Bug 33484: Fix TablesSettings.t 14e71798f7f Bug 33484: Add new columns as boolean in schema file c83226b3010 Bug 33484: Fix libraries table 9b5d3c6557e Bug 33484: Fix bookings 3b3db7f4596 Bug 33484: Fix Vue tables 24f85535e5b Bug 33484: Fix pending orders fe1646bec37 Bug 33484: Adjust columns_settings.inc 50c8ba82597 Bug 33484: Early return in add_filters if node does not exist 62a9da0552e Bug 33484: Do not defer loading if state is loaded from localStorage 5bea1604a6c Bug 33484: Fix category select f60f5b68766 Bug 33484: Fix patron search form f6fded1d4d2 Bug 33484: Fix patrons search d2045474ff9 Bug 33484: Redraw columns filters when column visibility is modified 2620e01756b Bug 33484: Remove localStorage on logout 5c6a1546531 Bug 33484: (Bug 27467) Fix patrons search 0a69f25bc64 Bug 33484: (Bug 27467) Ability to share a link 7f20a93f27a Bug 33484: Always save but restore on demand only 3271b62ca23 Bug 33484: Implement Vue KohaTable 79ade6e3eb4 Bug 33484: DBIC Schema change f221690d365 Bug 33484: Add tables_settings.default_save_state_search 82cdcee4473 Bug 33484: Clearly separate search and configuration 46332707da2 Bug 33484: Add table settings to pending orders 3f86d437d05 Bug 33484: Make default_save_state default to 1 a124435e8de Bug 33484: Add save state option to table settings a45695301e0 Bug 33484: Schema Update b5394d9ab01 Bug 33484: Add support to kohaTable (AJAX Tables) 70204f43c3c Bug 33484: Revert template change on suggestion.tt 71d39685d8d Bug 33484: Revert template change introduced in bug 22276 87d9ecc5e52 Bug 33484: Make saveState column handling global Also has: 2651447aa58 Bug 38240: Do not hide columns if no items to display d87b4bb2c12 Bug 38192: Fix restoration of table state for suggestion tables 597000dbb10 Bug 38191: Restore collapse/expand of filters on the suggestions page 6d48ab25b24 Bug 37395: Fix columns visibility on Vue tables
New commit "Bug 33484: Pass external filters only if exists"
New commit "Bug 33484: Include patron search code if no patron selected yet"
New commit "Bug 33484: Do not consider we loaded from state if search was not saved"
I've just spend a chunk of time working through this playing with various tables.. I'm happy to throw my QA stamp on.. going to make the bug PQA and let Jonathan add the SO lines to his branch ready for Katrin to merge. Great work here, thanks for jumping on and getting it applied everywhere Jonathan!
Really happy to see this one moving into my queue - waiting for Joubu to add sign-off lines and give the final "Go". Moving to BLOCKED meanwhile.
Created attachment 173443 [details] [review] Bug 33484: Make saveState column handling global This patch updates the handling for saveState added in bug 22276 to work globally for all KohaTables that pass a table_settings hash. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173444 [details] [review] Bug 33484: Revert template change introduced in bug 22276 With the globalisation of the stateSave handling introduced by bug 22276 we can remove the localised changes to the template in branches.tt Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173445 [details] [review] Bug 33484: Revert template change on suggestion.tt This reverts commit 2b0693dbfd6aa2dd2c7c91428b23aba717ccc0af as it's no longer required once we globalise the logic as we do in this patchset Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173446 [details] [review] Bug 33484: Add support to kohaTable (AJAX Tables) Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173447 [details] [review] Bug 33484: Schema Update Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173448 [details] [review] Bug 33484: Add save state option to table settings Amended-by: Jonathan Druart Drop changes to admin/columns_settings.yml to avoid conflict. Just assume that 1 is the default (see next patch) Add +x to atomicupdate Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173449 [details] [review] Bug 33484: Make default_save_state default to 1 Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173450 [details] [review] Bug 33484: Add table settings to pending orders Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173451 [details] [review] Bug 33484: Clearly separate search and configuration We want to make distinguish the search terms and the configuration/option of the table (number of entries, column visibility, etc.) Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173452 [details] [review] Bug 33484: Add tables_settings.default_save_state_search Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173453 [details] [review] Bug 33484: DBIC Schema change Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173454 [details] [review] Bug 33484: Implement Vue KohaTable Move up as beforeMount was called too late to init DT's options Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173455 [details] [review] Bug 33484: Always save but restore on demand only And remove get_columns_saved_state. I am not sure this what really helpful, maybe in case the yml or config in the DB changed, but I don't think we should complicate the code. If something is wrong, logout to clear localStorage... We will see later if we really need it. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173456 [details] [review] Bug 33484: (Bug 27467) Ability to share a link Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173457 [details] [review] Bug 33484: (Bug 27467) Fix patrons search Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173458 [details] [review] Bug 33484: Remove localStorage on logout Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173459 [details] [review] Bug 33484: Redraw columns filters when column visibility is modified This seems to be the most sensible approach, we redraw the whole th when the column visibility is modified. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173460 [details] [review] Bug 33484: Fix patrons search Setting a DT's custom setting to know if we loaded an existing state, so that we do not pick the value of the selects from the form (on the left). Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173461 [details] [review] Bug 33484: Fix patron search form Reset DT's filters if we are searching again using the search form. If you use the form on the left we need to reset the different filters that could have been set from a previous state we loaded. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173462 [details] [review] Bug 33484: Fix category select Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173463 [details] [review] Bug 33484: Do not defer loading if state is loaded from localStorage Squashing Revert "Bug 33484: (Bug 27467) Fix patrons search" This reverts commit 0d21ff768b97bc8393a01b0ad881961a3252f2cf. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173464 [details] [review] Bug 33484: Early return in add_filters if node does not exist Maybe we could even return early from kohaTable? Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173465 [details] [review] Bug 33484: Adjust columns_settings.inc Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173466 [details] [review] Bug 33484: Fix pending orders Sometimes 'this' is the DOM element, for instance on the pending orders table. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173467 [details] [review] Bug 33484: Fix Vue tables datatables.net-vue3 does not use a specific id, and does not let us pass one. It is generated, like: DataTables_Table_0 (and increment when a new table is created). So it's not possible to use this id: it can change and so is not reliable. I have tried to pass "data-koha-table-id" with a specific id (KohaTable_agreements). Which was working, but actually we could simply use the table settings key to identify the tables in the URL. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173468 [details] [review] Bug 33484: Fix bookings Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173469 [details] [review] Bug 33484: Fix libraries table We added columns to the table but never displayed them, and they were not part of the columns settings. I am not sure what was the purpose of that but it looks completely wrong. If we want to restore them it should be done correctly (ie. add them back to the column list and add the new ones to the columns settings). Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173470 [details] [review] Bug 33484: Add new columns as boolean in schema file Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173471 [details] [review] Bug 33484: Fix TablesSettings.t Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173472 [details] [review] Bug 33484: Rename 'Copy conditions' => 'Copy shareable link' Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173473 [details] [review] Bug 33484: Add 'Copied!' tooltip when the button is clicked Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173474 [details] [review] Bug 33484: Allow several tables using the same table settings - columns_settings In some cases (at least on the suggestions page) we display several pages using the same settings. In that case the key "DataTables_acqui_suggestions_suggestions" is not specific enough, we need to add the id of the table as well. This patch does not modify koha-tmpl/intranet-tmpl/prog/js/datatables.js / REST API wrapper as I don't think we have occurrences that need this. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173475 [details] [review] Bug 33484: Fix for suggestions This ugly fix is retrieving all CGI params and consider them attributes for suggestions. So we need to exclude other ones... There is more to make it works properly on this table. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173476 [details] [review] Bug 33484: Store external filters into state Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173477 [details] [review] Bug 33484: External filter into state - patron search Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173478 [details] [review] Bug 33484: External filter into state - support checkboxes Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173479 [details] [review] Bug 33484: External filter into state - catalogue concerns Use a checkbox to store the state. It does not make the code better but make possible to keep and restore and state of the filter. Not sure it's working correct on the cataloguing/concerns.pl page but the behaviour was already not good before. At least I think it fixes the filter/include icon in some situation. On main I managed to get the "include resolved" link with the "filter" icon and the resolved concerns were already displayed... Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173480 [details] [review] Bug 33484: External filter into state - support flatpickr Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173481 [details] [review] Bug 33484: External filter into state - ill requests Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173482 [details] [review] Bug 33484: Fix selenium/patrons_search.t Filters are saved, we need to clear the form when needed. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173483 [details] [review] Bug 33484: (follow-up) Fix category select Lower everything or broken for library Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173484 [details] [review] Bug 33484: Pass external filters only if exists The external_filter_nodes object should only be passed if the filters block on the left actually exists. So only for the main patron search, not the other ones. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173485 [details] [review] Bug 33484: Include patron search code if no patron selected yet When placing a hold we should not display the patron search code (html+js) if we have picked a patron already, otherwise a get a JS error Uncaught TypeError: table_dt.settings()[0] is undefined koha-tmpl/intranet-tmpl/prog/en/includes/patron-search.inc 781 let table_dt = patrons_table.DataTable(); 782 let loaded_from_state = table_dt.settings()[0].loaded_from_state; Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173486 [details] [review] Bug 33484: Do not consider we loaded from state if search was not saved The loaded_from_state flag is used to know if we defer the loading of the table or not. We should not display the result (ie not defer loading) if only the configuration has been retrieved from localStorage. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Are we ready to push?
On this now.
OK, doing my best here, so some notes :) 1) Copy shareable link This is a tad long. Could we go with "Link" or "Copy link" and a longer tooltip like on the other buttons? 2) Links are really really long A shareable link for "Borrowers that start with D" is: http://localhost:8081/cgi-bin/koha/members/members-home.pl?firstletter=D&DataTables_members_member_memberresultst_state=eyJ0aW1lIjoxNzMwMTI1Mzk2Mzc5LCJzdGFydCI6MCwibGVuZ3RoIjoyMCwib3JkZXIiOltbMiwiYXNjIl1dLCJzZWFyY2giOnsic2VhcmNoIjoiIiwic21hcnQiOnRydWUsInJlZ2V4IjpmYWxzZSwiY2FzZUluc2Vuc2l0aXZlIjp0cnVlfSwiY29sdW1ucyI6W3sidmlzaWJsZSI6dHJ1ZSwic2VhcmNoIjp7InNlYXJjaCI6IiIsInNtYXJ0Ijp0cnVlLCJyZWdleCI6ZmFsc2UsImNhc2VJbnNlbnNpdGl2ZSI6dHJ1ZX19LHsidmlzaWJsZSI6dHJ1ZSwic2VhcmNoIjp7InNlYXJjaCI6IiIsInNtYXJ0Ijp0cnVlLCJyZWdleCI6ZmFsc2UsImNhc2VJbnNlbnNpdGl2ZSI6dHJ1ZX19LHsidmlzaWJsZSI6dHJ1ZSwic2VhcmNoIjp7InNlYXJjaCI6IiIsInNtYXJ0Ijp0cnVlLCJyZWdleCI6ZmFsc2UsImNhc2VJbnNlbnNpdGl2ZSI6dHJ1ZX19LHsidmlzaWJsZSI6ZmFsc2UsInNlYXJjaCI6eyJzZWFyY2giOiIiLCJzbWFydCI6dHJ1ZSwicmVnZXgiOmZhbHNlLCJjYXNlSW5zZW5zaXRpdmUiOnRydWV9fSx7InZpc2libGUiOnRydWUsInNlYXJjaCI6eyJzZWFyY2giOiIiLCJzbWFydCI6dHJ1ZSwicmVnZXgiOmZhbHNlLCJjYXNlSW5zZW5zaXRpdmUiOnRydWV9fSx7InZpc2libGUiOnRydWUsInNlYXJjaCI6eyJzZWFyY2giOiIiLCJzbWFydCI6dHJ1ZSwicmVnZXgiOmZhbHNlLCJjYXNlSW5zZW5zaXRpdmUiOnRydWV9fSx7InZpc2libGUiOnRydWUsInNlYXJjaCI6eyJzZWFyY2giOiIiLCJzbWFydCI6dHJ1ZSwicmVnZXgiOmZhbHNlLCJjYXNlSW5zZW5zaXRpdmUiOnRydWV9fSx7InZpc2libGUiOnRydWUsInNlYXJjaCI6eyJzZWFyY2giOiIiLCJzbWFydCI6dHJ1ZSwicmVnZXgiOmZhbHNlLCJjYXNlSW5zZW5zaXRpdmUiOnRydWV9fSx7InZpc2libGUiOnRydWUsInNlYXJjaCI6eyJzZWFyY2giOiIiLCJzbWFydCI6dHJ1ZSwicmVnZXgiOmZhbHNlLCJjYXNlSW5zZW5zaXRpdmUiOnRydWV9fSx7InZpc2libGUiOnRydWUsInNlYXJjaCI6eyJzZWFyY2giOiIiLCJzbWFydCI6dHJ1ZSwicmVnZXgiOmZhbHNlLCJjYXNlSW5zZW5zaXRpdmUiOnRydWV9fSx7InZpc2libGUiOnRydWUsInNlYXJjaCI6eyJzZWFyY2giOiIiLCJzbWFydCI6dHJ1ZSwicmVnZXgiOmZhbHNlLCJjYXNlSW5zZW5zaXRpdmUiOnRydWV9fSx7InZpc2libGUiOmZhbHNlLCJzZWFyY2giOnsic2VhcmNoIjoiIiwic21hcnQiOnRydWUsInJlZ2V4IjpmYWxzZSwiY2FzZUluc2Vuc2l0aXZlIjp0cnVlfX0seyJ2aXNpYmxlIjpmYWxzZSwic2VhcmNoIjp7InNlYXJjaCI6IiIsInNtYXJ0Ijp0cnVlLCJyZWdleCI6ZmFsc2UsImNhc2VJbnNlbnNpdGl2ZSI6dHJ1ZX19LHsidmlzaWJsZSI6dHJ1ZSwic2VhcmNoIjp7InNlYXJjaCI6IiIsInNtYXJ0Ijp0cnVlLCJyZWdleCI6ZmFsc2UsImNhc2VJbnNlbnNpdGl2ZSI6dHJ1ZX19XSwiZXh0ZXJuYWxfZmlsdGVycyI6eyJzZWFyY2hfcGF0cm9uX2ZpbHRlciI6IiIsImZpcnN0bGV0dGVyX2ZpbHRlciI6IkQiLCJzZWFyY2h0eXBlX2ZpbHRlciI6InN0YXJ0c193aXRoIiwic2VhcmNoZmllbGRzdHlwZV9maWx0ZXIiOiJzdGFuZGFyZCIsImNhdGVnb3J5Y29kZV9maWx0ZXIiOiIiLCJicmFuY2hjb2RlX2ZpbHRlciI6IiJ9LCJjaGlsZFJvd3MiOltdfQ%3D%3D Is there a trick we could imagine (maybe as a future improvement) to create a shorter shareable link? 3) I got trapped :( (please follow-up) For testing purposes I set the patron search table to save the search state. I entered "ace" in the name column, which returns one result. No every time I got o patron search it immediately sends me to old Henry's account and I have no way to get out of that loop and change my settings. Logging out and in again fixes the issue of course. As I believe that this is an edge case, I won't make it blocker, but we should try and think about how to handle this better or maybe we need to remove the option from tables that auto-redirect you. Notes (no action required) A) Love that for pending orders "save search state" is pre-checked.
Forgot one (only cosmetics again): 4) Phrasing Save configuration state on page change: Save search state on page change: I wonder if "state" is easy to understand here for the local admins and translators. A suggestion (to be discussed): Save table settings on page change: Save selected search filters and options on page change:
Created attachment 173546 [details] [review] Bug 33484: Do not redirect if loaded from state
(In reply to Katrin Fischer from comment #105) > OK, doing my best here, so some notes :) > > 1) Copy shareable link > > This is a tad long. Could we go with "Link" or "Copy link" and a longer > tooltip like on the other buttons? It's consistent with the "item search" feature. We can adjust but should we then adjust the existing one as well? > 2) Links are really really long > > A shareable link for "Borrowers that start with D" is: > Is there a trick we could imagine (maybe as a future improvement) to create > a shorter shareable link? Possibly. It's base64 encoded so not supposed to reduce the size of the original string. > 3) I got trapped :( (please follow-up) > > For testing purposes I set the patron search table to save the search state. > I entered "ace" in the name column, which returns one result. > No every time I got o patron search it immediately sends me to old Henry's > account and I have no way to get out of that loop and change my settings. Fixed, see "Bug 33484: Do not redirect if loaded from state"
Pushed for 24.11! Well done everyone, thank you!
It looks like this may have broken regresions.t: # Failed test 'Checkouts table must be correctly aligned' # at t/db_dependent/selenium/regressions.t line 243. # got: '' # expected: '1' # thead: 18 ; tfoot: 17 ; tbody: 18 # Looks like you failed 1 test of 1. not ok 6 - Display circulation table correctly Please check!
Created attachment 173606 [details] [review] Bug 33484: Remove state settings from column level default_save_state and default_save_state_search were added at the column level, it's not needed.
Created attachment 173607 [details] [review] Bug 33484: checkouts - Move visibility handling to table_settings We need to set is_hidden for the column we want to force hide (depending on system config) This patch fixes t/db_dependent/selenium/regressions.t Obviously there will be incorrect behaviours if we restore from a state and that the settings have been modified in the meanwhile. Not sure we can deal with that properly.
Created attachment 173985 [details] [review] Bug 33484: (follow-up) checkouts - Move visibility handling to table_setting table_settings_issues_table must be defined before we include checkouts.js
(In reply to Jonathan Druart from comment #113) > Created attachment 173985 [details] [review] [review] > Bug 33484: (follow-up) checkouts - Move visibility handling to table_setting > > table_settings_issues_table must be defined before we include > checkouts.js It fixes "Uncaught ReferenceError: table_settings_issues_table is not defined"
Pushed (follow-up) to main. Is it possible that columns hidden by default and set to visible are not remembered when moving between pages? (they re-hide)
(In reply to Ewa Gózd from comment #115) > Is it possible that columns hidden by default and set to visible are not > remembered when moving between pages? (they re-hide) Which ones exactly? It is possible that renew, checkin and export are hidden even if shown before, because their visibility depends on sysprefs (AllowCirculate, ClaimReturnedLostValue). But it should work correctly for other tables.
(In reply to Jonathan Druart from comment #116) > (In reply to Ewa Gózd from comment #115) > > Is it possible that columns hidden by default and set to visible are not > > remembered when moving between pages? (they re-hide) > > Which ones exactly? It is possible that renew, checkin and export are hidden > even if shown before, because their visibility depends on sysprefs > (AllowCirculate, ClaimReturnedLostValue). > > But it should work correctly for other tables. Agreed, thanks!
Created attachment 174219 [details] [review] Bug 33484: Redraw columns filters when column visibility is modified - Vue
(In reply to Jonathan Druart from comment #118) > Created attachment 174219 [details] [review] [review] > Bug 33484: Redraw columns filters when column visibility is modified - Vue Picked to push to main.
New feature, no 24.05.x backport