The full version: [2022/11/17 08:51:39] [WARN] The job has not been sent to the message broker: (Wide character in syswrite at /usr/lib/x86_64-linux-gnu/perl-base/IO/Handle.pm line 200.) at /usr/share/koha/Koha/BackgroundJob.pm line 147.
I think we should encode via Net::Stomp but I will first add a quick fix.
Created attachment 143986 [details] [review] Bug 32242: Resolve encoding issue when sending to Net::Stomp This is meant as a quick fix. It should be enhanced later on by letting Net::Stomp do the UTF8 conversion. Test plan: Run batch mod items. Add a Unicode char in the notes. Check if job runs and results are correct. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
FYI Tomas: The json to UTF8 conversion was handled correctly at the database layer but this bug is about sending json to RabbitMQ. That conversion is not handled properly (yet). See also bug 32243.
Created attachment 143987 [details] [review] Bug 32242: Resolve encoding issue when sending to Net::Stomp This is meant as a quick fix. It should be enhanced later on by letting Net::Stomp do the UTF8 conversion. Test plan: Run batch mod items. Add a Unicode char in the notes. Check if job runs and results are correct. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Still added a use Encode
Created attachment 143994 [details] [review] Bug 32242: Resolve encoding issue when sending to Net::Stomp This is meant as a quick fix. It should be enhanced later on by letting Net::Stomp do the UTF8 conversion. Test plan: Run batch mod items. Add a Unicode char in the notes. Check if job runs and results are correct. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: David Nind <david@davidnind.com>
Testing notes (using KTD): 1. Use Batch item modification (Cataloging > Batch editing) to edit two records: - Add a Unicode character, such as 〼, in the non-public note and public note fields 2. View the details of job - no records modified 3. Check the items - no change was made 4. Check the /var/log/koha/kohadev/plack-intranet-error.log - there is a warning: [2022/11/17 11:43:13] [WARN] The job has not been sent to the message broker : (Wide character in syswrite at /usr/lib/x86_64-linux-gnu/perl-base/IO/Hand le.pm line 200. 5. Apply the patch (and restart all the things) 6. Repeat steps 1-4 - the records should be successfully modified and there should be no warning message in the logs.
It's fixing the problem. Any good reasons to not import encode_utf8 from Encode?
(In reply to Jonathan Druart from comment #8) > It's fixing the problem. > > Any good reasons to not import encode_utf8 from Encode? I didnt need to import it. Main reason ;)
That's not a discussion for this bug report, but I thought we agreed on the following pattern: use Module qw( sub list ); then call Module::list() of list()
(In reply to Jonathan Druart from comment #10) > That's not a discussion for this bug report, but I thought we agreed on the > following pattern: > > use Module qw( sub list ); > then call Module::list() of list() It is conform that agreement with the list being empty ;)
Created attachment 143998 [details] [review] Bug 32242: Add selenium tests This is not testing the thing at the correct level, but at least we test the whole workflow.
Created attachment 144016 [details] [review] Bug 32242: Resolve encoding issue when sending to Net::Stomp This is meant as a quick fix. It should be enhanced later on by letting Net::Stomp do the UTF8 conversion. Test plan: Run batch mod items. Add a Unicode char in the notes. Check if job runs and results are correct. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 144017 [details] [review] Bug 32242: Add selenium tests This is not testing the thing at the correct level, but at least we test the whole workflow. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Pushed to master for 22.11. Nice work everyone, thanks!
This broke tests: kohadev-koha@kohadevbox:/kohadevbox/koha$ prove t/db_dependent/Koha/BackgroundJobs.t t/db_dependent/Koha/BackgroundJobs.t .. 1/15 # Failed test 'No jobs found without userenv' # at t/db_dependent/Koha/BackgroundJobs.t line 135. # got: '2' # expected: '0' # Looks like you failed 1 test of 3. # Failed test 'search_limited' # at t/db_dependent/Koha/BackgroundJobs.t line 142. ... Reverting makes tests pass: git revert 022034a1c7b91adac539c7ed16fc7b48d566912e Please take your time to fix it for the release.
(In reply to Tomás Cohen Arazi from comment #16) > This broke tests: > > kohadev-koha@kohadevbox:/kohadevbox/koha$ prove > t/db_dependent/Koha/BackgroundJobs.t > t/db_dependent/Koha/BackgroundJobs.t .. 1/15 > # Failed test 'No jobs found without userenv' > # at t/db_dependent/Koha/BackgroundJobs.t line 135. > # got: '2' > # expected: '0' > # Looks like you failed 1 test of 3. > > # Failed test 'search_limited' > # at t/db_dependent/Koha/BackgroundJobs.t line 142. > ... > > Reverting makes tests pass: > > git revert 022034a1c7b91adac539c7ed16fc7b48d566912e > > Please take your time to fix it for the release. I was wrong. My bisect was flawed because of the reset_all I was using.
I am seeing this now Tomas, but it should not be related to this report: # Subtest: purge 1..9 ok 1 - Only the old finished type1 job would be purged not ok 2 - All finished old jobs would be purged with job_types = all # Failed test 'All finished old jobs would be purged with job_types = all' # at t/db_dependent/Koha/BackgroundJobs.t line 162. # got: '5' # expected: '2' ok 3 - All jobs still left in queue ok 4 - Only the old finished type1 job is purged ok 5 - 3 jobs still left in queue not ok 6 - The remaining old finished jobs is purged # Failed test 'The remaining old finished jobs is purged' # at t/db_dependent/Koha/BackgroundJobs.t line 183. # got: '4' # expected: '1' ok 7 - 2 jobs still left in queue ok 8 - Unfinished job still left in queue ok 9 - Recent finished job still left in queue # Looks like you failed 2 tests of 9. not ok 15 - purge
(In reply to Marcel de Rooy from comment #18) > not ok 2 - All finished old jobs would be purged with job_types = all > # Failed test 'All finished old jobs would be purged with job_types = > all' > # at t/db_dependent/Koha/BackgroundJobs.t line 162. > # got: '5' > # expected: '2' It seems that this test assumes that there are no other old jobs. So this should be corrected by removing them all first ? Opened bug 32304.
(In reply to Tomás Cohen Arazi from comment #17) > > kohadev-koha@kohadevbox:/kohadevbox/koha$ prove > > t/db_dependent/Koha/BackgroundJobs.t > > t/db_dependent/Koha/BackgroundJobs.t .. 1/15 > > # Failed test 'No jobs found without userenv' > > # at t/db_dependent/Koha/BackgroundJobs.t line 135. > > # got: '2' > > # expected: '0' > > # Looks like you failed 1 test of 3. > > > > # Failed test 'search_limited' > > # at t/db_dependent/Koha/BackgroundJobs.t line 142. Added a fix for this one on bug 32304 too now.
This should have been backported into 22.05.x
Created attachment 144772 [details] [review] Bug 32242: [22.05] fix selenium tests for 22.05
Backported to 22.05.x for upcoming 22.05.08 release
This one seems not to apply to 21.11 branch since it depends on 31351 (only in 22.05.x). Won't backport