code-corps / code-corps/code-corps-api

StripeConnectPlanController has unhandled response cases.

Open
#1,031 3 comments 0 reactions 0 assignees View on GitHub
needs clarification
Dominant language
Elixir
Stars
234
Forks
82
PR merge metrics
No merged PRs in 30d

Description

# Problem

This was pointed out to me while I was reviewing #1028

`StripeConnectPlanService.create`, called by the create action can return the following values:

```Elixir
{:ok, StripeConnectPlan.t}
{:error, Ecto.Changeset.t}
{:error, Stripe.APIErrorResponse.t}
{:error, :project_not_ready}
{:error, :not_found}
```

Out of those, the changeset and the `Stripe.APIErrorResponse` are handled by our `FallbackController`. The rest are not, so a 500 will be rendered due to a `ClauseError`. We should consider handling those properly.

I'm also noticing inconsistencies. `{:error, :not_found}` refers to the project. Since we have `{:error, :project_not_ready}`, `{:error, :project_not_found}` would make morse sense.

As to how we would render these, I believe it would make sense to rely on the changeset here. `:project_not_found`, really, is a `:project, :does_not_exist` validation error.

On the other hand, `:project_not_ready` would sooner fall into the category of authorization errors, or at the very least, some different level of validation, possibly even an error category of it's own.

# Steps needed in order to provide a time estimate

We should consider splitting these into separate tasks

- identify all possible responses for `CodeCorps.StripeService.StripeConnectPlanService.create`
- discuss if the responses should be mapped differently
- apply new mapping if any
- handle these responses in the controller/fallback controller

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.