Bug 35747 - Some PUT endpoints wipe data if embeds are missing from the payload
Summary: Some PUT endpoints wipe data if embeds are missing from the payload
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Bugs List
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-01-09 17:13 UTC by Pedro Amorim
Modified: 2024-01-10 09:15 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pedro Amorim 2024-01-09 17:13:45 UTC
To reproduce:
0) Enable RESTBasicAuth syspref
1) Add a new license, visit:
    /cgi-bin/koha/erm/licenses/add
2) Fill in mandatory fields, click "Add document" and upload a file. 
3) Click 'Submit'
4) Edit the license, visit:
    /cgi-bin/koha/erm/licenses/edit/1
5) Pretend you're a third-party app wanting to update name and description of the license. Run the following curl:
curl -u "koha:koha" -X PUT \
  'http://localhost:8081/api/v1/erm/licenses/1' \
  --header 'Accept: */*' \
  --header 'User-Agent: Thunder Client (https://www.thunderclient.com)' \
  --header 'Content-Type: application/json' \
  --data-raw '{"description":"testing","name":"testing","status":"not_yet_active","type":"consortial"}'
6) Check the license again, visit:
    /cgi-bin/koha/erm/licenses/edit/1
7) Notice the file you uploaded in 2) has been wiped.

This PUT request is missing the documents property, so Koha will wipe all documents related to that license. Same happens for users (property 'user_roles').
Same happens if the property is sent empty.
I believe this is by design, as the Koha form will always be pre-populated with any existing data thus not wiping anything unintendedly.
I understand this is how PUT is supposed to work, and that this should be a PATCH instead, but we're currently not supporting PATCH endpoints (?).
What is the best approach here, support PATCH or change the logic behind the PUT endpoint? Other?
Comment 1 Jonathan Druart 2024-01-10 07:36:45 UTC
We should use PATCH.

There have been discussions about that, but I cannot find them!