Summary: | Authority objects missing mapping | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Component: | REST API | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Status: | CLOSED FIXED | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
Severity: | normal | ||
Priority: | P5 - low | CC: | jonathan.druart, martin.renvoize, nick, pedro.amorim, tomascohen |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
23.11.00,23.05.02
|
|
Circulation function: | |||
Bug Depends on: | 31790 | ||
Bug Blocks: | 33971 | ||
Attachments: |
Bug 33996: Add missing authority objects mapping
Bug 33996: Add missing authority objects mapping Bug 33996: Add missing authority objects mapping Bug 33996: (QA follow-up) Fix mappings to match guidelines |
Description
Tomás Cohen Arazi (tcohen)
2023-06-13 12:59:47 UTC
Created attachment 152293 [details] [review] Bug 33996: Add missing authority objects mapping This patch adds a minimal mapping (i.e. some things get excluded from the API objects for now). It makes some attributes consistent with the current API names, and the rest can be added later when they become needed and we discuss a proper name. But right now is a mess :-/ To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/authorities.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 152341 [details] [review] Bug 33996: Add missing authority objects mapping This patch adds a minimal mapping (i.e. some things get excluded from the API objects for now). It makes some attributes consistent with the current API names, and the rest can be added later when they become needed and we discuss a proper name. But right now is a mess :-/ To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/authorities.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Sam Lau <samalau@gmail.com> You were against "_on". We have: Koha/Authority.pm: modification_time => 'updated_on', Koha/Biblio/ItemGroup.pm: updated_on => 'modification_date' api/v1/swagger/definitions/patron.yaml: updated_on: api/v1/swagger/definitions/return_claim.yaml: updated_on: Can we make a decision maybe? Created attachment 152688 [details] [review] Bug 33996: Add missing authority objects mapping This patch adds a minimal mapping (i.e. some things get excluded from the API objects for now). It makes some attributes consistent with the current API names, and the rest can be added later when they become needed and we discuss a proper name. But right now is a mess :-/ To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/authorities.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 152689 [details] [review] Bug 33996: (QA follow-up) Fix mappings to match guidelines The date field guidelines are recorded on the wiki: https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#SWAGGER1.3.4.1_date.2Fdatetime.2Ftimestamp_fields Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Pushed to master for 23.11. Nice work everyone, thanks! Should we fix the '*_on' occurrences on a separate bug report or is it too late now? You mean the *_on elsewhere in the codebase.. yeah, I think it's a good idea to work towards the guidelines on another bug. I'd love to see that done codebase wide, but I'm also worried that may constitute a V2 change for the API The other problem we had with "_date" is that we are using it regardless the datatype (datetime, date, timestamp) and could lead to confusion. Just noting here as it was the reason why I didn't like it (but not the place to discuss this anyway). Thanks for all the hard work! Pushed to 23.05.x for the next release Missing bug 31794 in 22.11.x. Doesn't apply. Not pushing. |