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

Tomás Cohen Arazi (tcohen) <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[email protected]

--- Comment #10 from Tomás Cohen Arazi (tcohen) <[email protected]> ---
(In reply to Matt Blenkinsop from comment #9)
> (In reply to Jonathan Druart from comment #8)
> > 1. Why do you make the basket's name not null?
> >    name:
> > -    type: string
> > +    type:
> > +      - string
> > +      - "null"
> > 
> > It can be NULL at DB level, but the UI is enforcing it. I think we should
> > keep the constraint at the REST API level.
> 
> I can reset this and do checks in the controller

If you remove the "null" option in the spec, the OpenAPI plugin will take care
of validation.

> > 2. 
> >    create_items:
> > -    type: string
> > +    type:
> > +      - string
> > +      - "null"
> > Same, why allowing null here?
> 
> In the description for create_items it says "When items should be created
> for orders in this basket (Options: 'ordering', 'receiving', 'cataloguing'.
> Null means system wide config)". Null is supposed to be allowed as an option
> but the endpoint crashes if it is set to null as we don't allow null to be
> passed

I don't like enum fields to not be advertised as such in the spec. But I'm not
sure how's that gonna work with the 'nullable' option. You should try. And
maybe add a 'system_wide' option that would map to NULL on its way to the DB.

> > 3. 
> > -  authorised_by:
> > +  creator_id:
> > 
> > It has been there for a while now, isn't it a problem to modify it,
> > especially without advertising it. There may be existing consumers.
> 
> How does this work with the to_api_mapping? That maps authorisedby to
> creator_id which suggests there would be conflict?

Having consumers already use the API as-is should be considered. I suggest we:

* Add the new mapping
* Manually add the old field in the Koha::Acquisition::Basket::to_api method
(the same way we do it for `closed`.
* Update the spec to *add* the new field, and add a [DEPRECATED] message on the
old one
* File a bug for removing it on 25.05.
* Make sure we mention this in the release notes.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to