A new request with request id 18329 has been created by koha-devel-request@lists.koha-community.org. Short info on the request is :

Title : Koha-devel Digest, Vol 186, Issue 15
Category :
Description :
Send Koha-devel mailing list submissions to
    koha-devel@lists.koha-community.org

To subscribe or unsubscribe via the World Wide Web, visit
    https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
or, via email, send a message with subject or body 'help' to
    koha-devel-request@lists.koha-community.org

You can reach the person managing the list at
    koha-devel-owner@lists.koha-community.org

When replying, please edit your Subject line so it is more specific
than "Re: Contents of Koha-devel digest..."


Today's Topics:

1. PUT /holds/:hold_id (Tomas Cohen Arazi)
2. Re: PUT /holds/:hold_id (dcook@prosentient.com.au)
3. Re: PUT /holds/:hold_id (Arthur)


----------------------------------------------------------------------

Message: 1
Date: Mon, 17 May 2021 18:05:10 -0300
From: Tomas Cohen Arazi <tomascohen@gmail.com>
To: koha-devel <koha-devel@lists.koha-community.org>
Subject: [Koha-devel] PUT /holds/:hold_id
Message-ID:
    <CABZfb=XgSmWcip-KJO51duBaA3ERtCY-OQZ5uXWp7p6Uic1YhA@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"

Hi all, I wanted to gather some opinions about the mentioned route.

