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

/accounts/* HTTP endpoint #129

Merged
merged 41 commits into from Jul 5, 2019
Merged

/accounts/* HTTP endpoint #129

merged 41 commits into from Jul 5, 2019

Conversation

@daltonmatos
Copy link
Member

daltonmatos commented Jun 27, 2019

Tasks

  • Mudar os try/except para pegar múltiplas exceptions de uma vez, quando der;
  • Mudar a adição de usuário para ser POST /accounts/{acc_id}/user/{u_id} da67e5e
  • Mudar a remoção de usuário para ser DELETE /accounts/{acc_id}/user/{u_id} f708504
daltonmatos added 3 commits Jun 27, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Jun 27, 2019

Codecov Report

Merging #129 into master will decrease coverage by 0.26%.
The diff coverage is 87.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #129      +/-   ##
==========================================
- Coverage   90.46%   90.19%   -0.27%     
==========================================
  Files          85       86       +1     
  Lines        2338     2591     +253     
  Branches       94      156      +62     
==========================================
+ Hits         2115     2337     +222     
- Misses        212      241      +29     
- Partials       11       13       +2
Flag Coverage Δ
#typehint 29.97% <30.76%> (+0.43%) ⬆️
#unittest 95.26% <100%> (+0.44%) ⬆️
Impacted Files Coverage Δ
asgard/exceptions/__init__.py 100% <100%> (ø)
asgard/api/resources/accounts.py 100% <100%> (ø) ⬆️
asgard/services/accounts.py 100% <100%> (ø) ⬆️
asgard/api/resources/users.py 100% <100%> (ø) ⬆️
asgard/models/user.py 89.65% <100%> (ø) ⬆️
asgard/services/users.py 100% <100%> (ø) ⬆️
asgard/backends/accounts.py 77.27% <71.42%> (-12.21%) ⬇️
asgard/backends/users.py 74.39% <79.36%> (+10.75%) ⬆️
asgard/api/users.py 90.9% <89.28%> (+9.09%) ⬆️
asgard/api/accounts.py 90.9% <90.19%> (-1.6%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fcc82a4...5bcb20e. Read the comment docs.

daltonmatos added 12 commits Jun 27, 2019
Não precisamos do prefixo `_` nesse caso.
…-users-from-account
@daltonmatos daltonmatos changed the title Feature/list users from account /acounts HTTP endpoint Jul 2, 2019
@daltonmatos daltonmatos changed the title /acounts HTTP endpoint /acounts/* HTTP endpoint Jul 2, 2019
@daltonmatos daltonmatos requested a review from igorgcosta Jul 2, 2019
@daltonmatos

This comment has been minimized.

Copy link
Member Author

daltonmatos commented Jul 2, 2019

Boa parte do código dos endpoints poderá ser simplificada com a implementação das issues b2wdigital/async-worker#114 e b2wdigital/async-worker#113

@daltonmatos daltonmatos requested a review from pabrrs Jul 3, 2019
@daltonmatos

This comment has been minimized.

Copy link
Member Author

daltonmatos commented Jul 3, 2019

Boa parte do código dos endpoints poderá ser simplificada com a implementação das issues B2W-BIT/async-worker#114 e B2W-BIT/async-worker#113

O que quermos é poder ter um handler nessa linha:

Pegando como exemplo um dos handlers desse PR:

@app.route(
    ["/accounts/{account_id:Account}/users"],
    type=RouteTypes.HTTP,
    methods=["POST"],
)
async def add_user_to_account(
    account: Account, user: RequestBody[UserResource]
):
    await AccountsService.add_user_to_account(user, account, AccountsBackend())

    return web.json_response(AccountUsersResource().dict())

Toda a validação de HTTP 400 e 404 já estaria embutida no framework (asynckworker) então se o código do handler é chamado significa que o input já foi validado e os registros de Account e User existem e podem ser processados.

Copy link
Collaborator

pabrrs left a comment

👏 🎉 🐍

P.S: Tem um typo no título da PR em acounts , quando deveria ser accounts

asgard/api/accounts.py Show resolved Hide resolved
asgard/api/accounts.py Outdated Show resolved Hide resolved
asgard/api/accounts.py Outdated Show resolved Hide resolved
itests/asgard/api/test_accounts.py Show resolved Hide resolved
itests/asgard/api/test_accounts.py Show resolved Hide resolved
itests/asgard/api/test_accounts.py Outdated Show resolved Hide resolved
@daltonmatos daltonmatos changed the title /acounts/* HTTP endpoint /accounts/* HTTP endpoint Jul 3, 2019
@daltonmatos

This comment has been minimized.

Copy link
Member Author

daltonmatos commented Jul 3, 2019

P.S: Tem um typo no título da PR em acounts , quando deveria ser accounts

Corrigido. =D

daltonmatos added 5 commits Jul 3, 2019
daltonmatos added 21 commits Jul 3, 2019
…count
@daltonmatos daltonmatos merged commit 5bcb20e into master Jul 5, 2019
7 checks passed
7 checks passed
ci/circleci: py368 Your tests passed on CircleCI!
Details
ci/circleci: py36x Your tests passed on CircleCI!
Details
ci/circleci: py37x Your tests passed on CircleCI!
Details
codecov/patch/typehint 30.76% of diff hit (target 29.54%)
Details
codecov/patch/unittest 100% of diff hit (target 94.82%)
Details
codecov/project/typehint 29.97% (+0.43%) compared to fcc82a4
Details
codecov/project/unittest 95.26% (+0.44%) compared to fcc82a4
Details
@daltonmatos daltonmatos deleted the feature/list-users-from-account branch Jul 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.