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

feat: access tokens management api #1880

Merged
merged 7 commits into from
Feb 14, 2024
Merged

feat: access tokens management api #1880

merged 7 commits into from
Feb 14, 2024

Conversation

Ziinc
Copy link
Contributor

@Ziinc Ziinc commented Dec 8, 2023

Adds in management api routes for access token management, needed for platform integration.

  • Only private/public scopes are permitted.
  • No updating of keys are permitted.

@Ziinc Ziinc requested a review from hauleth February 7, 2024 07:37
@Ziinc Ziinc force-pushed the feat/mgmt-api-access-tokens branch from 98571bc to b4d9d88 Compare February 13, 2024 10:23
@Ziinc Ziinc requested a review from hauleth February 13, 2024 10:23
Comment on lines +41 to +49
with {:scopes, true} <- {:scopes, "partner" not in String.split(scopes_input)},
{:ok, access_token} <- Auth.create_access_token(user, params) do
conn
|> put_status(201)
|> json(access_token)
else
{:scopes, false} -> {:error, :unauthorized}
{:error, _} = err -> err
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this particular code exactly matches "complex else" anti-pattern".

Copy link
Contributor Author

@Ziinc Ziinc Feb 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't really agree that it applies in this case. The "anti-pattern" is around handling ambiguous returns from multiple different functions, where there is possible overlap of error tuples when calling different functions.

In our case, the returns are known and clearly defined given the slimness of the function, and the possible error tuples are also known and handled by the fallback controller.

Also, if we were to follow the recommendation, it would add 2 lines to the stacktrace due to the additional wrapper functions, and make the whole action function more verbose than it needs to be.

I have added more explicit tuple matching to make the error boundaries clearer. We can relook at this again in the future, but i think further refactoring has no meaningful difference.

@Ziinc Ziinc requested a review from hauleth February 13, 2024 17:33
@Ziinc
Copy link
Contributor Author

Ziinc commented Feb 14, 2024

merging in first.

@Ziinc Ziinc merged commit e7b7a67 into main Feb 14, 2024
2 checks passed
@Ziinc Ziinc deleted the feat/mgmt-api-access-tokens branch February 14, 2024 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants