[Koha-bugs] [Bug 28701] primary_contact_method not part of the REST API spec

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28701

Lucas Gass <lucas at bywatersolutions.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |lucas at bywatersolutions.com

--- Comment #11 from Lucas Gass <lucas at 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.