See https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23990
Created attachment 181402 [details] [review] Bug 39721: Remove GetSuggestion from C4/Suggestions.pm This set of patches removes the major part of C4/Suggestions. They have been split for sake of QA's readability but should be squashed. Things to test: Create suggestion Update status of suggestion Create, see and receive an order containing a suggestion Create a suggestion on an already existing record (it will need validation) test cronjobs purge_suggestion and notice_unprocessed_suggestions
Created attachment 181416 [details] [review] Bug 39721: Remove GetSuggestion from C4/Suggestions.pm 1 - Create suggestion 2 - Display a suggestion 3 - Amend a suggestion To do with and without patch
Created attachment 182810 [details] [review] Bug 39721: Remove GetSuggestion from C4/Suggestions.pm 1 - Create suggestion 2 - Display a suggestion 3 - Amend a suggestion To do with and without patch
I would not call unblessed but manipulate the object directly here. It's a bit more work here but it will be easier for the other changes.
ping?
Created attachment 185280 [details] [review] Bug 39721: Remove GetSuggestion from C4/Suggestions.pm 1 - Create suggestion 2 - Display a suggestion 3 - Amend a suggestion To do with and without patch
Created attachment 185281 [details] [review] Bug 39721: Changes suggestion_ref in order to remove it suggestion_ref is quite messy, it needs to be reworked in order to use objects in suggestion.pl. As far as I studied, it is used as - the whole input from forms, - the default content to input in a form - the content that will be amended in order to propose another hash to modSuggestion. However, ModSuggestion will be removed so we don't need such a hash - every suggestion that need to be updated using the "change selected suggestion" tool - create the object suggestion_only, whose purpose is still unclear to me. As a result it is created by : > 25 my $suggestion_ref = { $input->Vars }; And by removing a number of keys depending on the use of suggestion_ref in the script. In this patch, we remove every code made to use suggestion_ref as the source object for form and use another variable, $stored_suggestion which is an object. In next patch we aim at fully removing $suggestion_ref, which is still used by ModSuggestion
Created attachment 185282 [details] [review] Bug 39721: (follow-up) Remove unblessed
Created attachment 185283 [details] [review] Bug 39721: tidy
Hi Baptiste, When I install your patch, I am unable to create or modify a suggestion in the staff intranet and get a 500 error. The log shows: DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'CAD' in 'where clause' at /kohadevbox/koha/Koha/Objects.pm line 401 Looks like that is a reference to Canadian Dollars currency (CAD) It still works fine to create a request in the OPAC. Hope this helps.
There is also: qw( suggestedbyme op displayby tabcode notify filter_archived koha_login_context auth_forwarded_hash password userid ); [2025/09/02 12:49:03] [WARN] Useless use of a constant ("suggestedbyme") in void context at /kohadevbox/koha/suggestion/suggestion.pl line 102. [2025/09/02 12:49:03] [WARN] Useless use of a constant ("op") in void context at /kohadevbox/koha/suggestion/suggestion.pl line 102. [2025/09/02 12:49:03] [WARN] Useless use of a constant ("displayby") in void context at /kohadevbox/koha/suggestion/suggestion.pl line 102. [2025/09/02 12:49:03] [WARN] Useless use of a constant ("tabcode") in void context at /kohadevbox/koha/suggestion/suggestion.pl line 102. [2025/09/02 12:49:03] [WARN] Useless use of a constant ("notify") in void context at /kohadevbox/koha/suggestion/suggestion.pl line 102. [2025/09/02 12:49:03] [WARN] Useless use of a constant ("filter_archived") in void context at /kohadevbox/koha/suggestion/suggestion.pl line 102. [2025/09/02 12:49:03] [WARN] Useless use of a constant ("koha_login_context") in void context at /kohadevbox/koha/suggestion/suggestion.pl line 102. [2025/09/02 12:49:03] [WARN] Useless use of a constant ("auth_forwarded_hash") in void context at /kohadevbox/koha/suggestion/suggestion.pl line 102. [2025/09/02 12:49:03] [WARN] Useless use of a constant ("password") in void context at /kohadevbox/koha/suggestion/suggestion.pl line 102. [2025/09/02 12:49:03] [WARN] Useless use of a constant ("userid") in void context at /kohadevbox/koha/suggestion/suggestion.pl line 102
I would restart from scratch here. You should not try to remove suggestion_ref. suggestion.pl is used for add/edit but also for searching. On this specific bug you should focus on GetSuggestion only.
(In reply to Jonathan Druart from comment #12) > I would restart from scratch here. You should not try to remove > suggestion_ref. > > suggestion.pl is used for add/edit but also for searching. Sorry, this is wrong since we now have bug 33430. Keep trying this way then! :)
comment 10 is because you removed this line: - map { delete $$suggestion_ref{$_} unless $_ eq 'branchcode' } keys %$suggestion_ref; There are other lines that have been removed incorrectly IMO: -foreach my $key ( keys %$suggestion_ref ) { - delete $suggestion_ref->{$key} if ( !$suggestion_ref->{$key} && ( $op eq 'else' ) ); - delete $suggestion_ref->{$key} if $key =~ m{^DataTables_acqui_suggestions_suggestions}; -}
Created attachment 188646 [details] [review] Bug 39721: fix suggestion_ref issues
Created attachment 188648 [details] [review] Bug 39721: Remove GetSuggestion from C4/Suggestions.pm 1 - Create suggestion 2 - Display a suggestion 3 - Amend a suggestion To do with and without patch
Created attachment 188649 [details] [review] Bug 39721: Changes suggestion_ref in order to remove it suggestion_ref is quite messy, it needs to be reworked in order to use objects in suggestion.pl. As far as I studied, it is used as - the whole input from forms, - the default content to input in a form - the content that will be amended in order to propose another hash to modSuggestion. However, ModSuggestion will be removed so we don't need such a hash - every suggestion that need to be updated using the "change selected suggestion" tool - create the object suggestion_only, whose purpose is still unclear to me. As a result it is created by : > 25 my $suggestion_ref = { $input->Vars }; And by removing a number of keys depending on the use of suggestion_ref in the script. In this patch, we remove every code made to use suggestion_ref as the source object for form and use another variable, $stored_suggestion which is an object. In next patch we aim at fully removing $suggestion_ref, which is still used by ModSuggestion
Created attachment 188650 [details] [review] Bug 39721: (follow-up) Remove unblessed
Created attachment 188651 [details] [review] Bug 39721: tidy
Created attachment 188652 [details] [review] Bug 39721: fix suggestion_ref issues
Created attachment 190940 [details] [review] Bug 39721: Remove GetSuggestion from C4/Suggestions.pm 1 - Create suggestion 2 - Display a suggestion 3 - Amend a suggestion To do with and without patch
Created attachment 190941 [details] [review] Bug 39721: Changes suggestion_ref in order to remove it suggestion_ref is quite messy, it needs to be reworked in order to use objects in suggestion.pl. As far as I studied, it is used as - the whole input from forms, - the default content to input in a form - the content that will be amended in order to propose another hash to modSuggestion. However, ModSuggestion will be removed so we don't need such a hash - every suggestion that need to be updated using the "change selected suggestion" tool - create the object suggestion_only, whose purpose is still unclear to me. As a result it is created by : > 25 my $suggestion_ref = { $input->Vars }; And by removing a number of keys depending on the use of suggestion_ref in the script. In this patch, we remove every code made to use suggestion_ref as the source object for form and use another variable, $stored_suggestion which is an object. In next patch we aim at fully removing $suggestion_ref, which is still used by ModSuggestion
Applying: Bug 39721: (follow-up) Remove unblessed error: sha1 information is lacking or useless (t/db_dependent/Suggestions.t). error: could not build fake ancestor Patch failed at 0001 Bug 39721: (follow-up) Remove unblessed
Sry I did not obsolete the patches after squashing, I just re-tested it should be fine now
Created attachment 191656 [details] [review] Bug 39721: Remove GetSuggestion from C4/Suggestions.pm 1 - Create suggestion 2 - Display a suggestion 3 - Amend a suggestion To do with and without patch
Created attachment 191657 [details] [review] Bug 39721: Changes suggestion_ref in order to remove it suggestion_ref is quite messy, it needs to be reworked in order to use objects in suggestion.pl. As far as I studied, it is used as - the whole input from forms, - the default content to input in a form - the content that will be amended in order to propose another hash to modSuggestion. However, ModSuggestion will be removed so we don't need such a hash - every suggestion that need to be updated using the "change selected suggestion" tool - create the object suggestion_only, whose purpose is still unclear to me. As a result it is created by : > 25 my $suggestion_ref = { $input->Vars }; And by removing a number of keys depending on the use of suggestion_ref in the script. In this patch, we remove every code made to use suggestion_ref as the source object for form and use another variable, $stored_suggestion which is an object. In next patch we aim at fully removing $suggestion_ref, which is still used by ModSuggestion
Created attachment 191909 [details] [review] Bug 39721: Remove GetSuggestion from C4/Suggestions.pm 1 - Create suggestion 2 - Display a suggestion 3 - Amend a suggestion To do with and without patch Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Created attachment 191910 [details] [review] Bug 39721: Changes suggestion_ref in order to remove it suggestion_ref is quite messy, it needs to be reworked in order to use objects in suggestion.pl. As far as I studied, it is used as - the whole input from forms, - the default content to input in a form - the content that will be amended in order to propose another hash to modSuggestion. However, ModSuggestion will be removed so we don't need such a hash - every suggestion that need to be updated using the "change selected suggestion" tool - create the object suggestion_only, whose purpose is still unclear to me. As a result it is created by : > 25 my $suggestion_ref = { $input->Vars }; And by removing a number of keys depending on the use of suggestion_ref in the script. In this patch, we remove every code made to use suggestion_ref as the source object for form and use another variable, $stored_suggestion which is an object. In next patch we aim at fully removing $suggestion_ref, which is still used by ModSuggestion Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Yep @joubu there are still 2 unblessed The first one is in ModSuggestion which will not survive this cycle of patches. The second is in acqui/neworderempty.pl, but it is stored in a variable called $data, which is either an hashref from GetSuggestion or an hashref from GetBiblioData. Do you think we can remove this unblessed ?
Please add test coverage for Koha::Suggestion::accepter.
(In reply to Baptiste Wojtkowski (bwoj) from comment #29) > The second is in acqui/neworderempty.pl, but it is stored in a variable > called $data, which is either an hashref from GetSuggestion or an hashref > from GetBiblioData. Do you think we can remove this unblessed ? I think it's ok for now.
Created attachment 191927 [details] [review] Bug 39721: Add test for accepter
Created attachment 192020 [details] [review] Bug 39721: Remove GetSuggestion from C4/Suggestions.pm 1 - Create suggestion 2 - Display a suggestion 3 - Amend a suggestion To do with and without patch Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> Amended-by: Jonathan Druart tidy Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 192021 [details] [review] Bug 39721: Changes suggestion_ref in order to remove it suggestion_ref is quite messy, it needs to be reworked in order to use objects in suggestion.pl. As far as I studied, it is used as - the whole input from forms, - the default content to input in a form - the content that will be amended in order to propose another hash to modSuggestion. However, ModSuggestion will be removed so we don't need such a hash - every suggestion that need to be updated using the "change selected suggestion" tool - create the object suggestion_only, whose purpose is still unclear to me. As a result it is created by : > 25 my $suggestion_ref = { $input->Vars }; And by removing a number of keys depending on the use of suggestion_ref in the script. In this patch, we remove every code made to use suggestion_ref as the source object for form and use another variable, $stored_suggestion which is an object. In next patch we aim at fully removing $suggestion_ref, which is still used by ModSuggestion Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 192022 [details] [review] Bug 39721: Add test for accepter Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nice work everyone! Pushed to main for 26.05