[Bug 26635] New: Expand authorised values in REST API call
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Bug ID: 26635 Summary: Expand authorised values in REST API call Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: REST API Assignee: agustinmoyano@theke.io Reporter: agustinmoyano@theke.io We need a way to embed expanded authorised values in a REST API call. It's specially needed when you have to show authorised values descriptions in tables that are fetched through api. The proposed solution adds to object's api definition (even embedded content) the following key: _authorised_values For each column that contains an authorised value, it will be reflected inside this property, with the authorised value expanded. So for example if we embed the authorised values of an item, the result from the API call could look like this: GET /api/v1/items/1 { "_authorised_values": { "location": { "authorised_value": "GEN", "category": "LOC", "id": 27, "imageurl": null, "lib": "General Stacks", "lib_opac": null } }, "acquisition_date": "2014-05-07", "acquisition_source": null, "biblio_id": 1, "item_id": 1, ... "location": "GEN", ... "uri": null, "withdrawn": 0, "withdrawn_date": null } -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |8179 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8179 [Bug 8179] Receiving multiple order lines at once -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |26636 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26636 [Bug 26636] New objects.find mojolicious helper -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Agustín Moyano <agustinmoyano@theke.io> 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=26635 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #1 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 111400 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111400&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111400|0 |1 is obsolete| | --- Comment #2 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 111401 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111401&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #3 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 111402 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111402&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111401|0 |1 is obsolete| | --- Comment #4 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 111411 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111411&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111402|0 |1 is obsolete| | --- Comment #5 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 111412 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111412&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111411|0 |1 is obsolete| | Attachment #111412|0 |1 is obsolete| | --- Comment #6 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 111413 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111413&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #7 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 111414 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=111414&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Nicolas Legrand <nicolas.legrand@bulac.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |nicolas.legrand@bulac.fr -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Nicolas Legrand <nicolas.legrand@bulac.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |severine.queune@bulac.fr -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com --- Comment #8 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Can't apply this patch in order to apply and test 8179 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #111413|0 |1 is obsolete| | Attachment #111414|0 |1 is obsolete| | --- Comment #9 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 112538 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=112538&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #10 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 112539 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=112539&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #11 from Agustín Moyano <agustinmoyano@theke.io> --- (In reply to Andrew Fuerste-Henry from comment #8)
Can't apply this patch in order to apply and test 8179
Hi Andrew, could you please try again? Thanks -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|8179 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8179 [Bug 8179] Receiving multiple order lines at once -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |david@davidnind.com --- Comment #12 from David Nind <david@davidnind.com> --- Patches apply, but running prove t/db_dependent/Koha/REST/Plugin/Objects.t fails (prove t/db_dependent/Koha/Object.t passes): prove t/db_dependent/Koha/REST/Plugin/Objects.t t/db_dependent/Koha/REST/Plugin/Objects.t .. Global symbol "$t" requires explicit package name (did you forget to declare "my $t"?) at t/db_dependent/Koha/REST/Plugin/Objects.t line 470. Execution of t/db_dependent/Koha/REST/Plugin/Objects.t aborted due to compilation errors. # Looks like your test exited with 255 before it could output anything. t/db_dependent/Koha/REST/Plugin/Objects.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 11/11 subtests Test Summary Report ------------------- t/db_dependent/Koha/REST/Plugin/Objects.t (Wstat: 65280 Tests: 0 Failed: 0) Non-zero exit status: 255 Parse errors: Bad plan. You planned 11 tests but ran 0. Files=1, Tests=0, 1 wallclock secs ( 0.03 usr 0.00 sys + 1.04 cusr 0.09 csys = 1.16 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|26636 | Depends on| |26636 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26636 [Bug 26636] New objects.find mojolicious helper -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |8179 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8179 [Bug 8179] Receiving multiple order lines at once -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 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=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #112538|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=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #112539|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=26635 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 116232 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116232&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t Sponsored-by: Virginia Tech Libraries Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #14 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 116233 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116233&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. Sponsored-by: Virginia Tech Libraries -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Andrew Fuerste-Henry <andrew@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=26635 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116232|0 |1 is obsolete| | --- Comment #15 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 116234 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116234&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t Sponsored-by: Virginia Tech Libraries Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116233|0 |1 is obsolete| | --- Comment #16 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 116235 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116235&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. Sponsored-by: Virginia Tech Libraries Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #17 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- This is interesting, and the code is nice to follow. I wonder a bit about the output form however... should we not expand AV's inline? So rather than: GET /api/v1/items/1 { "_authorised_values": { "location": { "authorised_value": "GEN", "category": "LOC", "id": 27, "imageurl": null, "lib": "General Stacks", "lib_opac": null } }, "acquisition_date": "2014-05-07", "acquisition_source": null, "biblio_id": 1, "item_id": 1, ... "location": "GEN", ... "uri": null, "withdrawn": 0, "withdrawn_date": null } We would do GET /api/v1/items/1 { "acquisition_date": "2014-05-07", "acquisition_source": null, "biblio_id": 1, "item_id": 1, ... "location": { "code": "GEN", "description": "General Stacks", "imageurl": null }, ... "uri": null, "withdrawn": 0, "withdrawn_date": null } and GET /public/items/1 { "acquisition_date": "2014-05-07", "acquisition_source": null, "biblio_id": 1, "item_id": 1, ... "location": { "code": "GEN", "description": null, "imageurl": null }, ... "uri": null, "withdrawn": 0, "withdrawn_date": null } This would more closely resemble how we return the data in the existing UI's. I'm not sure we need the full AV object expanded.. and I'm thinking if would be 'nicer' to access it directly rather than have to do a subsequent lookup in JS? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #17)
This would more closely resemble how we return the data in the existing UI's.
It is possible. But that means you will need to look at the attribute and determine if it is linked to an AV or not anyway. You could have a scalar or the attribute be an object. And if it is an object it could be some generated structure or an embed. We would still need to define a generic way to identify it is the case of an av-linked attribute. I'm not sure what's best, but having a separate lookup table felt cleaner to use. In the templates you would have proper objects that can be asked if they are av-linked. This certainly needs ideas. We are open to proposals, obviously!
I'm not sure we need the full AV object expanded.. and I'm thinking if would be 'nicer' to access it directly rather than have to do a subsequent lookup in JS?
I would go for an $av->to_api object, that names things better: lib/lib_opac => description, picking the right one according to the caller context. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116234|0 |1 is obsolete| | Attachment #116235|0 |1 is obsolete| | --- Comment #19 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 116592 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116592&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #20 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 116593 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=116593&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #18)
(In reply to Martin Renvoize from comment #17)
I'm not sure we need the full AV object expanded.. and I'm thinking if would be 'nicer' to access it directly rather than have to do a subsequent lookup in JS?
I would go for an $av->to_api object, that names things better: lib/lib_opac => description, picking the right one according to the caller context.
What do you think, Agustín? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (In reply to Tomás Cohen Arazi from comment #21)
(In reply to Tomás Cohen Arazi from comment #18)
(In reply to Martin Renvoize from comment #17)
I'm not sure we need the full AV object expanded.. and I'm thinking if would be 'nicer' to access it directly rather than have to do a subsequent lookup in JS?
I would go for an $av->to_api object, that names things better: lib/lib_opac => description, picking the right one according to the caller context.
What do you think, Agustín?
I'm onboard with this... embedding them and using to_api to make the naming clear.. I reckon we could be onto a winner -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #23 from Agustín Moyano <agustinmoyano@theke.io> --- (In reply to Martin Renvoize from comment #22)
(In reply to Tomás Cohen Arazi from comment #21)
(In reply to Tomás Cohen Arazi from comment #18)
(In reply to Martin Renvoize from comment #17)
I'm not sure we need the full AV object expanded.. and I'm thinking if would be 'nicer' to access it directly rather than have to do a subsequent lookup in JS?
I would go for an $av->to_api object, that names things better: lib/lib_opac => description, picking the right one according to the caller context.
What do you think, Agustín?
I'm onboard with this... embedding them and using to_api to make the naming clear.. I reckon we could be onto a winner
Now it is actually doing a "to_api" on avs when it does: + $avs = $self->_do_api_mapping($avs); and then +sub _do_api_mapping { + my ($self, $json_object) = @_; + # Rename attributes if there's a mapping + if ( $self->can('to_api_mapping') ) { + foreach my $column ( keys %{ $self->to_api_mapping } ) { + my $mapped_column = $self->to_api_mapping->{$column}; + if ( exists $json_object->{$column} + && defined $mapped_column ) + { + # key != undef + $json_object->{$mapped_column} = delete $json_object->{$column}; + } + elsif ( exists $json_object->{$column} + && !defined $mapped_column ) + { + # key == undef + delete $json_object->{$column}; + } + } + } return $json_object; } what we are currently lacking is "to_api_mapping" in AuthorisedValues -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #24 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Agustín Moyano from comment #23)
Now it is actually doing a "to_api" on avs when it does:
+ $avs = $self->_do_api_mapping($avs);
and then
+sub _do_api_mapping { + my ($self, $json_object) = @_; + # Rename attributes if there's a mapping + if ( $self->can('to_api_mapping') ) { + foreach my $column ( keys %{ $self->to_api_mapping } ) { + my $mapped_column = $self->to_api_mapping->{$column}; + if ( exists $json_object->{$column} + && defined $mapped_column ) + { + # key != undef + $json_object->{$mapped_column} = delete $json_object->{$column}; + } + elsif ( exists $json_object->{$column} + && !defined $mapped_column ) + { + # key == undef + delete $json_object->{$column}; + } + } + } return $json_object; }
what we are currently lacking is "to_api_mapping" in AuthorisedValues
Well, we certainly need that, but we also need to call ->to_api instead of the hand-crafted _do_api_mapping(). That's because we could want to call it with { public => 1 } and thus making the av representation dependent on the context. This example snippet from an hypothetical Koha::AuthorisedValue->to_api() might illustrate what I mean: my $json = $self->SUPER::to_api($params); my $description = $json->{description}; $description = $json->{description_opac} if $params->{public} and defined $json->{description_opac} and $json->{description_opac} ne ''; -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com QA Contact| |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=26635 Kyle M Hall <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=26635 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #116592|0 |1 is obsolete| | Attachment #116593|0 |1 is obsolete| | --- Comment #25 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 120698 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120698&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@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=26635 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120698|0 |1 is obsolete| | --- Comment #26 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 120699 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120699&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@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=26635 --- Comment #27 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 120700 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=120700&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Andrew Fuerste-Henry <andrew@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=26635 --- Comment #28 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 124268 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=124268&action=edit Bug 26635: Add tests for av-expand => 0 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #29 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- What about having it more generic? At least the naming to start. We will want to "expand" itemtypes as well for instance. Do we really need the whole Koha::AV object or do we finally only need the description? It's similar to the 'columns_to_str' methods I introduced on bug 27526 (yes Tomas, I know you told me already), where we want to have stringified values of object's attributes. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #30 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I have another question regards the output representation.. I've come round to the lookup table approach.. but shouldn't there be another level in there? { "_authorised_values": { "location": { "authorised_value": "GEN", "category": "LOC", "id": 27, "imageurl": null, "lib": "General Stacks", "lib_opac": null } }, "location": "GEN" } What about for searches.. it's unlikely all items will be in the 'GEN' location.. so wouldn't we actually need: { "_authorised_values": { "location": { "GEN": "General Stacks", "SHELVES": "Shelves" }, }, "location": "GEN" } Where the key is the AV code and the value is the appropriate description (based on public vs staff. Not sure we need any of the rest of the AV object detail honestly.. perhaps the imageurl later... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #31 from Agustín Moyano <agustinmoyano@theke.io> --- (In reply to Martin Renvoize from comment #30)
I have another question regards the output representation.. I've come round to the lookup table approach.. but shouldn't there be another level in there?
{ "_authorised_values": { "location": { "authorised_value": "GEN", "category": "LOC", "id": 27, "imageurl": null, "lib": "General Stacks", "lib_opac": null } }, "location": "GEN" }
What about for searches.. it's unlikely all items will be in the 'GEN' location.. so wouldn't we actually need:
{ "_authorised_values": { "location": { "GEN": "General Stacks", "SHELVES": "Shelves" }, }, "location": "GEN" }
Where the key is the AV code and the value is the appropriate description (based on public vs staff.
Not sure we need any of the rest of the AV object detail honestly.. perhaps the imageurl later...
Hi Martin, I did not think this to fill a search or select options (I thought better for that to have an authorised values endpoint). I designed the _authorised_values attribute to just bring the value of an authorised value in the original json. I'm not saying we shouldn't do it.. I just don't think it wise to bring every possible value here.. I've seen AV categories with hundreds of AV About what info of the AV should we bring, it's true that in most cases you would only need lib or lib_opac, but I didn't new if someone else for some reason would need any of the other info.. maybe we should sheep category too, to be able to populate a search or a select with the category options -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #32 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Hmmm, if that's the case I'm not sure I understand why there are tests for the search helper.. surely that will generally return multiple results? I'm vaguely feeling like a single page web app would actually grab the full AV list when it first needs it and cache it locally to use for subsequent uses.. but as Koha isn't at that point yet we're unlikely to be able to really utilise that methodology.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=28874 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #33 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #32)
Hmmm, if that's the case I'm not sure I understand why there are tests for the search helper.. surely that will generally return multiple results?
I'm vaguely feeling like a single page web app would actually grab the full AV list when it first needs it and cache it locally to use for subsequent uses.. but as Koha isn't at that point yet we're unlikely to be able to really utilise that methodology..
If you need to render a list of (say) items, you will need to be able to make the GET /items call return not only the 'damaged' attribute, but also a way to return the linked AV, for rendering purposes. The only thing that I'm not sure about this dev, is that (maybe) we want more fine-grained control of what objects should have their AVs expanded. For example, if we wanted the list of the items on a bib to display them grouped by the library they were checked out (I know, rare example heh): GET /biblio/123 x-koha-av-expand: true x-koha-embed: items.checkouts would mean we expand AVs for biblio, items, but also for the related checkouts (a probably expensive thing, as we don't have relationships in place for prefetching). I would prefer a syntax that allows us to do: GET /biblio/123 x-koha-embed: items+av,items.checkouts -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #34 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Tomás Cohen Arazi from comment #33)
(In reply to Martin Renvoize from comment #32)
Hmmm, if that's the case I'm not sure I understand why there are tests for the search helper.. surely that will generally return multiple results?
I'm vaguely feeling like a single page web app would actually grab the full AV list when it first needs it and cache it locally to use for subsequent uses.. but as Koha isn't at that point yet we're unlikely to be able to really utilise that methodology..
If you need to render a list of (say) items, you will need to be able to make the GET /items call return not only the 'damaged' attribute, but also a way to return the linked AV, for rendering purposes.
The only thing that I'm not sure about this dev, is that (maybe) we want more fine-grained control of what objects should have their AVs expanded. For example, if we wanted the list of the items on a bib to display them grouped by the library they were checked out (I know, rare example heh):
GET /biblio/123 x-koha-av-expand: true x-koha-embed: items.checkouts
would mean we expand AVs for biblio, items, but also for the related checkouts (a probably expensive thing, as we don't have relationships in place for prefetching).
I would prefer a syntax that allows us to do:
GET /biblio/123 x-koha-embed: items+av,items.checkouts
Where will this be documented? I think the biggest weakness with our API is a derth of documentation for end users. We have some much great functionality that most API users don't even know exists let alone how to use it! This is a great example. I'd really love to see at a minimum, this be documented on a wiki page. We could start there, and perhaps build an API User's Handbook, or even just an API User's Cookbook just like the SQL Reports and Javscript library pages. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #35 from Kyle M Hall <kyle@bywatersolutions.com> --- It would also be nice if it could be embedded in the self-documentation somehow. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |In Discussion -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #36 from Kyle M Hall <kyle@bywatersolutions.com> --- Started an API Users Guide on the Wiki: https://wiki.koha-community.org/wiki/Koha_REST_API_Users_Guide -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #37 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- What's next? Should we go with the proposed patches or think something else? (In reply to Jonathan Druart from comment #29)
What about having it more generic? At least the naming to start.
We will want to "expand" itemtypes as well for instance. Do we really need the whole Koha::AV object or do we finally only need the description?
It's similar to the 'columns_to_str' methods I introduced on bug 27526 (yes Tomas, I know you told me already), where we want to have stringified values of object's attributes.
This comment didn't get a reply, and I still think it's what we need here. A stringified version of codes seem sensible. It will work for AVs, branchcode, itemtypes, etc. However it will bring the problem of translatability (but same problem will appear with the current version). In a first step we only want to support AVs so this problem is for later. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #38 from Tomás Cohen Arazi <tomascohen@gmail.com> --- We name itemtype => item_type_id, branchcode => library_id, etc. We do that so we can do things like: x-koha-embed: item_type,library So we already have a generic way if doing it. You just add a method that returns the thing and add an embed definition on the API. The problem is, frameworks define how attributes are mapped to avs, and as such you cannot know in advance what you would need to embed. And even if you know in advance that some attribute is married to an av, it could happen that there isn't a match just because (of data). So maybe one item has a corresponding av, and the other doesn't, on a resultset. It felt (some time ago already) that it was better to return a structure in which you could query for the descriptions. I'm ok with whatever we decide, as long as it is practical and consistent. And we can use so we can push bug 8179 to master, which is hurting the sponsors for a year already. Bug 8179 needs some more polishing for sure, but uncertainty made it get stuck. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #39 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Trying to comment here in order to get this unstuck... maybe. As this is going to be used for bug 8179 which I very much would like to see moving :) If I understand Tomas last comment correctly the problem might be with result lists that contain objects that can have different sets of AV mappings... does that sound right? I am thinking about a list of order lines (aqorders) where every order line is linked to a fund... that can be linked to different or none AV for the 2 statistical values. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #40 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #39)
Trying to comment here in order to get this unstuck... maybe.
As this is going to be used for bug 8179 which I very much would like to see moving :)
If I understand Tomas last comment correctly the problem might be with result lists that contain objects that can have different sets of AV mappings... does that sound right?
I am thinking about a list of order lines (aqorders) where every order line is linked to a fund... that can be linked to different or none AV for the 2 statistical values.
Right, the idea is to make each object return (inside of it) done generic structure that lets the caller know how to find the av description. I'll get back to this next week, as we are responsible for having 8179 done, as the sponsor requested. We just need that people having different pov on the matter just say how they'd do it. And eventually vote something. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 George Williams (NEKLS) <george@nekls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |george@nekls.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #41 from Lucas Gass <lucas@bywatersolutions.com> --- Would love to see this get unstuck, authorized values descriptions from the REST API would be really handy! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #42 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm still coming back around to comment #32, in particular:
a single page web app would actually grab the full AV list when it first needs it and cache it locally to use for subsequent uses..
I think for efficiency we're going to really struggle with embedding AV's dynamically in the results. I'm thinking more and more we need to focus on having AV and Framework routes and then building a JS equivalent to the RecordProcessor pattern for the UI. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #43 from Tomás Cohen Arazi <tomascohen@gmail.com> --- One thing to consider is we should (ideally) not change the API often so devs can rely on it. That said what if we streamline the data structure (i.e. do not return the AV object, but just the description, making it context (opac?)) and try this. Once we find a better pattern in real-life uses, we can flag this as 'slow' and propose a different one, while keeping it for some cases in which it really doesn't hurt performance. Bug 8179 is a real example of using this feature without any noticeable overhead. So maybe when this is *not enough* we look for other approaches. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #44 from Lucas Gass <lucas@bywatersolutions.com> --- *** Bug 31683 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #45 from David Cook <dcook@prosentient.com.au> --- Another wrinkle could be translations for authorised values: https://lists.katipo.co.nz/public/koha/2021-December/057052.html -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #46 from David Cook <dcook@prosentient.com.au> --- In any case, I reckon backwards compatibility is important, especially since we're still using "v1" for the API, so I don't think we can change the existing "location" field. Whether we embed the data or provide a lookup reference, I think it needs to be as a separate field. On non-Koha projects where I've needed to maintain backwards compatibility, I've added fields like "location_description". -- Sometimes, I think that authorised values should be removed from item data, and replaced with proper foreign key references. Location is an important field which by rights deserves its own database table I think. Of course, that data migration is easier said than done... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #47 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to David Cook from comment #46)
In any case, I reckon backwards compatibility is important, especially since we're still using "v1" for the API, so I don't think we can change the existing "location" field.
Agreed!
On non-Koha projects where I've needed to maintain backwards compatibility, I've added fields like "location_description".
Right. We also need the category it is linked to, do we can draw things like drop-downs.
Sometimes, I think that authorised values should be removed from item data, and replaced with proper foreign key references.
Location is an important field which by rights deserves its own database table I think.
Locations, collections... We've done that with credit/debit tired recently.
Of course, that data migration is easier said than done...
Generally speaking, the problem is lack of infinite time. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|agustinmoyano@theke.io |tomascohen@gmail.com Status|In Discussion |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #48 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Make the REST API returns a "_str" key (generated only if you requested it) which would contains the stringified values { _str: { attr: "stringified_value" }, attr: "coded_value" } JS code would be let string_to_display = json._str.attr || json.attr Not sure why this is still stuck. I needed it for ERM and ended with retrieving the AVs from the TT plugin, serialized to JSON and cached them. The main problem is that we don't even have a route for AVs. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #49 from Lucas Gass <lucas@bywatersolutions.com> --- Tomas, This is marked as 'ASSIGNED', is there work to do or is ready for testing? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_22_11_candidate --- Comment #50 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Lucas Gass from comment #49)
Tomas,
This is marked as 'ASSIGNED', is there work to do or is ready for testing?
We need to rework this a bit. I marked it for inclusion in 22.11 and will submit a revisited version ASAP. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Expand authorised values in |Expand coded values in REST |REST API call |API call -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #124268|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=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120699|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=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #120700|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=26635 --- Comment #51 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 142761 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142761&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@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=26635 --- Comment #52 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 142762 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142762&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Andrew Fuerste-Henry <andrew@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=26635 --- Comment #53 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 142763 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142763&action=edit Bug 26635: Add tests for av-expand => 0 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #54 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Rebased so it works and devs can take a look. I will refactor this into 'coded values' (not just AV) and will stick to a similar approach to Agustin's. While I think just adding <attribute>_str to the response might suffice some use cases, I think having a lookup table will be better for knowing things like 'where to pick the values for a dropdown' etc. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #55 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 142776 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142776&action=edit Bug 26635: AV expansion should ackowledge $params->{public} This patch simplifies the original implementation so it is more readable, removing extra loops and methods and just dealing with the coded values inline on the existing loops. It also adds non-public attributes removal from the original list, to address the changes introduced by bug 28948. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|Trivial patch |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142776|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=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142761|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=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142762|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=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142763|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=26635 --- Comment #56 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 142792 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142792&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@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=26635 --- Comment #57 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 142793 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142793&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Andrew Fuerste-Henry <andrew@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=26635 --- Comment #58 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 142794 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142794&action=edit Bug 26635: Add tests for av-expand => 0 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #59 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 142795 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142795&action=edit Bug 26635: AV expansion should ackowledge $params->{public} This patch simplifies the original implementation so it is more readable, removing extra loops and methods and just dealing with the coded values inline on the existing loops. It also adds non-public attributes removal from the original list, to address the changes introduced by bug 28948. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #60 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 142796 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142796&action=edit Bug 26635: Refined data structure and methods This patch makes the returned data structure be simpler: _authorised_values => { attribute_1 => { category => 'some_category_name', description => 'description' } } The description is sensible to context, so if public => 1 is passed, then lib_opac is passed, and lib is returned otherwise. Whenever we add language to the combo, we will add it to the implementation. Tests are adjusted accordingly, also to reflect the public => 1 use case. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #61 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- So.. for my own sanity.. the final structure is this right: GET /api/v1/items/1 { "_authorized_values": { "location": { "category": "LOC", "description": "General Stacks", } }, "acquisition_date": "2014-05-07", "acquisition_source": null, "biblio_id": 1, "item_id": 1, ... "location": "GEN", ... "uri": null, "withdrawn": 0, "withdrawn_date": null } GET /api/v1/items [ { "_authorized_values": { "location": { "category": "LOC", "description": "General Stacks", } }, "acquisition_date": "2014-05-07", "acquisition_source": null, "biblio_id": 1, "item_id": 1, ... "location": "GEN", ... "uri": null, "withdrawn": 0, "withdrawn_date": null }, { "_authorized_values": { "location": { "category": "LOC", "description": "Birchwood Shelves", } }, "acquisition_date": "2017-02-14", "acquisition_source": null, "biblio_id": 2, "item_id": 12, ... "location": "BIR", ... "uri": null, "withdrawn": 0, "withdrawn_date": null } ] So.. an embedded _authorized_values structure per individual object (not at a top level.. as we can't do that for things like biblios where the framework governs stuff..) We keep the lookup as '_authorized' so we know it's an authorized value (not another coded lookup).. and we include 'category' so we can know the link between the field and which av category we're looking at (for later lookups should we want to produce a pick list).. and then we have the interface specific return for description. I'm no against also including image url if we think that's helpful.. I could see if being so.. but it also feels fairly specific to only a minority of AV cases.. so maybe not too.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142792|0 |1 is obsolete| | --- Comment #62 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 142801 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142801&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142793|0 |1 is obsolete| | --- Comment #63 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 142802 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142802&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142794|0 |1 is obsolete| | --- Comment #64 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 142803 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142803&action=edit Bug 26635: Add tests for av-expand => 0 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142795|0 |1 is obsolete| | --- Comment #65 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 142804 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142804&action=edit Bug 26635: AV expansion should ackowledge $params->{public} This patch simplifies the original implementation so it is more readable, removing extra loops and methods and just dealing with the coded values inline on the existing loops. It also adds non-public attributes removal from the original list, to address the changes introduced by bug 28948. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142796|0 |1 is obsolete| | --- Comment #66 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 142805 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142805&action=edit Bug 26635: Refined data structure and methods This patch makes the returned data structure be simpler: _authorised_values => { attribute_1 => { category => 'some_category_name', description => 'description' } } The description is sensible to context, so if public => 1 is passed, then lib_opac is passed, and lib is returned otherwise. Whenever we add language to the combo, we will add it to the implementation. Tests are adjusted accordingly, also to reflect the public => 1 use case. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #67 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- All works for me.. Signing Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #68 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- As said earlier (comment 29), IMO this should not be specific to AVs. It (the new hash we are injecting) needs to reflect that it's a stringified version of a coded field. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #69 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I though Tomas had answered your comment already Jonathan. I think the key piece is that he wants both the string and the av category so that one can produce the correct pick lists when needed. That of course also assumes we have and av endpoint and but it does let us off adding framework endpoints at this time. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #70 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Then we could use the following structure { _str: { location: { str: "General Stacks", type: "authorised_values" }, itemtype: { str: "Books", type: "itemtypes" } }, location: "GEN", itemtype: "BK", }
From a dev pov it will be easier to const location = r._str.location.str || r.location const itemtype = r._str.itemtype.str || r.itemtype
than to *know* that location is in _authorised_values and itemtype in _itemtypes! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #71 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK, with that example I understand better where you were going with it.. makes sense to me. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #72 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #70)
Then we could use the following structure { _str: { location: { str: "General Stacks", type: "authorised_values" }, itemtype: { str: "Books", type: "itemtypes" } }, location: "GEN", itemtype: "BK", }
From a dev pov it will be easier to const location = r._str.location.str || r.location const itemtype = r._str.itemtype.str || r.itemtype
than to *know* that location is in _authorised_values and itemtype in _itemtypes!
I like this. Was thinking the same way after Martin's recap highlighted I didn't approach the coded fields approach we chatted about. Follow-ups welcome or wait for me while I recover from this virus. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142801|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=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142802|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=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142803|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=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142804|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=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142805|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=26635 --- Comment #73 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 143039 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143039&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@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=26635 --- Comment #74 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 143040 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143040&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Andrew Fuerste-Henry <andrew@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=26635 --- Comment #75 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 143041 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143041&action=edit Bug 26635: Add tests for av-expand => 0 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #76 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 143042 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143042&action=edit Bug 26635: AV expansion should ackowledge $params->{public} This patch simplifies the original implementation so it is more readable, removing extra loops and methods and just dealing with the coded values inline on the existing loops. It also adds non-public attributes removal from the original list, to address the changes introduced by bug 28948. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #77 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 143043 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143043&action=edit Bug 26635: Refined data structure and methods This patch makes the returned data structure be simpler: _str => { attribute_1 => { category => 'some_category_name', str => 'description', type => 'av' }, ... } The description is sensible to context, so if public => 1 is passed, then lib_opac is passed, and lib is returned otherwise. Whenever we add language to the combo, we will add it to the implementation. Tests are adjusted accordingly, also to reflect the public => 1 use case. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #78 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 143044 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143044&action=edit Bug 26635: Move expand syntax to x-koha-embed Instead of a global av-expand flag (through a header) this patch proposes to allow specifying +av_expand at the x-koha-embed header level. This allows a more fine-grained control on what objects get avs expanded. e.g.: GET /patrons/123 x-koha-expand: +av_expand,checkouts.item+av_expand => { "_str": { "city": { "str": "Córdoba", ... } }, ... "checkouts": [ { ..., "item": { "_str": { "not_for_loan_status": { "str": "Reference material", ... }, ... }, ... } } ] } To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/Koha/REST/Plugin/Query.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 2. 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=26635 --- Comment #79 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I haven't fully recovered, but... I get bored so... I have submitted a revisited patch for revisiting the data structure and methods, and then another interesting follow-up for getting rid of the x-koha-expand-av flag, and just add a syntax for +av_expand on the x-koha-embed, which makes a lot more sense and provides a fine-grained approach instead of an all-or-nothing one. I implemented things mostly as Jonathan proposed (I only added an obvious 'category' attribute). We'll see how we later add 'str' counterpart for other types of things. Please test! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143039|0 |1 is obsolete| | --- Comment #80 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143073 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143073&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143040|0 |1 is obsolete| | --- Comment #81 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143074 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143074&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143041|0 |1 is obsolete| | --- Comment #82 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143075 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143075&action=edit Bug 26635: Add tests for av-expand => 0 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143042|0 |1 is obsolete| | --- Comment #83 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143076 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143076&action=edit Bug 26635: AV expansion should ackowledge $params->{public} This patch simplifies the original implementation so it is more readable, removing extra loops and methods and just dealing with the coded values inline on the existing loops. It also adds non-public attributes removal from the original list, to address the changes introduced by bug 28948. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143043|0 |1 is obsolete| | --- Comment #84 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143077 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143077&action=edit Bug 26635: Refined data structure and methods This patch makes the returned data structure be simpler: _str => { attribute_1 => { category => 'some_category_name', str => 'description', type => 'av' }, ... } The description is sensible to context, so if public => 1 is passed, then lib_opac is passed, and lib is returned otherwise. Whenever we add language to the combo, we will add it to the implementation. Tests are adjusted accordingly, also to reflect the public => 1 use case. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143044|0 |1 is obsolete| | --- Comment #85 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143078 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143078&action=edit Bug 26635: Move expand syntax to x-koha-embed Instead of a global av-expand flag (through a header) this patch proposes to allow specifying +av_expand at the x-koha-embed header level. This allows a more fine-grained control on what objects get avs expanded. e.g.: GET /patrons/123 x-koha-expand: +av_expand,checkouts.item+av_expand => { "_str": { "city": { "str": "Córdoba", ... } }, ... "checkouts": [ { ..., "item": { "_str": { "not_for_loan_status": { "str": "Reference material", ... }, ... }, ... } } ] } To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/Koha/REST/Plugin/Query.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 2. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #86 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143079 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143079&action=edit Bug 26635: (follow-up) Add optional support for `:` as the modifier This patch adds optional support for using `:` as the modifier for invoking the 'count' method for relations as discussed. We retain support for `+` for backward compatability to prevent a breaking change. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #87 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143080 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143080&action=edit Bug 26635: (QA follow-up) Remove test for x-koha-av-expand header This patch updates the last remaining test that mentioned the now defunkt x-koha-av-expand header. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off --- Comment #88 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm loving this… signing off. The one very, very minor niggle… I'm wondering if `+av_expand` is right; With us switching to _str to generalize it, perhaps the key string should be `+str_expand` or `+code_expand` -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #89 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Martin Renvoize from comment #88)
I'm loving this… signing off.
The one very, very minor niggle… I'm wondering if `+av_expand` is right; With us switching to _str to generalize it, perhaps the key string should be `+str_expand` or `+code_expand`
Oh I didn't mention that... On our chat we agreed that `attribute+count` was not the best syntax, as it is not 'adding', and that we should push for `attribute:count`. That said, I feel like we will end up implementing something like: `item_type_id:str` or `pickup_location_id:str`. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143073|0 |1 is obsolete| | Attachment #143074|0 |1 is obsolete| | Attachment #143075|0 |1 is obsolete| | Attachment #143076|0 |1 is obsolete| | Attachment #143077|0 |1 is obsolete| | Attachment #143078|0 |1 is obsolete| | Attachment #143079|0 |1 is obsolete| | Attachment #143080|0 |1 is obsolete| | --- Comment #90 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143127 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143127&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 --- Comment #91 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143128 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143128&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 --- Comment #92 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143129 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143129&action=edit Bug 26635: Add tests for av-expand => 0 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 --- Comment #93 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143130 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143130&action=edit Bug 26635: AV expansion should ackowledge $params->{public} This patch simplifies the original implementation so it is more readable, removing extra loops and methods and just dealing with the coded values inline on the existing loops. It also adds non-public attributes removal from the original list, to address the changes introduced by bug 28948. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 --- Comment #94 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143131 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143131&action=edit Bug 26635: Refined data structure and methods This patch makes the returned data structure be simpler: _str => { attribute_1 => { category => 'some_category_name', str => 'description', type => 'av' }, ... } The description is sensible to context, so if public => 1 is passed, then lib_opac is passed, and lib is returned otherwise. Whenever we add language to the combo, we will add it to the implementation. Tests are adjusted accordingly, also to reflect the public => 1 use case. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 --- Comment #95 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143132 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143132&action=edit Bug 26635: Move expand syntax to x-koha-embed Instead of a global av-expand flag (through a header) this patch proposes to allow specifying +av_expand at the x-koha-embed header level. This allows a more fine-grained control on what objects get avs expanded. e.g.: GET /patrons/123 x-koha-expand: +av_expand,checkouts.item+av_expand => { "_str": { "city": { "str": "Córdoba", ... } }, ... "checkouts": [ { ..., "item": { "_str": { "not_for_loan_status": { "str": "Reference material", ... }, ... }, ... } } ] } To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/Koha/REST/Plugin/Query.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 2. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 --- Comment #96 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143133 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143133&action=edit Bug 26635: (follow-up) Add optional support for `:` as the modifier This patch adds optional support for using `:` as the modifier for invoking the 'count' method for relations as discussed. We retain support for `+` for backward compatability to prevent a breaking change. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 --- Comment #97 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143134 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143134&action=edit Bug 26635: (QA follow-up) Remove test for x-koha-av-expand header This patch updates the last remaining test that mentioned the now defunkt x-koha-av-expand header. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 --- Comment #98 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- 1. "Add optional support for `:` as the modifier" We should document that. Why do we have it here actually? Shouldn't it be on its own bug? 2. Cannot we come with something better than "_str" and "str"? 3. We still have specific reference to "av" when I think we should focus on "stringify coded values" as a general concept. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #99 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #98)
1. "Add optional support for `:` as the modifier" We should document that. Why do we have it here actually? Shouldn't it be on its own bug?
I agree.
2. Cannot we come with something better than "_str" and "str"?
I agree with that, you proposed. You don't like?
3. We still have specific reference to "av" when I think we should focus on "stringify coded values" as a general concept.
AV expansion can be expensive. And so other strings. The idea of ':', was to later add <attribute>:str, so things that are not linked to av, might be generated on demand as well. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #100 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #99)
(In reply to Jonathan Druart from comment #98)
1. "Add optional support for `:` as the modifier" We should document that. Why do we have it here actually? Shouldn't it be on its own bug?
I agree.
2. Cannot we come with something better than "_str" and "str"?
I agree with that, you proposed. You don't like?
Yes, the idea, but I was expecting someone to tell me that "str" is ugly (especially when appearing twice :D) The second one can be 'str', not the first one.
3. We still have specific reference to "av" when I think we should focus on "stringify coded values" as a general concept.
AV expansion can be expensive. And so other strings. The idea of ':', was to later add <attribute>:str, so things that are not linked to av, might be generated on demand as well.
In my mind consumers don't want to ask for each attributes to be expanded, but one "expand everything" (actually "stringify everything") switch. I may be wrong. Maybe we are mixing "expand" and "stringify" into the same concept and we should not. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Kyle M Hall <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=26635 --- Comment #101 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143276 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143276&action=edit Bug 26635: (QA follow-up) Rename key _str to _strings -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #102 from Kyle M Hall <kyle@bywatersolutions.com> ---
Yes, the idea, but I was expecting someone to tell me that "str" is ugly (especially when appearing twice :D) The second one can be 'str', not the first one.
Fix in the latest followup!
In my mind consumers don't want to ask for each attributes to be expanded, but one "expand everything" (actually "stringify everything") switch. I may be wrong.
I think it depends on the use case. For speed, it is good to specify each expansion you'd like. Adding an 'expand all' switch would be a nice enhancement on top of this, but I don't think it's necessary for this to move forward. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32118 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32118 [Bug 32118] Clarify expansion/embed modifiers -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143127|0 |1 is obsolete| | --- Comment #103 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143354 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143354&action=edit Bug 26635: Add tests Add tests in t/db_dependent/Koha/Object.t and t/db_dependent/Koha/REST/Plugin/Objects.t Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143128|0 |1 is obsolete| | --- Comment #104 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143355 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143355&action=edit Bug 26635: Expand authorised values in to_api method This patch adds the posibility to expand authorised values when to_api method is called. The classes where authorised values should expand must implememnt the _fetch_authorised_values method, and must return a hash like the following { column_name => Koha::AuthorisedValue->unblessed ... } This patch will be used in bug 8179, so please test there. Sponsored-by: Virginia Polytechnic Institute and State University Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143129|0 |1 is obsolete| | --- Comment #105 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143356 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143356&action=edit Bug 26635: Add tests for av-expand => 0 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143130|0 |1 is obsolete| | --- Comment #106 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143357 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143357&action=edit Bug 26635: AV expansion should ackowledge $params->{public} This patch simplifies the original implementation so it is more readable, removing extra loops and methods and just dealing with the coded values inline on the existing loops. It also adds non-public attributes removal from the original list, to address the changes introduced by bug 28948. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143131|0 |1 is obsolete| | --- Comment #107 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143358 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143358&action=edit Bug 26635: Refined data structure and methods This patch makes the returned data structure be simpler: _str => { attribute_1 => { category => 'some_category_name', str => 'description', type => 'av' }, ... } The description is sensible to context, so if public => 1 is passed, then lib_opac is passed, and lib is returned otherwise. Whenever we add language to the combo, we will add it to the implementation. Tests are adjusted accordingly, also to reflect the public => 1 use case. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143132|0 |1 is obsolete| | --- Comment #108 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143359 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143359&action=edit Bug 26635: Move expand syntax to x-koha-embed Instead of a global av-expand flag (through a header) this patch proposes to allow specifying +av_expand at the x-koha-embed header level. This allows a more fine-grained control on what objects get avs expanded. e.g.: GET /patrons/123 x-koha-expand: +av_expand,checkouts.item+av_expand => { "_str": { "city": { "str": "Córdoba", ... } }, ... "checkouts": [ { ..., "item": { "_str": { "not_for_loan_status": { "str": "Reference material", ... }, ... }, ... } } ] } To test: 1. Run: $ kshell k$ prove t/db_dependent/Koha/Object.t \ t/Koha/REST/Plugin/Query.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 2. Sign off :-D Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.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=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143134|0 |1 is obsolete| | --- Comment #109 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143360 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143360&action=edit Bug 26635: (QA follow-up) Remove test for x-koha-av-expand header This patch updates the last remaining test that mentioned the now defunkt x-koha-av-expand header. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143276|0 |1 is obsolete| | --- Comment #110 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143361 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143361&action=edit Bug 26635: (QA follow-up) Rename key _str to _strings Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #111 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 143362 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143362&action=edit Bug 26635: (follow-up) Rename av_expand to strings This patch updates the `av_expand` occurences to `strings` to match the change of the return structure of `_strings`. We replace `+av_expand` in the headers with `+strings`, the expected object method name from `api_av_mapping` to `api_strings_mapping` and the internal hash key from `av_expand` to `strings`. Test plan 1) Run the included unit tests.. all should still pass. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143133|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=26635 --- Comment #112 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- As discussed.. I updated the embed call to also refer to 'strings' as aposed to 'av_expand'. Passed QA now :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 --- Comment #113 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |33161 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33161 [Bug 33161] Implement +strings for GET /items and /items/:item_id -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |22440 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22440 [Bug 22440] Improve ILL page performance by moving to server side filtering -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26635 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34211 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34211 [Bug 34211] Add +strings for GET /api/v1/biblios/:biblio_id/items -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org