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

Added api endpoints #320

Merged
merged 6 commits into from
May 17, 2023
Merged

Added api endpoints #320

merged 6 commits into from
May 17, 2023

Conversation

Zaph-x
Copy link
Member

@Zaph-x Zaph-x commented Oct 19, 2022

This pull request aims to get us closer to moving away from a monolithic application.
This is done by creating API endpoints, which exposes mostly public facing data in stregsystemet through GET requests.
Furthermore, a sale endpoint which accepts POST requests has been added.

Copy link
Member

@falkecarlsen falkecarlsen left a comment

Choose a reason for hiding this comment

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

Needs some polish and definitely tests but cool 👍

stregsystem/views.py Outdated Show resolved Hide resolved
stregsystem/views.py Show resolved Hide resolved
stregsystem/views.py Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Merging #320 (f3b0a0e) into next (35d32f9) will decrease coverage by 2.73%.
The diff coverage is 31.50%.

@@            Coverage Diff             @@
##             next     #320      +/-   ##
==========================================
- Coverage   83.07%   80.33%   -2.74%     
==========================================
  Files          31       31              
  Lines        2659     2782     +123     
  Branches      188      214      +26     
==========================================
+ Hits         2209     2235      +26     
- Misses        418      515      +97     
  Partials       32       32              
Impacted Files Coverage Δ
stregsystem/urls.py 75.00% <ø> (ø)
stregsystem/views.py 45.52% <31.50%> (-11.20%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

LowkeyCoding added a commit to f-klubben/fappen that referenced this pull request Oct 21, 2022
Copy link
Contributor

@jonasKjellerup jonasKjellerup left a comment

Choose a reason for hiding this comment

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

The handling of error states is in some cases a bit inconsistent (there are quite a few conditions that cause internal server error response).

Otherwise most things seem to work as expected (based on some manual testing)

stregsystem/views.py Outdated Show resolved Hide resolved
stregsystem/views.py Outdated Show resolved Hide resolved
Comment on lines +602 to +603
if username != member.username:
return HttpResponseBadRequest("Username does not match member_id")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for requiring a user identifier both as a parameter in the post body and as a part of the buystring?

@falkecarlsen
Copy link
Member

@Zaph-x fails black

@Nobogo Nobogo merged commit 31d8c2a into f-klubben:next May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants