Create API v1 with customers endpoint #8891

mkllnk · 2022-02-15T01:03:03Z

What? Why?

Step 2 of #8787

Rebased version of #8326. We are setting the scene for the new OFN API V1 and add the customers endpoint as first example.

What should we test?

Visit /api-docs to see what kind of requests need testing.
You should be able to try the API in that UI.
The endpoint supports full CRUD: create, read, update, delete
You should be able to access the endpoints in your browser while logged in with the right permissions.

You should be able to access the endpoints via curl in the command line if you provide your API key.
Examples:

curl -X GET "http://localhost:3000/api/v1/customers?token=97...ad2"\
 -H "accept: application/json"

curl -X GET "http://localhost:3000/api/v1/customers"\
 -H "accept: application/json"\
 -H "X-Api-Token: 97...ad2"

You should not be able to access customer data without authorisation.
Pagination links should work.
The enterprise relationship link doesn't work yet because we don't have that endpoint yet.
Not all attributes are show yet. There's another issue and PR for that in the pipe.
Users can see their own customer records and the records of the enterprises they manage.
All records are returned without a given enterprise id.
Only customer records of one enterprise are shown if the enterprise id is given.

Release notes

Changelog Category: User facing changes

The title of the pull request will be included in the release notes.

Dependencies

We are waiting for:

Documentation updates

Let's not publish this yet but we can upload the new swagger file to Swaggerhub at some point. In the meantime, staging servers will have documentation at /api-docs.

jibees

Nice and though one! 💪

