It's not used and is confusing. It does not make sense for now with plack and our current codebase. We should remove it (and the other places using this pattern) and reintroduce it later, correctly implemented, if we need it back.
Created attachment 163500 [details] [review] Bug 36367: Remove get_schema_cached
Created attachment 163501 [details] [review] Bug 36367: Remove flush_schema_cached
Created attachment 163502 [details] [review] Bug 36367: Remove _new_userenv TODO - better review C4::Auth's changes. Are all the removal of _new_userenv correct/enough?
Created attachment 163503 [details] [review] Bug 36367: Remove context stack We are not using it and it's confusing, let's remove the context stack.
Created attachment 163504 [details] [review] Bug 36367: Remove dbh_stack Same pattern, remove dbh stack
Created attachment 163505 [details] [review] Bug 36367: Remove schema_stack Same pattern in Koha::Database
Created attachment 163506 [details] [review] Bug 36367: Set to undef when unsetting We usually test if C4::Context->userenv, so we need to undef when unsetting, not {} (evaluated true)
Created attachment 163507 [details] [review] Bug 36367: Remove obsolete tests
Created attachment 163508 [details] [review] Bug 36367: Fix BackgroundJob.t This test is useless it seems, it is not testing background jobs behaviour.
Created attachment 165115 [details] [review] Bug 36367: Remove get_schema_cached Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 165116 [details] [review] Bug 36367: Remove flush_schema_cached Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 165117 [details] [review] Bug 36367: Remove _new_userenv TODO - better review C4::Auth's changes. Are all the removal of _new_userenv correct/enough? Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 165118 [details] [review] Bug 36367: Remove context stack We are not using it and it's confusing, let's remove the context stack. Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 165119 [details] [review] Bug 36367: Remove dbh_stack Same pattern, remove dbh stack Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 165120 [details] [review] Bug 36367: Remove schema_stack Same pattern in Koha::Database Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 165121 [details] [review] Bug 36367: Set to undef when unsetting We usually test if C4::Context->userenv, so we need to undef when unsetting, not {} (evaluated true) Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 165122 [details] [review] Bug 36367: Remove obsolete tests Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 165123 [details] [review] Bug 36367: Fix BackgroundJob.t This test is useless it seems, it is not testing background jobs behaviour. Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 165124 [details] [review] Bug 36367: Remove call to non-existent subroutine new_userenv Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 165125 [details] [review] Bug 36367: Fix test t/Token.t Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 165126 [details] [review] Bug 36367: Fix test t/db_dependent/Auth.t Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Patches look good. Tests pass. QA tests pass (except one about POD but I think it's a false negative). I didn't notice any bug trying to login/logout with different patrons and using staff interface and opac
Created attachment 167380 [details] [review] Bug 36367: Remove get_schema_cached Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 167381 [details] [review] Bug 36367: Remove flush_schema_cached Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 167382 [details] [review] Bug 36367: Remove _new_userenv TODO - better review C4::Auth's changes. Are all the removal of _new_userenv correct/enough? Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 167383 [details] [review] Bug 36367: Remove context stack We are not using it and it's confusing, let's remove the context stack. Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 167384 [details] [review] Bug 36367: Remove dbh_stack Same pattern, remove dbh stack Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 167385 [details] [review] Bug 36367: Remove schema_stack Same pattern in Koha::Database Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 167386 [details] [review] Bug 36367: Set to undef when unsetting We usually test if C4::Context->userenv, so we need to undef when unsetting, not {} (evaluated true) Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 167387 [details] [review] Bug 36367: Remove obsolete tests Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 167388 [details] [review] Bug 36367: Fix BackgroundJob.t This test is useless it seems, it is not testing background jobs behaviour. Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 167389 [details] [review] Bug 36367: Remove call to non-existent subroutine new_userenv Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 167390 [details] [review] Bug 36367: Fix test t/Token.t Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 167391 [details] [review] Bug 36367: Fix test t/db_dependent/Auth.t Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Awesome work here, thanks for following through on it. Passing QA
Thanks for all the hard work! Pushed to main for the next 24.11.00 release as RM Assistant