Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EPIC: Basic REST API #256

Open
12 tasks done
nelsonic opened this issue Jan 11, 2023 · 12 comments
Open
12 tasks done

EPIC: Basic REST API #256

nelsonic opened this issue Jan 11, 2023 · 12 comments
Assignees
Labels
discuss Share your constructive thoughts on how to make progress with this issue enhancement New feature or enhancement of existing functionality epic A feature idea that is large enough to require a sprint (5 days) or more and has smaller sub-issues. help wanted If you can help make progress with this issue, please comment! priority-1 Highest priority issue. This is costing us money every minute that passes. T1d Time Estimate 1 Day technical A technical issue that requires understanding of the code, infrastructure or dependencies

Comments

@nelsonic
Copy link
Member

nelsonic commented Jan 11, 2023

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:

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. 🙏

@nelsonic nelsonic added enhancement New feature or enhancement of existing functionality help wanted If you can help make progress with this issue, please comment! T1d Time Estimate 1 Day epic A feature idea that is large enough to require a sprint (5 days) or more and has smaller sub-issues. technical A technical issue that requires understanding of the code, infrastructure or dependencies priority-1 Highest priority issue. This is costing us money every minute that passes. discuss Share your constructive thoughts on how to make progress with this issue labels Jan 11, 2023
@nelsonic nelsonic moved this to 📋 Backlog in dwyl app kanban Jan 11, 2023
@nelsonic nelsonic pinned this issue Jan 11, 2023
@LuchoTurtle LuchoTurtle moved this from 📋 Backlog to 🏗 In progress in dwyl app kanban Jan 11, 2023
@LuchoTurtle
Copy link
Member

LuchoTurtle commented Jan 11, 2023

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).

@nelsonic
Copy link
Member Author

@LuchoTurtle in this instance a PR with tests is very welcome. 🙏

LuchoTurtle added a commit that referenced this issue Jan 12, 2023
LuchoTurtle added a commit that referenced this issue Jan 12, 2023
LuchoTurtle added a commit that referenced this issue Jan 13, 2023
LuchoTurtle added a commit that referenced this issue Jan 13, 2023
LuchoTurtle added a commit that referenced this issue Jan 23, 2023
LuchoTurtle added a commit that referenced this issue Jan 23, 2023
# Conflicts:
#	api.md
#	lib/app_web/router.ex
#	test/api/item_test.exs
#	test/api/timer_test.exs
LuchoTurtle added a commit that referenced this issue Jan 23, 2023
LuchoTurtle added a commit that referenced this issue Jan 23, 2023
LuchoTurtle added a commit that referenced this issue Jan 23, 2023
LuchoTurtle added a commit that referenced this issue Jan 25, 2023
LuchoTurtle added a commit that referenced this issue Jan 25, 2023
@nelsonic
Copy link
Member Author

@LuchoTurtle this is what I had mentioned for API docs: https://github.com/Doctave/doctave 💭

@LuchoTurtle
Copy link
Member

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

@nelsonic
Copy link
Member Author

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

@LuchoTurtle
Copy link
Member

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

@nelsonic
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Share your constructive thoughts on how to make progress with this issue enhancement New feature or enhancement of existing functionality epic A feature idea that is large enough to require a sprint (5 days) or more and has smaller sub-issues. help wanted If you can help make progress with this issue, please comment! priority-1 Highest priority issue. This is costing us money every minute that passes. T1d Time Estimate 1 Day technical A technical issue that requires understanding of the code, infrastructure or dependencies
Projects
Status: 🏗 In progress
Development

No branches or pull requests

2 participants