[Bug 32370] New: Provide a generic set of tools for JSON fields
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Bug ID: 32370 Summary: Provide a generic set of tools for JSON fields Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Status|NEW |ASSIGNED Depends on| |30943 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30943 [Bug 30943] Make background job classes use helpers -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 144329 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144329&action=edit Bug 32370: Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 144330 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144330&action=edit Bug 32370: Provide a generic set of tools for JSON fields This patch introduces a new class for extending Koha::Object-based classes so they have reusable ways to deal with JSON encoded fields. As documented in the POD, classes can now ```perl use base qw(Koha::Object Koha::Object::JSONFields); ``` and will have the following methods available: * decode_json_field * set_encoded_json_field For convenience, the Koha::Auth::Identity::Provider class is tweaked to make use of this, and also to prove the behavior is correct (expected results, exceptions, encoding, etc). To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Auth/Identity/Provider.t \ t/db_dependent/api/v1/idp.t => SUCCESS: Identity provider related tests pass! 2. Apply this patches 3. Repeat 1 => SUCCESS: Tests still pass! 4. Run the specific tests: k$ prove t/db_dependent/Koha/Object/JSONField.t => SUCCESS: Tests pass! 5. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |31351 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31351 [Bug 31351] Worker dies on reindex job when operator last name/first name/branch name contains non-ASCII chars -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com, | |kyle@bywatersolutions.com, | |m.de.rooy@rijksmuseum.nl, | |martin.renvoize@ptfs-europe | |.com, | |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 144331 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144331&action=edit Bug 32370: (follow-up) Make Koha::BackgroundJob use Koha::Object::JSONFields -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #3)
Created attachment 144331 [details] [review] Bug 32370: (follow-up) Make Koha::BackgroundJob use Koha::Object::JSONFields
^^ to avoid the 'completeness' discussion we always have (y) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144329|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144330|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144331|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 144332 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144332&action=edit Bug 32370: Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 144333 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144333&action=edit Bug 32370: Provide a generic set of tools for JSON fields This patch introduces a new class for extending Koha::Object-based classes so they have reusable ways to deal with JSON encoded fields. As documented in the POD, classes can now ```perl use base qw(Koha::Object Koha::Object::JSONFields); ``` and will have the following methods available: * decode_json_field * set_encoded_json_field For convenience, the Koha::Auth::Identity::Provider class is tweaked to make use of this, and also to prove the behavior is correct (expected results, exceptions, encoding, etc). To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Auth/Identity/Provider.t \ t/db_dependent/api/v1/idp.t => SUCCESS: Identity provider related tests pass! 2. Apply this patches 3. Repeat 1 => SUCCESS: Tests still pass! 4. Run the specific tests: k$ prove t/db_dependent/Koha/Object/JSONField.t => SUCCESS: Tests pass! 5. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 144334 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144334&action=edit Bug 32370: (follow-up) Make Koha::BackgroundJob use Koha::Object::JSONFields -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I am certainly missing the context here, but this feels like unnecessary overhead. You are basically adding a Koha::Object base class for JSON decode and encode calls. That's all. The exception is there only to raise a Koha exception, but having a JSON exception is also fine. The MissingParameter exceptions are not needed either IMO, you will get a perl error if you are calling the method with missing mandatory parameter. If we really need these methods, and that I am missing the root idea and context of these patches, can you clarify why a base class instead of a regular class? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm still contemplating this... Considering https://metacpan.org/pod/DBIx::Class::InflateColumn::Serializer::JSON as an example to compare to. I think I still prefer the idea of a Koha::Object::InflateField::* hierarchy if we want something like this... to me, I think it's more elegant to outright replace the field accessor with automatic inflation rather than add additional accessors.. then fall back to get_column should one need the raw data.. but perhaps just because I'm so familiar with 'the DBIC way' I'm not seeing what we'd be losing in Koha land. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #8)
I am certainly missing the context here, but this feels like unnecessary overhead.
I thought the examples in IdP and BackgroundJob were enough context. But fair.
You are basically adding a Koha::Object base class for JSON decode and encode calls. That's all.
We are repeating code everywhere, and I provide a way to extend Koha::Object-derived classes with accessors for JSON fields. Yes. It sounds like a simple enough approach. If we did a regular class, we would need to instantiate it as needed and it feels counter productive and unnecessary. (In reply to Martin Renvoize from comment #9)
I'm still contemplating this...
Considering https://metacpan.org/pod/DBIx::Class::InflateColumn::Serializer::JSON as an example to compare to.
I agree with this approach and I found it interesting. YOu mentioned it after I already wrote this so I thought I would post this to start the discussion. Will think about it. The main drawback is that devs might be caught by surprise with inflation, but it shouldn't be a problem. It will jus tbe a general behavior change and we would need to measure performance/memory overhead, as my implementation will only introduce overhead if the new accessors are really called. I tried to provide real-life examples on how it would be used to make the point code-wise. (In reply to Jonathan Druart from comment #8)
The exception is there only to raise a Koha exception, but having a JSON exception is also fine. The MissingParameter exceptions are not needed either IMO, you will get a perl error if you are calling the method with missing mandatory parameter.
Yeah, I tried that. You get - "Can't call method undefined", I prefer a named exception but this is really taste. - As of JSON, I think I picked what we already had. But it is fair not to throw our own exception; again, a matter of taste. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #11 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- malformed JSON string, neither tag, array, object, number, string or atom, at character offset 0 (before "HASH(0x55ae880d9628)") at /kohadevbox/koha/misc/background_jobs_worker.pl line 93. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #12 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #11)
malformed JSON string, neither tag, array, object, number, string or atom, at character offset 0 (before "HASH(0x55ae880d9628)") at /kohadevbox/koha/misc/background_jobs_worker.pl line 93.
Hum, forget that, I guess I need to copy the new version of the misc script. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144334|0 |1 is obsolete| | --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 144347 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144347&action=edit Bug 32370: (follow-up) Make Koha::BackgroundJob use Koha::Object::JSONFields -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144347|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144332|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144333|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 144376 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144376&action=edit Bug 32370: Unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 144377 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144377&action=edit Bug 32370: Provide a generic set of tools for JSON fields This patch introduces a new class for extending Koha::Object-based classes so they have reusable ways to deal with JSON encoded fields. As documented in the POD, classes can now ```perl use base qw(Koha::Object Koha::Object::JSONFields); ``` and will have the following methods available: * decode_json_field * set_encoded_json_field For convenience, the Koha::Auth::Identity::Provider class is tweaked to make use of this, and also to prove the behavior is correct (expected results, exceptions, encoding, etc). To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Auth/Identity/Provider.t \ t/db_dependent/api/v1/idp.t => SUCCESS: Identity provider related tests pass! 2. Apply this patches 3. Repeat 1 => SUCCESS: Tests still pass! 4. Run the specific tests: k$ prove t/db_dependent/Koha/Object/JSONField.t => SUCCESS: Tests pass! 5. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #16 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 144378 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144378&action=edit Bug 32370: (follow-up) Make Koha::BackgroundJob use Koha::Object::JSONFields -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #17 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- 108 $self->set_encoded_json_field( { data => $job_args, field => 'data' } ); 120 $job_args->{job_id} = $self->id; 130 my $json_args = $self->data; # we pass the JSON string 137 my $encoded_args = Encode::encode_utf8( $json_args ); # FIXME We should better leave this to Net::Stomp? This is terribly wrong, we are loosing job_id... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I think attention must be on bug 32393, bug 32394 and bug 32395 first. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #19 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #12)
(In reply to Jonathan Druart from comment #11)
malformed JSON string, neither tag, array, object, number, string or atom, at character offset 0 (before "HASH(0x55ae880d9628)") at /kohadevbox/koha/misc/background_jobs_worker.pl line 93.
Hum, forget that, I guess I need to copy the new version of the misc script.
I have faced some very weird behaviours testing this (and linked bugs). At some point I got background_jobs.pl processes that were not attached to the koha-worker daemon, and were using an old version of the code (like this message that was because of this bug, but was also in the log after a checkout master && restart_all). Just noting, but good to keep in mind. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #20 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #19)
(In reply to Jonathan Druart from comment #12)
(In reply to Jonathan Druart from comment #11)
malformed JSON string, neither tag, array, object, number, string or atom, at character offset 0 (before "HASH(0x55ae880d9628)") at /kohadevbox/koha/misc/background_jobs_worker.pl line 93.
Hum, forget that, I guess I need to copy the new version of the misc script.
I have faced some very weird behaviours testing this (and linked bugs).
At some point I got background_jobs.pl processes that were not attached to the koha-worker daemon, and were using an old version of the code (like this message that was because of this bug, but was also in the log after a checkout master && restart_all). Just noting, but good to keep in mind.
Not having the job_id might be a good source for weirdness :-D well spotted, Jonathan. I should've put the background jobs related change on a separate bug. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144378|0 |1 is obsolete| | --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 144445 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144445&action=edit Bug 32370: (follow-up) Make Koha::BackgroundJob use Koha::Object::JSONFields https://bugs.koha-community.org/show_bug.cgi?id=32371 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=32393 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144376|0 |1 is obsolete| | --- Comment #22 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 145213 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145213&action=edit Bug 32370: Unit tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144377|0 |1 is obsolete| | --- Comment #23 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 145214 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145214&action=edit Bug 32370: Provide a generic set of tools for JSON fields This patch introduces a new class for extending Koha::Object-based classes so they have reusable ways to deal with JSON encoded fields. As documented in the POD, classes can now ```perl use base qw(Koha::Object Koha::Object::JSONFields); ``` and will have the following methods available: * decode_json_field * set_encoded_json_field For convenience, the Koha::Auth::Identity::Provider class is tweaked to make use of this, and also to prove the behavior is correct (expected results, exceptions, encoding, etc). To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Auth/Identity/Provider.t \ t/db_dependent/api/v1/idp.t => SUCCESS: Identity provider related tests pass! 2. Apply this patches 3. Repeat 1 => SUCCESS: Tests still pass! 4. Run the specific tests: k$ prove t/db_dependent/Koha/Object/JSONField.t => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144445|0 |1 is obsolete| | --- Comment #24 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 145215 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145215&action=edit Bug 32370: (follow-up) Make Koha::BackgroundJob use Koha::Object::JSONFields https://bugs.koha-community.org/show_bug.cgi?id=32371 Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Bug 32370 depends on bug 30943, which changed state. Bug 30943 Summary: Make background job classes use helpers https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30943 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145213|0 |1 is obsolete| | Attachment #145214|0 |1 is obsolete| | Attachment #145215|0 |1 is obsolete| | --- Comment #25 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 184914 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184914&action=edit Bug 32370: Unit tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #26 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 184915 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184915&action=edit Bug 32370: Provide a generic set of tools for JSON fields This patch introduces a new class for extending Koha::Object-based classes so they have reusable ways to deal with JSON encoded fields. As documented in the POD, classes can now ```perl use base qw(Koha::Object Koha::Object::JSONFields); ``` and will have the following methods available: * decode_json_field * set_encoded_json_field For convenience, the Koha::Auth::Identity::Provider class is tweaked to make use of this, and also to prove the behavior is correct (expected results, exceptions, encoding, etc). To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Object/JSONField.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #27 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 184916 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184916&action=edit Bug 32370: Adapt Koha::Auth::Identity::Provider This patch adapts the Koha::Auth::Identity::Provider class to make use of this, and also to prove the behavior is correct (expected results, exceptions, encoding, etc). To test: 1. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Auth/Identity/Provider.t \ t/db_dependent/api/v1/idp.t => SUCCESS: Identity provider related tests pass! 2. Apply this patches 3. Repeat 1 => SUCCESS: Tests still pass! 4. Sign off :-D Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |kyle@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184914|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184915|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184916|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #28 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 191053 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191053&action=edit Bug 32370: Unit tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #29 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 191054 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191054&action=edit Bug 32370: Provide a generic set of tools for JSON fields This patch introduces a new class for extending Koha::Object-based classes so they have reusable ways to deal with JSON encoded fields. As documented in the POD, classes can now ```perl use base qw(Koha::Object Koha::Object::JSONFields); ``` and will have the following methods available: * decode_json_field * set_encoded_json_field For convenience, the Koha::Auth::Identity::Provider class is tweaked to make use of this, and also to prove the behavior is correct (expected results, exceptions, encoding, etc). To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Object/JSONField.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #30 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 191055 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191055&action=edit Bug 32370: Adapt Koha::Auth::Identity::Provider This patch adapts the Koha::Auth::Identity::Provider class to make use of this, and also to prove the behavior is correct (expected results, exceptions, encoding, etc). To test: 1. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Auth/Identity/Provider.t \ t/db_dependent/api/v1/idp.t => SUCCESS: Identity provider related tests pass! 2. Apply this patches 3. Repeat 1 => SUCCESS: Tests still pass! 4. Sign off :-D Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #31 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 191056 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191056&action=edit Bug 32370: (QA follow-up) Add https to license url, use Test::NoWarnings Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed CC| |lucas@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #32 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|26.05.00 |26.05.00,25.11.02 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #33 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Thanks all, pushed to 25.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Needs documenting CC| |Laura.escamilla@bywatersolu | |tions.com --- Comment #34 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Not backporting to 25.05 as this is an enhancement. Happy to revisit if needed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 --- Comment #35 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- I totally forgot about this. Thanks folks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32370 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED --- Comment #36 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Nothing to add or edit in the Koha manual regarding this change. Please reopen and provide more information is this is not the case. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org