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

StripeConnectPlanController has unhandled response cases.

未关闭
#1,031 3 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
needs clarification
主要语言
Elixir
星标
234
派生
82
PR 合并指标
30 天内没有已合并 PR

描述

# 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

贡献指南

打开贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。