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(vipps)/check payment #943

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from
Open

Feat(vipps)/check payment #943

wants to merge 11 commits into from

Conversation

LeMiTam
Copy link
Contributor

@LeMiTam LeMiTam commented Jan 20, 2025

Implemented an option to check if the vipps payment have been done.

@LeMiTam LeMiTam requested review from MadsNyl and EmilJohns1 January 20, 2025 15:53
Copy link
Contributor

@MadsNyl MadsNyl left a comment

Choose a reason for hiding this comment

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

Bra start, men fiks aksesskontroll

urlpatterns = [re_path(r"", include(router.urls))]
urlpatterns = [
re_path(r"", include(router.urls)),
path("check-vipps-payment", check_vipps_payment),
Copy link
Contributor

Choose a reason for hiding this comment

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

Kan gjøre url'en lettere her ved f.eks "check-payment"

Comment on lines 9 to 35
@api_view(["POST"])
def check_vipps_payment(self, request, *args, **kwargs):
has_changed = False

serializer = CheckPaymentSerializer(data=request.data)
serializer.is_valid(raise_exception=True)

event_id = serializer.validated_data["event_id"]
user_id = serializer.validated_data["user_id"]

orders = self.queryset.filter(user_id=user_id, event_id=event_id)
if not orders.exists():
return Response(
{"message": "No orders found for the user in this event."},
status=status.HTTP_404_NOT_FOUND,
)

for order in orders:
order_status = get_payment_order_status(order.order_id)
if order_status != order.status:
has_changed = True
order.status = order_status
order.save()

if has_changed(order):
return Response({"is_paid": True}, status=status.HTTP_200_OK)
return Response({"is_paid": False}, status=status.HTTP_200_OK)
Copy link
Contributor

Choose a reason for hiding this comment

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

Her må dere implementere aksesskontroll. Dere må sjekke om den som sender requesten har lov til å redigere dette eventet. Sjekk ut logikken på dette for edit på events

Copy link
Contributor

@MadsNyl MadsNyl left a comment

Choose a reason for hiding this comment

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

Good

@EmilJohns1
Copy link
Contributor

Ska funke som det skal no :)
Blei aksesskontrollen på klassenivå gjort riktig? Litt usikker på implementasjonen min @MadsNyl

Copy link
Contributor

@MadsNyl MadsNyl left a comment

Choose a reason for hiding this comment

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

Hvis dere først skal teste om et medlem av sosialen får lov, så må dere også teste at medlemmer som ikke er en del av organizer ikke får lov. Denne testen i seg selv tester ikke om aksesskontrollen fungerer godt nok

Copy link
Contributor

@MadsNyl MadsNyl left a comment

Choose a reason for hiding this comment

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

Ser bra ut. Liten fiks for å gjøre det lettere for frontend

Comment on lines 41 to 43
if has_changed(order):
return Response({"is_paid": True}, status=status.HTTP_200_OK)
return Response({"is_paid": False}, status=status.HTTP_200_OK)
Copy link
Contributor

Choose a reason for hiding this comment

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

Endre disse til "details" siden det er den nøkkelen frontend sjekker etter

Copy link
Contributor

@EmilJohns1 EmilJohns1 left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants