dwyl/mvp

EPIC: Basic `REST API`

nelsonic opened this issue ยท 12 comments

As discussed in: dwyl/app#273 having a REST and WebSocket API is a Top Priority for us. ๐Ÿ”
We want anyone to be able to securely query their data using the API and an AUTH_API_KEY or JWT; this is our API Roadmap. For now we just need something super basic so that we can get moving on the Flutter MVP: dwyl/product-roadmap#40

Todo

We are breaking down this EPIC into 3 stages, an attempt chunk the work
so that we can ship each stage as fast as possible.
We expect there to be 3 separate Pull Requests; one for each stage.

Rather than extending the already very long BUILDIT.md file,
which is 75 pages at the latest viewing:

buildit-log-is-75-pages

We will instead:

  • Create a new file: API.md in the root of the project where we write-up the details of the `API

Stage 1: No Auth ๐Ÿ“–

  • Create an unauthenticated API endpoint [:authOptional] in the MVP App
    that enables the basic CRUD operations for items and timers:
  • POST to create an item anonymously, /api/items/new
    should only accept the text data and return an item.id, e.g: 200 '{"id":42}'
  • PATCH to Edit/Update an item.text: `/api/items/:id/update
  • GET to READ the contents of the item, e.g: /api/items/:id

You'll notice that we are not doing tags in the first stage; this is deliberate because there's quite a lot going on in the tags and we want to get the first

!important

The purpose of this first stage is to ensure we have a good foundation for testing the API.
That means having automated:

  • API Consumer Tests: #268

Stage 2: Tags ๐Ÿท๏ธ

Creating tags and associating them to items is a bit more involved because it can require multiple API requests if we follow traditional REST conventions. We should definitely do that for completeness. But for UX we should also allow people to create an item and specify the tags in a single API request. ๐Ÿ’ญ

  • Create the /api/tags resource with:
  • POST for /api/tags/new
  • GET for /api/tags/:id and
  • PATCH for /api/tags/:id/update

Next:

  • Retrofit the POST /api/items/new and PATCH /api/items/:id/update endpoints to allow sending the tags on item creation and update.
    e.g: {"text": "my awesome update", "tags": "awesome, priority-1"} This will add some complexity so make sure you break it down into tiny + testable functions.
  • Modify the GET /api/items/:id to return the tags with the item e.g: {"text": "Learn elixir", "tags": "learn"}

This is definitely a denormalisation and potentially "frowned upon" by some REST purists. But we don't care. We only care about UX. If the person using the API can make fewer requests and get their data faster, we are happy. ๐Ÿ˜Š

Stage 3: With Authentication! ๐Ÿ”

Once the Basic REST API is working [deployed!] we will allow people to view a JWT within the App which they can use to make authenticated API Requests. There are a few extra steps to enable this so as soon as the previous 2 stages are complete (PRs merged & deployed) we will re-visit and expand on this stage. ๐Ÿ‘Œ

Note: What we will eventually do is have an api.dwyl.com similar to how GitHub does it: https://docs.github.com/en/rest/overview/resources-in-the-rest-api so the API will run on a completely different server/service

@LuchoTurtle please take a look at this and LMK if it's clear enough for you to start work on it today. ๐Ÿ™

Just an update on this.
Since :authOptional has to be used (according to what is said in the issue), it's giving me lots of problems because of sessions (that don't exist when calling an API anonymously).
Making a simple POST request is yielding me the following error:

session not fetched, call fetch_session/2

This error occurs in auth_plug and I've located where. I'm just thinking of how I should change this code so it doesn't break the core functionality of the package and doesn't fetch_session two times needlessly. The error is in get_jwt, when calling get_session..

It's weird, this error shouldn't be happening according to the Conn.Plug's source code, since the :jwt atom is being passed.

This could easily be surpassed by. just creating the authOptional pipeline with a plug:fetch_session, like so:

  pipeline :authOptional do
    plug :fetch_session
    plug(AuthPlugOptional)
  end

However, I feel like auth_plug should gracefully handle these scenarios. I won't create any PRs because it is out of the scope of this, but I created an issue for discussion -> dwyl/auth_plug#97 (comment).

@LuchoTurtle in this instance a PR with tests is very welcome. ๐Ÿ™

Just attempted:

curl -X POST http://localhost:4000/api/items/1/timers -H 'Content-Type: application/json'

Got:

{"code":400,"errors":{"start":["can't be blank"]},"message":"Malformed request"}

This is not the desired behaviour.
We want a timer to be started on the server without having to specify the timer.start.
Apologies for not making that clear above. But that is the desired action.
We also want the ability to specify the timer.start in order to control the start time.
But the default response should not be a 404 if the timer.start is not set by the API requestor.

While testing the API using cURL on my localhost I noted that the 404 page for /api routes is HTML ...
Please see: #269

At present the API route to stop a timer is: /api/items/:item_id/timers/:id
It should be /api/timers/:id I shouldn't need to specify the :item_id

Regarding the tag stage 2, when creating an item with tags, doesn't it make sense and less prone to errors if tags are passed as an array instead of a comma-separated string?

{"text": "my awesome update", "tags": ["awesome", "priority-1"]}

instead of

{"text": "my awesome update", "tags": "awesome, priority-1"}

Side Quest: #278

@LuchoTurtle this is what I had mentioned for API docs: https://github.com/Doctave/doctave ๐Ÿ’ญ

Would it make sense to use it for our API docs? Or do you want to battle-test it first?

Fairly confident that doctave will work for our needs. ๐Ÿ’ญ
The docs/tutorial seems pretty straightforward: https://cli.doctave.com/tutorial

question is: will we get API docs by using doctave or will we still need to use a tool like: https://github.com/open-api-spex/open_api_spex/blob/v3.16.1/README.md

Should this issue be closed? A basic API Rest is already implemented (albeit without auth).

@LuchoTurtle as much as I would love to close this, auth is what makes this useful.