Reading the initial issue (#8788), I'm a bit concerned about the needed attributes (see my comment below), but also about the feature toggle mentioned in the acceptance criteria: "8- Feature exists behind a feature toggle"

jibees · 2022-02-28T10:36:51Z

app/serializers/api/v1/customer_serializer.rb

+module Api
+  module V1
+    class CustomerSerializer < BaseSerializer
+      attributes :id, :enterprise_id, :first_name, :last_name, :code, :email


Reading the initial issue: #8788, I was wondering if there were some necessary attributes missing?

This is not closing #8788. This is only step two of #8787. I will create another PR for the missing attributes and the feature toggle.

oh yes, I didn't see that step2 ! Thanks !

- Inherits from ActionController::API - Lots of superfluous junk removed

Update translation key namespacing

And add new customers routes. Disable them in production for now.

Add customer serializer

Include test helpers

https://jsonapi.org/examples/#pagination Update schema for collections rendered with pagination data

It raised an error: NoMethodError: undefined method `map' for nil:NilClass

We don't know what unknown errors would report. They could expose sensitive data. So let's not pass that data on to the public while we have the full details in Bugsnag. Also, let's not catch Exception because that could catch interrupts to gracefully shut down the application.

apricot12

Went through this. Looks good to my eye 🚀

filipefurtad0 · 2022-03-18T13:05:50Z

Hey @mkllnk ,

After staging, the server displays the

Checked the docs, and tested the following endpoints:

customers

curl -X GET "https://staging.openfoodnetwork.org.uk/api/v1/customers?token=0_____________________________5" -H "accept: application/json"

creating a customer

Whhoaa. Awesome. We can now create customers via the console 👀 🎉 🚀

However, it did not really seemed to work correctly for first, last and code:

the request:
curl -X POST "https://staging.openfoodnetwork.org.uk/api/v1/customers?token=0_____________________________5" -H "accept: application/json" -H "Content-Type: application/json" -d "{\"enterprise_id\":1819,\"first_name\":\"Alice\",\"last_name\":\"Springs\",\"code\":\"BUYER1\",\"email\":\"alice@example.com\"}"

Created this customer:

Is there an obvious reason for this, that I might be missing? I'll need to stop here and continue testing later.

filipefurtad0 · 2022-03-18T13:45:59Z

I'm not really done with testing all the test cases above (thanks for adding them Maikel) nor eventual regression testing. I'm not sure on the risk of regressions this PR introduces.

From the discussion above, it seems the feature toggle is not yet implemented here. I also could not find any reference to Flipper in the code, or anything on how to activate/deactivate this feature.

On the other hand, we should merge asap to unblock work on this funded feature. So, moving to ready to go, feel free to merge.

Matt-Yorkley · 2022-03-20T21:57:45Z

app/controllers/api/v1/customers_controller.rb

+      end
+
+      def customer_params
+        params.require(:customer).permit(:email, :enterprise_id)


This needs the other params like :first_name, :last_name, right?

(That's why they didn't show up in the test @filipefurtad0 did above...)

Yes, that's in the next PR which adds all the specified attributes.

Matt-Yorkley · 2022-03-20T22:01:45Z

spec/requests/api/v1/customers_spec.rb

+      it "informs about invalid pages" do
+        get "/api/v1/customers", params: { page: "0" }
+        expect(json_response_ids).to eq nil
+        expect(json_error_detail).to eq 'expected :page >= 1; got "0"'


This seems like a weird case...

We could just avoid it entirely by forcing the value to always be positive at the point we're doing the pagination, eg:

params[:per_page].to_i.clamp(1..)

What do you think?

Tempting. Much simpler. But I think that we should give people direct feedback about invalid parameters. It may have been a typo and we don't want to let our sever work if not needed. And someone may have intended to get page 10 and then got page 1 instead, may not notice.

mkllnk mentioned this pull request Feb 15, 2022

API - rswag integration #8326

Closed

mkllnk marked this pull request as ready for review February 25, 2022 02:51

mkllnk self-assigned this Feb 28, 2022

jibees reviewed Feb 28, 2022

View reviewed changes

Matt-Yorkley and others added 22 commits March 2, 2022 11:55

Bring simplified / cleaned up base controller into V1

7d8ded5

- Inherits from ActionController::API - Lots of superfluous junk removed

Tidy up error handling and update response formats for JSON:API standard

c4e2c8c

Update translation key namespacing

Create customers controller

3128232

And add new customers routes. Disable them in production for now.

Fill out customers controller

cc41920

Add customer serializer

Generate docs with rails rswag

39f4fee

Add specs and JSON schemas

76f14a0

Include test helpers

Test permissions combined with Ransack searches

46f9d3e

Test serialized relationship data

028d02c

Regenerate swagger docs with rails rswag

56bc554

Update and document authentication methods

a222b50

Add 401 Unauthorized response test

8e61428

Add RequestTimeout concern

c102ce8

Add pagination with Pagy using structure from JSON:API specification

3dbf00f

https://jsonapi.org/examples/#pagination Update schema for collections rendered with pagination data

Update use of links on relationships

d66d6d6

Add relationships to resource serializers

d87e180

Refactor to make more use of schema refs, and regenerate swagger file

2e59812

Update schemas for relationships

bd9bed7

Don't list guest customer records to guest users

414bf5d

Make json_response test helper deal with error response

75fc355

It raised an error: NoMethodError: undefined method `map' for nil:NilClass

Sanitise pagination input for new API

b897151

Render pagination errors

4aa70c1

mkllnk requested a review from Matt-Yorkley March 7, 2022 23:57

jibees approved these changes Mar 11, 2022

View reviewed changes

apricot12 approved these changes Mar 15, 2022

View reviewed changes

filipefurtad0 self-assigned this Mar 17, 2022

filipefurtad0 added the pr-staged-uk staging.openfoodnetwork.org.uk label Mar 18, 2022

filipefurtad0 removed the pr-staged-uk staging.openfoodnetwork.org.uk label Mar 18, 2022

Matt-Yorkley reviewed Mar 20, 2022

View reviewed changes

mkllnk merged commit 877002a into openfoodfoundation:master Mar 22, 2022

mkllnk deleted the api-customers branch March 22, 2022 08:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create API v1 with customers endpoint #8891

Create API v1 with customers endpoint #8891

mkllnk commented Feb 15, 2022 •

edited

jibees left a comment

jibees Feb 28, 2022

mkllnk Feb 28, 2022

jibees Mar 1, 2022

apricot12 left a comment

filipefurtad0 commented Mar 18, 2022 •

edited

filipefurtad0 commented Mar 18, 2022

Matt-Yorkley Mar 20, 2022 •

edited

Matt-Yorkley Mar 20, 2022 •

edited

mkllnk Mar 22, 2022

Matt-Yorkley Mar 20, 2022

mkllnk Mar 22, 2022

Create API v1 with customers endpoint #8891

Create API v1 with customers endpoint #8891

Conversation

mkllnk commented Feb 15, 2022 • edited

What? Why?

What should we test?

Release notes

Dependencies

Documentation updates

jibees left a comment

Choose a reason for hiding this comment

jibees Feb 28, 2022

Choose a reason for hiding this comment

mkllnk Feb 28, 2022

Choose a reason for hiding this comment

jibees Mar 1, 2022

Choose a reason for hiding this comment

apricot12 left a comment

Choose a reason for hiding this comment

filipefurtad0 commented Mar 18, 2022 • edited

filipefurtad0 commented Mar 18, 2022

Matt-Yorkley Mar 20, 2022 • edited

Choose a reason for hiding this comment

Matt-Yorkley Mar 20, 2022 • edited

Choose a reason for hiding this comment

mkllnk Mar 22, 2022

Choose a reason for hiding this comment

Matt-Yorkley Mar 20, 2022

Choose a reason for hiding this comment

mkllnk Mar 22, 2022

Choose a reason for hiding this comment

mkllnk commented Feb 15, 2022 •

edited

filipefurtad0 commented Mar 18, 2022 •

edited

Matt-Yorkley Mar 20, 2022 •

edited

Matt-Yorkley Mar 20, 2022 •

edited