[Bug 28701] New: primary_contact_method not part of the REST API spec
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Bug ID: 28701 Summary: primary_contact_method not part of the REST API spec Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart+koha@gmail.com Bug 11879 added borrowers.primary_contact_method but not the REST API spec changes. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |11879 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11879 [Bug 11879] Add a new field to patron record: main contact method -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Bug 28701 depends on bug 11879, which changed state. Bug 11879 Summary: Add a new field to patron record: main contact method https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11879 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Noémie Labine <noemie.labine@collecto.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |noemie.labine@collecto.ca --- Comment #1 from Noémie Labine <noemie.labine@collecto.ca> --- +1 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 --- Comment #2 from Alexis Ripetti <alexis.ripetti@inLibro.com> --- Created attachment 136124 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=136124&action=edit Bug 28701 - Adding primary_contact_method to the REST API spec -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Alexis Ripetti <alexis.ripetti@inLibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alexis.ripetti@inLibro.com --- Comment #3 from Alexis Ripetti <alexis.ripetti@inLibro.com> --- I tried to test this patch but my master cannot access the API because of modules dependencies. I test this patch in 21.05 and it works. If anyone wants to try, you just need to be connected in the OPAC and to call a endpoint involving patron. Like `/api/v1/patrons`. You may need to check your permissions. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Andrew Fuerste-Henry <andrewfh@dubcolib.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrewfh@dubcolib.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Assignee|koha-bugs@lists.koha-commun |alexis.ripetti@inLibro.com |ity.org | Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #136124|0 |1 is obsolete| | --- Comment #4 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 143972 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143972&action=edit Bug 28701 - Adding primary_contact_method to the REST API spec This patch adds the primary_contact_method field to the api response for patrons To test: 1 - GET http://localhost:8081/api/v1/patrons/51 2 - Look for primary_contact_method, it's not there 3 - Apply patch 4 - GET http://localhost:8081/api/v1/patrons/51 5 - Look for primary_contact_method, it's not there Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I think it should be an enum -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #5)
I think it should be an enum
`primary_contact_method` varchar(45) DEFAULT NULL COMMENT 'useful for reporting purposes', Scope creep ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> ---
This patch adds the primary_contact_method field to the api response for patrons Please look at step 5 of your test plan. Contradiction :)
To test: 4 - GET http://localhost:8081/api/v1/patrons/51 5 - Look for primary_contact_method, it's not there
It was not there indeed. But it should be! Adding a follow-up. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #143972|0 |1 is obsolete| | --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 145715 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145715&action=edit Bug 28701 - Adding primary_contact_method to the REST API spec This patch adds the primary_contact_method field to the api response for patrons To test: 1 - GET http://localhost:8081/api/v1/patrons/51 2 - Look for primary_contact_method, it's not there 3 - Apply patch 4 - GET http://localhost:8081/api/v1/patrons/51 5 - Look for primary_contact_method, it's not there Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 145716 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=145716&action=edit Bug 28701: (QA follow-up) Make primary_contact_method visible Via to_api_mapping. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #6)
(In reply to Tomás Cohen Arazi from comment #5)
I think it should be an enum
`primary_contact_method` varchar(45) DEFAULT NULL COMMENT 'useful for reporting purposes',
Scope creep ?
If you git grep primary_contact_method, you will notice that the possible values there are hardcoded: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: <select id="borrower_primary_contact_method" name ="borrower_primary_contact_method"> koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: [% IF ( borrower.primary_contact_method = = 'phone' ) %] koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: [% IF ( borrower.primary_contact_method = = 'phonepro' ) %] koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: [% IF ( borrower.primary_contact_method = = 'mobile' ) %] koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: [% IF ( borrower.primary_contact_method = = 'email' ) %] koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: [% IF ( borrower.primary_contact_method = = 'emailpro' ) %] koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: [% IF ( borrower.primary_contact_method = = 'fax' ) %] when we write endpoint specs, we want them to be future proof (as much as possible). So I'm not expecting this bug to update the DB structure, but either: (a) make the type of the 'new API object attribute' be an enum with those values (b) choose better names for those hardcoded values, and list them as an enum IN THE SPEC and make to_api() handle the name mapping. I'd prefer (b). Regarding the follow-up, it does the job, but we've better only map things that need mapping (i.e. mapping to null removes it from the response, mapping to a value makes the API translate the name, not mapping makes the API return it as-is. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #11 from Lucas Gass <lucas@bywatersolutions.com> --- (In reply to Tomás Cohen Arazi from comment #10)
(In reply to Marcel de Rooy from comment #6)
(In reply to Tomás Cohen Arazi from comment #5)
I think it should be an enum
`primary_contact_method` varchar(45) DEFAULT NULL COMMENT 'useful for reporting purposes',
Scope creep ?
If you git grep primary_contact_method, you will notice that the possible values there are hardcoded:
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: <select id="borrower_primary_contact_method" name ="borrower_primary_contact_method"> koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: [% IF ( borrower.primary_contact_method = = 'phone' ) %] koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: [% IF ( borrower.primary_contact_method = = 'phonepro' ) %] koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: [% IF ( borrower.primary_contact_method = = 'mobile' ) %] koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: [% IF ( borrower.primary_contact_method = = 'email' ) %] koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: [% IF ( borrower.primary_contact_method = = 'emailpro' ) %] koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: [% IF ( borrower.primary_contact_method = = 'fax' ) %]
when we write endpoint specs, we want them to be future proof (as much as possible). So I'm not expecting this bug to update the DB structure, but either:
(a) make the type of the 'new API object attribute' be an enum with those values (b) choose better names for those hardcoded values, and list them as an enum IN THE SPEC and make to_api() handle the name mapping.
I'd prefer (b).
Regarding the follow-up, it does the job, but we've better only map things that need mapping (i.e. mapping to null removes it from the response, mapping to a value makes the API translate the name, not mapping makes the API return it as-is.
I'd like to point out that many librarians don't like the fact that these are hard coded values. I've written an enhancement to make them customizable via authorized values in Bug 31498. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Christopher Brannon <cbrannon@cdalibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |cbrannon@cdalibrary.org --- Comment #12 from Christopher Brannon <cbrannon@cdalibrary.org> --- Looking forward to the API being updated to include the contact method. :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|alexis.ripetti@inLibro.com | Assignee|alexis.ripetti@inLibro.com |koha-bugs@lists.koha-commun | |ity.org -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 --- Comment #13 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 188010 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=188010&action=edit Bug 28701: Adding primary_contact_method to the REST API This patch adds the primary_contact_method field to the api response for patrons To test: 1 - GET http://localhost:8081/api/v1/patrons/51 2 - Look for primary_contact_method, it's not there 3 - Apply patch 4 - GET http://localhost:8081/api/v1/patrons/51 5 - Look for primary_contact_method, it's not there Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 --- Comment #14 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 188011 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=188011&action=edit Bug 28701: (QA follow-up) Make primary_contact_method visible Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|Small patch |Trivial patch Status|Failed QA |Needs Signoff --- Comment #15 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- This patch adds the primary_contact_method field to the api response for patrons To test: 1 - GET http://localhost:8081/api/v1/patrons/51 2 - Look for primary_contact_method, it's not there 3 - Apply patch 4 - GET http://localhost:8081/api/v1/patrons/51 5 - Look for primary_contact_method, it's not there Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145715|0 |1 is obsolete| | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #145716|0 |1 is obsolete| | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |alexis.ripetti@inLibro.com |ity.org | Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #16 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- I rewrote the patches so they apply on (much) newer main. I kept the code attribution. I also made a small change to Marcel's follow-up, inline and fixed Alexis' commit title (didn't pass the QA script check). This is good to go. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Brandon <brandon@wwcrld.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |brandon@wwcrld.org --- Comment #17 from Brandon <brandon@wwcrld.org> --- +1 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #18 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- xt/api.t needs to be adjusted: kohadev-koha@kohadevbox:koha(main)$ prove xt/api.t xt/api.t .. 6/8 # Failed test 'maxLength tests for patron' # at xt/api.t line 264. # got: '1' # expected: '0' # $VAR1 = 'patron.primary_contact_method should have maxLength=45 in api spec'; # Looks like you failed 1 test of 7. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |caroline.cyr-la-rose@inlibr | |o.com Assignee|alexis.ripetti@inLibro.com |koha-bugs@lists.koha-commun | |ity.org --- Comment #19 from Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> --- Changed the assignee. Alexis no longer works here. Tomás, is this yours? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 --- Comment #20 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Created attachment 191018 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191018&action=edit Bug 28701: Add mexlength to the API spec -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 --- Comment #21 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Caroline Cyr La Rose from comment #19)
Changed the assignee. Alexis no longer works here. Tomás, is this yours?
Sure. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 --- Comment #22 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 26.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|26.05.00 |26.05.00,25.11.02 released in| | Status|Pushed to main |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 --- Comment #23 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Thanks all, pushed to 25.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701 Laura Escamilla <Laura.escamilla@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |RESOLVED Resolution|--- |FIXED CC| |Laura.escamilla@bywatersolu | |tions.com --- Comment #24 from Laura Escamilla <Laura.escamilla@bywatersolutions.com> --- Not backporting to 25.05 as this is an enhancement. Happy to revisit if needed. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org