We've made sure all routes have the additionalProperties: false setting,
and some routes, like this one, don't have it. In the task to add it we
noticed:
1. Theroute doesn't work as expected: the documented attributes don't match
the coding guidelines (i.e. terminology hasn't been adjusted on the spec)
but it did in the code, it just works because we don't set
additionalProperties: false. TL;DR no one following the spec can use it.
2. The route seems to be wrong regarding how our RESTful implementation
works in the rest of the routes: PUT should get a Koha::Hold object for
updating it. Instead, it only 'accepts': 'priority', 'pickup_library_id'
and 'suspended_until'. If we wanted to allow to selectively change those
attributes, we should do a PATCH instead.
3. We already have individual routes for changing those things on the hold:
PUT /holds/:hold_id/pickup_location
PUT /holds/:hold_id/priority
POST /holds/:hold_id/suspension

The route itself doesn't work now. And any change will break things for
people 'using' it. And tests pass only because the spec is loose. The
options are:
- 'Fix' the route so the attributes match the code, i.e. change the spec as
bug 20006 intended to do.
- Leave it as-is (probably documenting it should not be used).
- Remove the route until we really need to implement that PUT for something.
- Make it a PATCH route, in which we could add new attributes without
breaking anyone's dev (besides the verb change, that should be advertised)
- Fully implement a PUT, making it mandatory to send all the attributes
(this would be an important breaking change).

I would vote removing it, or making it a PATCH route. But I still don't see
the use case in the UI, for the latter. Most places in Koha do atomic
updates for priority, pickup location or hold suspension for which we
clearly have routes already.

Looking forward to your comments.

--
Tomás Cohen Arazi
Theke Solutions (http://theke.io)
✆ +54 9351 3513384
GPG: B2F3C15F
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.koha-community.org/pipermail/koha-devel/attachments/20210517/20c74819/attachment-0001.htm>

------------------------------

Message: 2
Date: Tue, 18 May 2021 13:31:03 +1000
From: <dcook@prosentient.com.au>
To: "'Tomas Cohen Arazi'" <tomascohen@gmail.com>, "'koha-devel'"
    <koha-devel@lists.koha-community.org>
Subject: Re: [Koha-devel] PUT /holds/:hold_id
Message-ID: <01bf01d74b96$3adabbb0$b0903310$@prosentient.com.au>
Content-Type: text/plain; charset="utf-8"

I’m not sure that I have much to contribute.



One comment I would have is… what is the threshold for creating a V2 of the REST API?



And maybe a breaking change for the API is OK for a major version change in Koha but just not a minor version change? (That said, I’ve encountered breaking changes in PostgreSQL on minor version updates. They’re annoying, but they are a fact of life I suppose.)



David Cook

Software Engineer

Prosentient Systems

Suite 7.03

6a Glen St

Milsons Point NSW 2061

Australia



Office: 02 9212 0899

Online: 02 8005 0595



From: Koha-devel <koha-devel-bounces@lists.koha-community.org> On Behalf Of Tomas Cohen Arazi
Sent: Tuesday, 18 May 2021 7:05 AM
To: koha-devel <koha-devel@lists.koha-community.org>
Subject: [Koha-devel] PUT /holds/:hold_id



Hi all, I wanted to gather some opinions about the mentioned route.



We've made sure all routes have the additionalProperties: false setting, and some routes, like this one, don't have it. In the task to add it we noticed:

1. Theroute doesn't work as expected: the documented attributes don't match the coding guidelines (i.e. terminology hasn't been adjusted on the spec) but it did in the code, it just works because we don't set additionalProperties: false. TL;DR no one following the spec can use it.
2. The route seems to be wrong regarding how our RESTful implementation works in the rest of the routes: PUT should get a Koha::Hold object for updating it. Instead, it only 'accepts': 'priority', 'pickup_library_id' and 'suspended_until'. If we wanted to allow to selectively change those attributes, we should do a PATCH instead.
3. We already have individual routes for changing those things on the hold:
PUT /holds/:hold_id/pickup_location
PUT /holds/:hold_id/priority
POST /holds/:hold_id/suspension

The route itself doesn't work now. And any change will break things for people 'using' it. And tests pass only because the spec is loose. The options are:

- 'Fix' the route so the attributes match the code, i.e. change the spec as bug 20006 intended to do.

- Leave it as-is (probably documenting it should not be used).

- Remove the route until we really need to implement that PUT for something.

- Make it a PATCH route, in which we could add new attributes without breaking anyone's dev (besides the verb change, that should be advertised)

- Fully implement a PUT, making it mandatory to send all the attributes (this would be an important breaking change).



I would vote removing it, or making it a PATCH route. But I still don't see the use case in the UI, for the latter. Most places in Koha do atomic updates for priority, pickup location or hold suspension for which we clearly have routes already.



Looking forward to your comments.



--

Tomás Cohen Arazi

Theke Solutions (http://theke.io <http://theke.io/> )
✆ +54 9351 3513384
GPG: B2F3C15F

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.koha-community.org/pipermail/koha-devel/attachments/20210518/17103125/attachment-0001.htm>

------------------------------

Message: 3
Date: Tue, 18 May 2021 11:58:54 +0200
From: Arthur <arthur.suzuki@biblibre.com>
To: koha-devel@lists.koha-community.org
Subject: Re: [Koha-devel] PUT /holds/:hold_id
Message-ID: <ea8a616e-d98f-d842-f1f9-8c1c72dcc102@biblibre.com>
Content-Type: text/plain; charset="utf-8"

Hi Thomas, David,

I vote for making it a PATCH route and either document the PUT method as
"do-not-use" or making it a real "PUT".

In my opinion making the api a v2 is going to make things confusing for
consumers as there are already several web-services available for Koha
(ILS-DI, OAI-PMH, SRU, Rest...).

For each API version, consumer app must develop a new connector and a
mechanism to select which connector is appropriate to use, this is going
to be quite confusing + added complexity for api consumers.

Unnecessary complexity in my opinion.

Arthur Suzuki,

BibLibre Support Developper

On 18/05/2021 05:31, dcook@prosentient.com.au wrote:
>
> I’m not sure that I have much to contribute.
>
>  
>
> One comment I would have is… what is the threshold for creating a V2
> of the REST API?
>
>  
>
> And maybe a breaking change for the API is OK for a major version
> change in Koha but just not a minor version change? (That said, I’ve
> encountered breaking changes in PostgreSQL on minor version updates.
> They’re annoying, but they are a fact of life I suppose.)
>
>  
>
> David Cook
>
> Software Engineer
>
> Prosentient Systems
>
> Suite 7.03
>
> 6a Glen St
>
> Milsons Point NSW 2061
>
> Australia
>
>  
>
> Office: 02 9212 0899
>
> Online: 02 8005 0595
>
>  
>
> *From:*Koha-devel <koha-devel-bounces@lists.koha-community.org> *On
> Behalf Of *Tomas Cohen Arazi
> *Sent:* Tuesday, 18 May 2021 7:05 AM
> *To:* koha-devel <koha-devel@lists.koha-community.org>
> *Subject:* [Koha-devel] PUT /holds/:hold_id
>
>  
>
> Hi all, I wanted to gather some opinions about the mentioned route.
>
>  
>
> We've made sure all routes have the additionalProperties: false
> setting, and some routes, like this one, don't have it. In the task to
> add it we noticed:
>
> 1. Theroute doesn't work as expected: the documented attributes don't
> match the coding guidelines (i.e. terminology hasn't been adjusted on
> the spec) but it did in the code, it just works because we don't set
> additionalProperties: false. TL;DR no one following the spec can use it.
> 2. The route seems to be wrong regarding how our RESTful
> implementation works in the rest of the routes: PUT should get a
> Koha::Hold object for updating it. Instead, it only 'accepts':
> 'priority', 'pickup_library_id' and 'suspended_until'. If we wanted to
> allow to selectively change those attributes, we should do a PATCH
> instead.
> 3. We already have individual routes for changing those things on the
> hold:
> PUT /holds/:hold_id/pickup_location
> PUT /holds/:hold_id/priority
> POST /holds/:hold_id/suspension
>
> The route itself doesn't work now. And any change will break things
> for people 'using' it. And tests pass only because the spec is loose.
> The options are:
>
> - 'Fix' the route so the attributes match the code, i.e. change the
> spec as bug 20006 intended to do.
>
> - Leave it as-is (probably documenting it should not be used).
>
> - Remove the route until we really need to implement that PUT for
> something.
>
> - Make it a PATCH route, in which we could add new attributes without
> breaking anyone's dev (besides the verb change, that should be advertised)
>
> - Fully implement a PUT, making it mandatory to send all the
> attributes (this would be an important breaking change).
>
>  
>
> I would vote removing it, or making it a PATCH route. But I still
> don't see the use case in the UI, for the latter. Most places in Koha
> do atomic updates for priority, pickup location or hold suspension for
> which we clearly have routes already.
>
>  
>
> Looking forward to your comments.
>
>  
>
> --
>
> Tomás Cohen Arazi
>
> Theke Solutions (http://theke.io <http://theke.io/>)
> ✆+54 9351 3513384
> GPG: B2F3C15F
>
>
> _______________________________________________
> Koha-devel mailing list
> Koha-devel@lists.koha-community.org
> https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
> website : https://www.koha-community.org/
> git : https://git.koha-community.org/
> bugs : https://bugs.koha-community.org/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.koha-community.org/pipermail/koha-devel/attachments/20210518/3d021eb6/attachment.htm>

------------------------------

Subject: Digest Footer

_______________________________________________
Koha-devel mailing list
Koha-devel@lists.koha-community.org
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel
website : https://www.koha-community.org/
git : https://git.koha-community.org/
bugs : https://bugs.koha-community.org/


------------------------------

End of Koha-devel Digest, Vol 186, Issue 15
*******************************************


NOTE: You are receiving this mail because, the Requester/Technician wanted you to get notified on this request creation.