|
Description
Jonathan Druart
2019-11-07 16:39:07 UTC
Created attachment 95172 [details] [review] Bug 23991: Move SearchSuggestion to Koha::Suggestions The hardest part is missing, suggestion/suggestion.pl I will come back to this one later. Created attachment 115203 [details] [review] Bug 23991: Move SearchSuggestion to Koha::Suggestions Created attachment 115207 [details]
Script for making some suggestions for testing
For making suggestions in various statuses in several branches
Updating suggestions, applying filters all seem to work as expected - the only thing I note is that if I have selected a library on the dropdown above the table and then clear filters on the left I am defaulted back to my logged in library Also, on the homepage the count of pending suggestions does not exclude archived suggestions Two more things, not necessarily caused by this: 1 - If I add a new suggestion I am returned to the suggestions for that library, but I will be on the first tab (accepted for example), not the pending tab 2 - if I have multiple suggestions in unknown statuses I get multiple tabs when organizing by status, but they only say 'unknown' I can tell what the actual status is - the tabe shoudl include the unknown code (In reply to Nick Clemens from comment #5) > Updating suggestions, applying filters all seem to work as expected - the > only thing I note is that if I have selected a library on the dropdown above > the table and then clear filters on the left I am defaulted back to my > logged in library That's the behaviour in master. > Also, on the homepage the count of pending suggestions does not exclude > archived suggestions Same, opened bug 27462. Maybe we should simplify this view and have it using the REST API (and the datatables wrapper). With correct column filtering we shouldn't need the different "organized by" views. What do you think? Created attachment 116639 [details] [review] Bug 23991: Move SearchSuggestion to Koha::Suggestions The C4::Suggestions::SearchSuggestion subroutine is badly written and can be replaced by calls to Koha::Suggestions->search. The hard part in this patch is suggestion.pl, the other occurrences have been replaced easily. Test plan: The idea is to test the whole suggestion workflow. 1. Create a suggestion on OPAC 2. Create a suggestion on the staff interface 3. Edit suggestions 4. Filter suggestions (use the different filters and "organize by" values) This is ready for testing :) Created attachment 118454 [details] [review] Bug 23991: Move SearchSuggestion to Koha::Suggestions The C4::Suggestions::SearchSuggestion subroutine is badly written and can be replaced by calls to Koha::Suggestions->search. The hard part in this patch is suggestion.pl, the other occurrences have been replaced easily. Test plan: The idea is to test the whole suggestion workflow. 1. Create a suggestion on OPAC 2. Create a suggestion on the staff interface 3. Edit suggestions 4. Filter suggestions (use the different filters and "organize by" values) Conflicts with bug 26406 fixed. Suggestions are hidden on the opac side, on the 'cgi-bin/koha/opac-suggestions.pl' page. Checked on master without the patch, they are correctly displayed, as well on the intranet side on '/cgi-bin/koha/members/purchase-suggestions.pl' with the patch. Created attachment 119256 [details] [review] Bug 23991: Move SearchSuggestion to Koha::Suggestions The C4::Suggestions::SearchSuggestion subroutine is badly written and can be replaced by calls to Koha::Suggestions->search. The hard part in this patch is suggestion.pl, the other occurrences have been replaced easily. Test plan: The idea is to test the whole suggestion workflow. 1. Create a suggestion on OPAC 2. Create a suggestion on the staff interface 3. Edit suggestions 4. Filter suggestions (use the different filters and "organize by" values) Thanks Séverine, indeed there were a couple of mistakes in the patch. It should work correctly now. This has been forgotten! Created attachment 127058 [details] [review] Bug 23991: Move SearchSuggestion to Koha::Suggestions The C4::Suggestions::SearchSuggestion subroutine is badly written and can be replaced by calls to Koha::Suggestions->search. The hard part in this patch is suggestion.pl, the other occurrences have been replaced easily. Test plan: The idea is to test the whole suggestion workflow. 1. Create a suggestion on OPAC 2. Create a suggestion on the staff interface 3. Edit suggestions 4. Filter suggestions (use the different filters and "organize by" values) Patch rebased. I get an error when I try to access the suggestions page in the OPAC: Could not compile /kohadevbox/koha/opac/opac-suggestions.pl: "SearchSuggestion" is not exported by the C4::Suggestions module Can't continue after import errors at /kohadevbox/koha/opac/opac-suggestions.pl line 27. Created attachment 127100 [details] [review] Bug 23991: Move SearchSuggestion to Koha::Suggestions The C4::Suggestions::SearchSuggestion subroutine is badly written and can be replaced by calls to Koha::Suggestions->search. The hard part in this patch is suggestion.pl, the other occurrences have been replaced easily. Test plan: The idea is to test the whole suggestion workflow. 1. Create a suggestion on OPAC 2. Create a suggestion on the staff interface 3. Edit suggestions 4. Filter suggestions (use the different filters and "organize by" values) Looking. I'm investigating this query log differences: before: 242 Query select DISTINCT(STATUS) as value, count(*) as cnt from suggestions group by value order by STATUS 242 Query SELECT COUNT( * ) FROM `suggestions` `me` WHERE ( ( `STATUS` = 'ASKED' AND `archived` = '0' AND `branchcode` = 'CPL' ) ) 242 Query select DISTINCT(managedby) as value, count(*) as cnt from suggestions group by value order by managedby 242 Query select DISTINCT(acceptedby) as value, count(*) as cnt from suggestions group by value order by acceptedby 242 Query select DISTINCT(suggestedby) as value, count(*) as cnt from suggestions group by value order by suggestedby 242 Query select DISTINCT(budgetid) as value, count(*) as cnt from suggestions group by value order by budgetid after: 242 Query select DISTINCT(STATUS) as value, count(*) as cnt from suggestions group by value order by STATUS 242 Query SELECT COUNT( * ) FROM `suggestions` `me` WHERE ( ( `STATUS` = 'ASKED' AND `archived` = '0' AND `branchcode` = 'CPL' ) ) 242 Query select DISTINCT(managedby) as value, count(*) as cnt from suggestions group by value order by managedby 242 Query select DISTINCT(acceptedby) as value, count(*) as cnt from suggestions group by value order by acceptedby 242 Query select DISTINCT(suggestedby) as value, count(*) as cnt from suggestions group by value order by suggestedby 242 Query select DISTINCT(budgetid) as value, count(*) as cnt from suggestions group by value order by budgetid 242 Query SELECT COUNT( * ) FROM `suggestions` `me` WHERE ( ( `STATUS` = 'ASKED' AND `archived` = '0' AND `branchcode` = 'CPL' ) ) 242 Query SELECT COUNT( * ) FROM `suggestions` `me` WHERE ( ( `STATUS` = 'ASKED' AND `archived` = '0' AND `branchcode` = 'CPL' ) ) 242 Query SELECT `me`.`suggestionid`, `me`.`suggestedby`, `me`.`suggesteddate`, `me`.`managedby`, `me`.`manageddate`, `me`.`acceptedby`, `me`.`accepteddate`, `me`.`rejectedby`, `me`.`rejecteddate`, `me`.`lastmodificationby`, `me`.`lastmodificationdate`, `me`.`STATUS`, `me`.`archived`, `me`.`note`, `me`.`author`, `me`.`title`, `me`.`copyrightdate`, `me`.`publishercode`, `me`.`date`, `me`.`volumedesc`, `me`.`publicationyear`, `me`.`place`, `me`.`isbn`, `me`.`biblionumber`, `me`.`reason`, `me`.`patronreason`, `me`.`budgetid`, `me`.`branchcode`, `me`.`collectiontitle`, `me`.`itemtype`, `me`.`quantity`, `me`.`currency`, `me`.`price`, `me`.`total` FROM `suggestions` `me` WHERE ( ( `STATUS` = 'ASKED' AND `archived` = '0' AND `branchcode` = 'CPL' ) ) 242 Query SELECT `me`.`module`, `me`.`page`, `me`.`tablename`, `me`.`columnname`, `me`.`cannot_be_toggled`, `me`.`is_hidden` FROM `columns_settings` `me` WHERE ( ( `module` = 'acqui' AND `page` = 'suggestions' AND `tablename` = 'suggestions' ) ) 242 Query SELECT COUNT( * ) FROM `suggestions` `me` WHERE ( ( `STATUS` = 'ASKED' AND `archived` = '0' AND `branchcode` = 'CPL' ) ) Created attachment 128071 [details] [review] Bug 23991: Move SearchSuggestion to Koha::Suggestions The C4::Suggestions::SearchSuggestion subroutine is badly written and can be replaced by calls to Koha::Suggestions->search. The hard part in this patch is suggestion.pl, the other occurrences have been replaced easily. Test plan: The idea is to test the whole suggestion workflow. 1. Create a suggestion on OPAC 2. Create a suggestion on the staff interface 3. Edit suggestions 4. Filter suggestions (use the different filters and "organize by" values) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Bug 23991: Remove unused variable Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 128072 [details] [review] Bug 23991: (follow-up) Fix wrong import Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 128073 [details] [review] Bug 23991: Remove SearchSuggestion tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 128074 [details] [review] Bug 23991: Add Koha::Suggestions->search_limited POD Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 128075 [details] [review] Bug 23991: (QA follow-up) Save some DB queries This patch makes the suggestion-related pages rely on array size instead of querying the DB each time they need to. In the case of suggestion/suggestion.pl it goes from 4 COUNT(*) to 1. To test, with KTD: 1. Run on the host machine: $ docker exec -ti koha_db_1 bash $ mysql -ppassword > SET GLOBAL general_log_file='/var/log/mysql/mycustom.log'; > SET GLOBAL log_output = 'FILE'; > SET GLOBAL general_log = 'ON'; > \q $ tail -f /var/log/mysql/mycustom.log | grep suggestions 2. Visit the different pages changed on this bug => SUCCESS: Some queries 3. Apply this patch 4. Repeat 2 => SUCCESS: Less queries! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> bug 23991 - I ran the script to generate random suggesstions - I can see them - apply patch, I don't see them If I load from library link on homepage, I see them but if i change library using pulldown I can't the 'all libraries' link form homepage does not work either http://localhost:8081/cgi-bin/koha/suggestion/suggestion.pl?branchcode=FFL&displayb[…]eptedby=&accepteddate_from=&accepteddate_to=&budgetid=__ANY__ that shows nothing http://localhost:8081/cgi-bin/koha/suggestion/suggestion.pl?branchcode=FFL That shows things Logs show this often: [2022/01/14 15:21:34] [WARN] Use of uninitialized value in string eq at /kohadevbox/koha/suggestion/suggestion.pl line 113. [2022/01/14 15:21:34] [WARN] Use of uninitialized value $v in string eq at /kohadevbox/koha/suggestion/suggestion.pl line 115. Created attachment 129511 [details] [review] Bug 23991: Move SearchSuggestion to Koha::Suggestions The C4::Suggestions::SearchSuggestion subroutine is badly written and can be replaced by calls to Koha::Suggestions->search. The hard part in this patch is suggestion.pl, the other occurrences have been replaced easily. Test plan: The idea is to test the whole suggestion workflow. 1. Create a suggestion on OPAC 2. Create a suggestion on the staff interface 3. Edit suggestions 4. Filter suggestions (use the different filters and "organize by" values) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 129512 [details] [review] Bug 23991: Remove SearchSuggestion tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 129513 [details] [review] Bug 23991: (QA follow-up) Save some DB queries This patch makes the suggestion-related pages rely on array size instead of querying the DB each time they need to. In the case of suggestion/suggestion.pl it goes from 4 COUNT(*) to 1. To test, with KTD: 1. Run on the host machine: $ docker exec -ti koha_db_1 bash $ mysql -ppassword > SET GLOBAL general_log_file='/var/log/mysql/mycustom.log'; > SET GLOBAL log_output = 'FILE'; > SET GLOBAL general_log = 'ON'; > \q $ tail -f /var/log/mysql/mycustom.log | grep suggestions 2. Visit the different pages changed on this bug => SUCCESS: Some queries 3. Apply this patch 4. Repeat 2 => SUCCESS: Less queries! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Nick, I stripped the Koha::Suggestions->search_limited and moved to 29886. I found issues when writing unit tests for it, and they might be responsible for the problems you found. I have attached a new patchset, which is the same as before, without that. But given the fact I fixed some issues on how ->search_limited worked, it might be good if you could re-test. Thanks! Created attachment 129523 [details] [review] Bug 23991: Fix branchcode and budgetid filtering (In reply to Nick Clemens from comment #26) > bug 23991 - I ran the script to generate random suggesstions - I can see > them - apply patch, I don't see them > > > If I load from library link on homepage, I see them > but if i change library using pulldown I can't > > the 'all libraries' link form homepage does not work either > > > http://localhost:8081/cgi-bin/koha/suggestion/suggestion. > pl?branchcode=FFL&displayb[…]eptedby=&accepteddate_from=&accepteddate_to=&bud > getid=__ANY__ > that shows nothing > > http://localhost:8081/cgi-bin/koha/suggestion/suggestion.pl?branchcode=FFL > That shows things > > > Logs show this often: > [2022/01/14 15:21:34] [WARN] Use of uninitialized value in string eq at > /kohadevbox/koha/suggestion/suggestion.pl line 113. > [2022/01/14 15:21:34] [WARN] Use of uninitialized value $v in string eq at > /kohadevbox/koha/suggestion/suggestion.pl line 115. Should be fixed with the last patch. There is certainly more to do to clean this controller and template, but prefer to not rewrite them here and focus on the SearchSuggestion move only. This error is suspicious: "my" variable $suggestion masks earlier declaration in same scope there's effectively a variable overwrite at the top level. About patch "Bug 23991: (QA follow-up) Save some DB queries". I am not sure about it, I always privileged iterator over retrieving the whole set. If this is what we really want it needs to be deeply investigated and that we agree on it. Created attachment 130049 [details] [review] Bug 23991: Fix conflict with bug 28941 Well, this patchset fixed the security bug... Redoing on top of bug 28941 (In reply to Jonathan Druart from comment #34) > About patch "Bug 23991: (QA follow-up) Save some DB queries". I am not sure > about it, I always privileged iterator over retrieving the whole set. > If this is what we really want it needs to be deeply investigated and that > we agree on it. Another option would've been to calculate the count and add a new template param. I decided to minimize the changes to your patchset. Can we get this moving please? Created attachment 131896 [details] [review] Bug 23991: (follow-up) Missing semicolon Everything works well, one issue I found: If I add a new suggestion with acquisition library 'Any' I am returned to view for logged in branch, but no suggestions are loaded Created attachment 132026 [details] [review] Bug 23991: Fix 'all' libraries (In reply to Nick Clemens from comment #39) > Everything works well, one issue I found: > If I add a new suggestion with acquisition library 'Any' I am returned to > view for logged in branch, but no suggestions are loaded I don't recreate that, can you provide more detail please? (In reply to Nick Clemens from comment #39) > Everything works well, one issue I found: > If I add a new suggestion with acquisition library 'Any' I am returned to > view for logged in branch, but no suggestions are loaded Thanks! I found another weirdness but it's in master as well. If you have 2 suggestions, branchcodes are 'CPL' and NULL, organize by "library". You will see CPL (1) and Any (2). Will fix on a follow-up bug report if it's needed. Created attachment 132169 [details] [review] Bug 23991: Move SearchSuggestion to Koha::Suggestions The C4::Suggestions::SearchSuggestion subroutine is badly written and can be replaced by calls to Koha::Suggestions->search. The hard part in this patch is suggestion.pl, the other occurrences have been replaced easily. Test plan: The idea is to test the whole suggestion workflow. 1. Create a suggestion on OPAC 2. Create a suggestion on the staff interface 3. Edit suggestions 4. Filter suggestions (use the different filters and "organize by" values) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132170 [details] [review] Bug 23991: Remove SearchSuggestion tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132171 [details] [review] Bug 23991: (QA follow-up) Save some DB queries This patch makes the suggestion-related pages rely on array size instead of querying the DB each time they need to. In the case of suggestion/suggestion.pl it goes from 4 COUNT(*) to 1. To test, with KTD: 1. Run on the host machine: $ docker exec -ti koha_db_1 bash $ mysql -ppassword > SET GLOBAL general_log_file='/var/log/mysql/mycustom.log'; > SET GLOBAL log_output = 'FILE'; > SET GLOBAL general_log = 'ON'; > \q $ tail -f /var/log/mysql/mycustom.log | grep suggestions 2. Visit the different pages changed on this bug => SUCCESS: Some queries 3. Apply this patch 4. Repeat 2 => SUCCESS: Less queries! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132172 [details] [review] Bug 23991: Fix branchcode and budgetid filtering Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132173 [details] [review] Bug 23991: Fix conflict with bug 28941 Well, this patchset fixed the security bug... Redoing on top of bug 28941 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132174 [details] [review] Bug 23991: (follow-up) Missing semicolon Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 132175 [details] [review] Bug 23991: Fix 'all' libraries Signed-off-by: Nick Clemens <nick@bywatersolutions.com> (In reply to Jonathan Druart from comment #41) > I found another weirdness but it's in master as well. > If you have 2 suggestions, branchcodes are 'CPL' and NULL, organize by > "library". > You will see CPL (1) and Any (2). > > Will fix on a follow-up bug report if it's needed. Yeah, 'Any' looks like 'All libraries' - we could add a new tab for 'Any' - but another report indeed I already tested this one. Will do a second round to check the rebase fixes and Nick's follow-up (In reply to Nick Clemens from comment #49) > (In reply to Jonathan Druart from comment #41) > > I found another weirdness but it's in master as well. > > If you have 2 suggestions, branchcodes are 'CPL' and NULL, organize by > > "library". > > You will see CPL (1) and Any (2). > > > > Will fix on a follow-up bug report if it's needed. > > Yeah, 'Any' looks like 'All libraries' - we could add a new tab for 'Any' - > but another report indeed I believe it was supposed to work that way... but I don't remember why anymore :) So possibly a rename would do? I have 2 suggestions for Centerville, both are Pending and archived. On master, I can make them display by choosing "Include archived" and starting the search: http://localhost:8081/cgi-bin/koha/suggestion/suggestion.pl?branchcode=CPL&displayby=branchcode&title=&author=&isbn=&publishercode=©rightdate=&collectiontitle=&filter_archived=on&STATUS=&suggestedby=&suggesteddate_from=&suggesteddate_to=&managedby=&manageddate_from=&manageddate_to=&acceptedby=&accepteddate_from=&accepteddate_to=&budgetid=__ANY__ This doesn't work with the patch set applied. I can't get archived suggestions to display. Created attachment 133874 [details] [review] Bug 23991: (follow-up) Add value to filter_archived Created attachment 134922 [details] [review] Bug 23991: Move SearchSuggestion to Koha::Suggestions The C4::Suggestions::SearchSuggestion subroutine is badly written and can be replaced by calls to Koha::Suggestions->search. The hard part in this patch is suggestion.pl, the other occurrences have been replaced easily. Test plan: The idea is to test the whole suggestion workflow. 1. Create a suggestion on OPAC 2. Create a suggestion on the staff interface 3. Edit suggestions 4. Filter suggestions (use the different filters and "organize by" values) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: Remove SearchSuggestion tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: (QA follow-up) Save some DB queries This patch makes the suggestion-related pages rely on array size instead of querying the DB each time they need to. In the case of suggestion/suggestion.pl it goes from 4 COUNT(*) to 1. To test, with KTD: 1. Run on the host machine: $ docker exec -ti koha_db_1 bash $ mysql -ppassword > SET GLOBAL general_log_file='/var/log/mysql/mycustom.log'; > SET GLOBAL log_output = 'FILE'; > SET GLOBAL general_log = 'ON'; > \q $ tail -f /var/log/mysql/mycustom.log | grep suggestions 2. Visit the different pages changed on this bug => SUCCESS: Some queries 3. Apply this patch 4. Repeat 2 => SUCCESS: Less queries! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: Fix branchcode and budgetid filtering Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: Fix conflict with bug 28941 Well, this patchset fixed the security bug... Redoing on top of bug 28941 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: (follow-up) Missing semicolon Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: Fix 'all' libraries Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: (follow-up) Add value to filter_archived I had to squash the patches, sorry for the authorship. This one is becoming painful to rebase... Created attachment 136441 [details] [review] Bug 23991: Move SearchSuggestion to Koha::Suggestions The C4::Suggestions::SearchSuggestion subroutine is badly written and can be replaced by calls to Koha::Suggestions->search. The hard part in this patch is suggestion.pl, the other occurrences have been replaced easily. Test plan: The idea is to test the whole suggestion workflow. 1. Create a suggestion on OPAC 2. Create a suggestion on the staff interface 3. Edit suggestions 4. Filter suggestions (use the different filters and "organize by" values) Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: Remove SearchSuggestion tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: (QA follow-up) Save some DB queries This patch makes the suggestion-related pages rely on array size instead of querying the DB each time they need to. In the case of suggestion/suggestion.pl it goes from 4 COUNT(*) to 1. To test, with KTD: 1. Run on the host machine: $ docker exec -ti koha_db_1 bash $ mysql -ppassword > SET GLOBAL general_log_file='/var/log/mysql/mycustom.log'; > SET GLOBAL log_output = 'FILE'; > SET GLOBAL general_log = 'ON'; > \q $ tail -f /var/log/mysql/mycustom.log | grep suggestions 2. Visit the different pages changed on this bug => SUCCESS: Some queries 3. Apply this patch 4. Repeat 2 => SUCCESS: Less queries! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: Fix branchcode and budgetid filtering Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: Fix conflict with bug 28941 Well, this patchset fixed the security bug... Redoing on top of bug 28941 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: (follow-up) Missing semicolon Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: Fix 'all' libraries Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Bug 23991: (follow-up) Add value to filter_archived Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 136442 [details] [review] Bug 23991: (QA follow-up) Fix tabbing It looks like during one of the many rebases we lost some of the tabn handling, likely due to the conversion from jquery-ui to bootstrap tabs. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Everything working as far as I can tell... Code is looking good and with my minor follow-up fixing a rebase issue somewhere along the line I think we're good to go. Passing QA. PS.. I would love to see a switch to API driven tables etc here as suggested earlier in the bug.. the UX is a bit wierd. Pushed to master for 22.11. Nice work everyone, thanks! Created attachment 137201 [details] [review] Bug 23991: Don't display empty div if no suggestions exist If no suggestions exist we should not display an empty tab div Follow-up pushed to master. Thanks Jonathan! Doesn't apply cleanly to 22.05.x, if needed please rebase Please don't backport until the depending bug 31158 has been resolved. This change shouldn't be backported. |