23859 – Cities endpoint not setting the Location header

Bug 23859 - Cities endpoint not setting the Location header

Summary: Cities endpoint not setting the Location header

Status:	CLOSED FIXED

Alias:	None

Product:	Koha
Classification:	Unclassified
Component:	REST API (show other bugs)
Version:	Main
Hardware:	All All

Importance:	P5 - low minor
Assignee:	Tomás Cohen Arazi (tcohen)
QA Contact:	Martin Renvoize (ashimema)

URL:
Keywords:

Depends on:	23843
Blocks:
	Show dependency tree / graph

Reported:	2019-10-21 17:37 UTC by Tomás Cohen Arazi (tcohen)
Modified:	2021-06-14 21:28 UTC (History)
CC List:	2 users (show)

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

Attachments
Bug 23859: Update tests to new style (17.92 KB, patch) 2019-10-21 19:12 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 23859: Regression tests (1.21 KB, patch) 2019-10-21 19:15 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 23859: Make POST add the Location header on cities endpoint (1.77 KB, patch) 2019-10-21 19:15 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 23859: Update tests to new style (18.20 KB, patch) 2019-10-21 19:15 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 23859: Regression tests (1.27 KB, patch) 2019-11-01 08:42 UTC, Martin Renvoize (ashimema)	Details \| Diff \| Splinter Review
Bug 23859: Make POST add the Location header on cities endpoint (1.83 KB, patch) 2019-11-01 08:42 UTC, Martin Renvoize (ashimema)	Details \| Diff \| Splinter Review
Bug 23859: Update tests to new style (18.27 KB, patch) 2019-11-01 08:42 UTC, Martin Renvoize (ashimema)	Details \| Diff \| Splinter Review
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Tomás Cohen Arazi (tcohen) 2019-10-21 17:37:00 UTC

Comment 1 Tomás Cohen Arazi (tcohen) 2019-10-21 19:12:38 UTC

Created attachment 94497 [details] [review]
Bug 23859: Update tests to new style

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Comment 2 Tomás Cohen Arazi (tcohen) 2019-10-21 19:15:27 UTC

Created attachment 94498 [details] [review]
Bug 23859: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Comment 3 Tomás Cohen Arazi (tcohen) 2019-10-21 19:15:32 UTC

Created attachment 94499 [details] [review]
Bug 23859: Make POST add the Location header on cities endpoint

According to the coding guidelines, this endpoint should be adding the
Location header on POST actions.

To test:
1. Apply the tests patch
2. Run:
    $ kshell
    k$ prove t/db_dependent/api/v1/cities.t
=> FAIL: Tests fail!
3. Apply this patch
4. Repeat (1)
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Comment 4 Tomás Cohen Arazi (tcohen) 2019-10-21 19:15:36 UTC

Created attachment 94500 [details] [review]
Bug 23859: Update tests to new style

The /cities endpoint is refered in several places as the reference
implementation of a route for devs to pick it for copy and pasting. Yet,
it is outdated in terms of how the tests are written.

This patch rewrites some bits so it uses BASIC auth and the built-ins
Test::Mojo provides.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Comment 5 Martin Renvoize (ashimema) 2019-11-01 08:42:15 UTC

Created attachment 94938 [details] [review]
Bug 23859: Regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Comment 6 Martin Renvoize (ashimema) 2019-11-01 08:42:18 UTC

Created attachment 94939 [details] [review]
Bug 23859: Make POST add the Location header on cities endpoint

According to the coding guidelines, this endpoint should be adding the
Location header on POST actions.

To test:
1. Apply the tests patch
2. Run:
    $ kshell
    k$ prove t/db_dependent/api/v1/cities.t
=> FAIL: Tests fail!
3. Apply this patch
4. Repeat (1)
=> SUCCESS: Tests pass!
5. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Comment 7 Martin Renvoize (ashimema) 2019-11-01 08:42:22 UTC

Created attachment 94940 [details] [review]
Bug 23859: Update tests to new style

The /cities endpoint is refered in several places as the reference
implementation of a route for devs to pick it for copy and pasting. Yet,
it is outdated in terms of how the tests are written.

This patch rewrites some bits so it uses BASIC auth and the built-ins
Test::Mojo provides.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Comment 8 Martin Renvoize (ashimema) 2019-11-01 08:43:27 UTC

Trivial patch that improves the consistency of our API's.

Passing QA

Comment 9 Martin Renvoize (ashimema) 2019-11-01 08:49:00 UTC

Nice work!

Pushed to master for 19.11.00