Created attachment 41125 [details] [review] Bug 14589: Replace fetch CGI::Cookie calls This patch is inspired by the change in bug 14588. It replaces the indirect object syntax for CGI::Cookie. Some trivial corrections are made to authorities/merge_ajax.pl just as bug 14588 does for cataloguing. Test plan: Run the URL authorities/merge_ajax.pl in staff. Upload a file with Stage MARC records for import.
Created attachment 41457 [details] [review] Bug 14589: Replace fetch CGI::Cookie calls This patch is inspired by the change in bug 14588. It replaces the indirect object syntax for CGI::Cookie. Some trivial corrections are made to authorities/merge_ajax.pl just as bug 14588 does for cataloguing. Test plan: Run the URL authorities/merge_ajax.pl in staff. Upload a file with Stage MARC records for import.
Rebased on top of 6874. I need this (trivial) patch for 14321. Please add your (easy) signoff..
Created attachment 41675 [details] [review] Bug 14589: Replace fetch CGI::Cookie calls This patch is inspired by the change in bug 14588. It replaces the indirect object syntax for CGI::Cookie. Some trivial corrections are made to authorities/merge_ajax.pl just as bug 14588 does for cataloguing. Test plan: Run the URL authorities/merge_ajax.pl in staff. Upload a file with Stage MARC records for import.
Comment on attachment 41675 [details] [review] Bug 14589: Replace fetch CGI::Cookie calls Review of attachment 41675 [details] [review]: ----------------------------------------------------------------- There is scope creep as noted below. It would make more sense to have a "Replace 'new CGI' calls" bug separately, as this occurs in more than just these places. ::: tools/background-job-progress.pl @@ +30,4 @@ > use CGI::Cookie; # need to check cookies before > # having CGI parse the POST request > > +my $input = CGI->new; Scope creep. @@ +51,4 @@ > $job_status = $job->status(); > } > > +my $reply = CGI->new; Scope creep. ::: tools/upload-file-progress.pl @@ +47,4 @@ > } > } > > +my $reply = CGI->new; Scope creep.
(In reply to M. Tompsett from comment #5) > There is scope creep as noted below. It would make more sense to have a > "Replace 'new CGI' calls" bug separately, as this occurs in more than just > these places. I would not really mind, but most important for me now is to get bug 14321 rolling. And this is a dependency.. What about a title change :) ?
Created attachment 41691 [details] [review] Bug 14589: Adjust authorities_merge_ajax and replace some indirect syntax This patch does the following: [1] Adjust authorities_merge_ajax just as in bug 14588. [2] Replace some indirect syntax for fetch GGI::Cookie. [3] Along the way replace some new CGI's. Note that I am not aiming to replace them Koha wide. The "fetch class" variant is less readable. Test plan: [1] Run the URL authorities/merge_ajax.pl in staff. [2] Upload a file with Stage MARC records for import.
Created attachment 41882 [details] [review] Bug 14589: Replace fetch CGI::Cookie calls This patch is inspired by the change in bug 14588. It replaces the indirect object syntax for CGI::Cookie. Some trivial corrections are made to authorities/merge_ajax.pl just as bug 14588 does for cataloguing. NOTE: The changes to tools/upload-file.pl and upload-file-progress.pl are moved to report 14321. Test plan: Run the URL authorities/merge_ajax.pl in staff. Upload a file with Stage MARC records for import.
Created attachment 41883 [details] [review] Bug 14589: Adjust authorities_merge_ajax and replace some indirect syntax This patch does the following: [1] Adjust authorities_merge_ajax just as in bug 14588. [2] Replace some indirect syntax for fetch GGI::Cookie. [3] Along the way replace some new CGI's. Note that I am not aiming to replace them Koha wide. The "fetch class" variant is less readable. NOTE: The changes to tools/upload-file.pl and upload-file-progress.pl are moved to report 14321. Test plan: [1] Run the URL authorities/merge_ajax.pl in staff. [2] Upload a file with Stage MARC records for import.
Created attachment 44177 [details] [review] [SIGNED-OFF] Bug 14589: Adjust authorities_merge_ajax and replace some indirect syntax This patch does the following: [1] Adjust authorities_merge_ajax just as in bug 14588. [2] Replace some indirect syntax for fetch GGI::Cookie. [3] Along the way replace some new CGI's. Note that I am not aiming to replace them Koha wide. The "fetch class" variant is less readable. NOTE: The changes to tools/upload-file.pl and upload-file-progress.pl are moved to report 14321. Test plan: [1] Run the URL authorities/merge_ajax.pl in staff. [2] Upload a file with Stage MARC records for import. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> [1] It runs, but also before patch [2] File uploads without problem No errors
Created attachment 44187 [details] [review] Bug 14589: Adjust authorities_merge_ajax and replace some indirect syntax This patch does the following: [1] Adjust authorities_merge_ajax just as in bug 14588. [2] Replace some indirect syntax for fetch GGI::Cookie. [3] Along the way replace some new CGI's. Note that I am not aiming to replace them Koha wide. The "fetch class" variant is less readable. NOTE: The changes to tools/upload-file.pl and upload-file-progress.pl are moved to report 14321. Test plan: [1] Run the URL authorities/merge_ajax.pl in staff. [2] Upload a file with Stage MARC records for import. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> [1] It runs, but also before patch [2] File uploads without problem No errors Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> We could certainly remove 1 or 2 call to CGI->new in tools/background-job-progress.pl
Patch pushed to master. Thanks Marcel!