Re-send validation email, refactor to APIViews by GergiH · Pull Request #840 · IFRCGo/go-api

GergiH · 2020-08-04T08:38:49Z

Ref.: #737

Changes

Refactored all PublicJsonPostViews and PublicJsonRequestViews to be APIViews instead
Added ResendValidation View which requires a username and resends the registration validation email to the user (if it's within a day of the registration, the same as it is handled by the main registration process)

thenav56 · 2020-08-05T12:33:14Z

+class ResendValidation(PublicJsonPostView):
+    def handle_post(self, request, *args, **kwargs):
+        body = json.loads(request.body.decode('utf-8'))


We should avoid using PublicJsonPostView and instead try to use DRF for any API views.

Something like this

go-api/api/views.py

Lines 51 to 62 in c1e11c4

class UpdateSubscriptionPreferences(APIView):

authentication_classes = (authentication.TokenAuthentication,)

permissions_classes = (permissions.IsAuthenticated,)

def post(self, request):

errors, created = Subscription.sync_user_subscriptions(self.request.user, request.data, True) # deletePrevious

if len(errors):

return Response({

'status': 400,

'data': 'Could not create one or more subscription(s), aborting'

})

return Response({'data': 'Success'})

All the other similar views use the same pattern, like ShowUsername or RecoverPassword. Should I also change those to use APIView?

We would need to refactor so that all API views are using DRF views instead of the custom PublicJsonPostView.
It will be cool to follow this for any new addition to the go-api.
We planned to do this right? @batpad @GregoryHorvath

@thenav56 Since these are public Views we don't need these for the PublicJsonPostView->APIViews, right?

go-api/api/views.py

Lines 52 to 53 in c1e11c4

authentication_classes = (authentication.TokenAuthentication,)

permissions_classes = (permissions.IsAuthenticated,)

The part here is what confusing me, though it doesn't seem that we use this anywhere atm:

go-api/api/views.py

Lines 283 to 286 in c1e11c4

def get_authenticated_user(self, request):

auth_header = request.META.get('HTTP_AUTHORIZATION')

if not auth_header:

return None

@GregoryHorvath Yeah we don't need them. The best way will be to just define empty permissions_classses

permissions_classes = []

Basically we should use request.user directly instead of get_authenticated_user, and for any custom logic, we should create a custom authentication class and add it to DRF DEFAULT_AUTHENTICATION_CLASSES. But yeah it isn't used anywhere so we don't need to worry about it.

GergiH · 2020-08-11T11:08:41Z

 )
-from rest_framework.authtoken.models import Token
-
-def create_draft(raw):


@batpad @thenav56 Removed most of these unnecessarily abstracted functions. Most of these can be found now inside the Views, ex. this one:
b8b8e9e#diff-8e4a20b18006b7a31a84250b0828251eR182

GergiH · 2020-08-11T11:09:47Z

+from api.views import bad_request
+
+
+def get_client_ip(request):


@batpad @thenav56 No clue if there's a nicer way to achieve this so I just made this a function from Zoltán's code. I don't even know why are we saving IPs here...

GergiH · 2020-08-11T11:11:02Z

+        except Exception:
            return bad_request('Could not change PER form record.')

-        if hasattr(form,'status_code') and form.status_code == 400:


@batpad @thenav56 This felt redundant to me since we're already throwing a bad_request above if it fails. Is there any scenario where we might want to keep this in?

GergiH · 2020-08-11T11:11:54Z

+        # Update Form Data of the Form
+        if data:
+            try:
+                with transaction.atomic():  # all or nothing


@batpad @thenav56 I hope I used with transaction.atomic() right. Also I guess we don't to save half-good Form Data so if one would fail, all should?

GergiH · 2020-08-11T11:13:17Z


-class DraftSent(PublicJsonPostView):
-    def handle_post(self, request, *args, **kwargs):
-        u = None


@batpad @thenav56 I guessed this part (repeating in most Views below) is just a custom way for doing:

authentication_classes = (authentication.TokenAuthentication,) permissions_classes = (permissions.IsAuthenticated,)

So I just switched them out.

GergiH · 2020-08-11T12:01:01Z

-    return form_data
-
-class FormSent(PublicJsonPostView):
-    def handle_post(self, request, *args, **kwargs):


@batpad @thenav56 This didn't need authentication before but I added it now: https://app.circleci.com/pipelines/github/IFRCGo/go-api/613/workflows/4c8458c8-22d6-491e-81e5-7f1e37800d85/jobs/2529

Should I remove the auth from this? It was weird to me why we allowed insertion without auth...

GergiH · 2020-08-12T14:44:18Z

@batpad @thenav56 Also removed PublicJsonRequestView totally, tested the affected endpoints, all good on my side.

GergiH added 2 commits August 4, 2020 10:33

Added resend_validation endpoint, view

356b085

flake8 fix

54e91e8

GergiH requested review from batpad and thenav56 August 4, 2020 08:38

GergiH mentioned this pull request Aug 4, 2020

Re-send validation email IFRCGo/go-frontend#1490

Merged

thenav56 reviewed Aug 5, 2020

View reviewed changes

GergiH added 6 commits August 6, 2020 09:37

Error handling fix

dcaa397

Change PublicJsonPostView to APIView

4847a24

flake8 fix

3b32ed0

Change PublicJsonPostView to APIView for api

3adbd95

Refactor NewRegistration to use APIView and fixes

359f0ec

Refactor of PER PublicJsonPostViews to APIViews

b8b8e9e

GergiH changed the title ~~Re-send validation email~~ Re-send validation email, refactor to APIViews Aug 11, 2020

GergiH commented Aug 11, 2020

View reviewed changes

GergiH requested a review from thenav56 August 11, 2020 11:14

GergiH commented Aug 11, 2020

View reviewed changes

GergiH added 6 commits August 11, 2020 14:19

Remove PublicJsonPostView

18cb7f3

Fix dict elements, fix PER test

4520ab4

Remove unnecessary imports, flake8 fixes

6ec1559

Removed unnecessary imports, flake8 fixes

d2593c3

EsPage Views to APIView

20d12f2

Removed PublicJsonRequestView

4fd9dc3

GergiH added 3 commits August 13, 2020 15:36

Merge branch 'develop' into feature/resend-verif-email

1d792b9

Imports and flake8 fixes

48ea1ae

Fix form_id is None

fcff707

GergiH merged commit 0005a6f into develop Aug 13, 2020

GergiH deleted the feature/resend-verif-email branch August 13, 2020 14:03

GergiH mentioned this pull request Aug 13, 2020

[PROD] Emails not getting sent to email queue - registration - intermittent issue #737

Closed

batpad mentioned this pull request Sep 21, 2020

Release v4.4.0 - Safari IFRCGo/go-frontend#1610

Merged

8 tasks

GergiH mentioned this pull request Oct 5, 2020

Refactor PublicJson views #541

Closed

	class UpdateSubscriptionPreferences(APIView):
	authentication_classes = (authentication.TokenAuthentication,)
	permissions_classes = (permissions.IsAuthenticated,)

	def post(self, request):
	errors, created = Subscription.sync_user_subscriptions(self.request.user, request.data, True) # deletePrevious
	if len(errors):
	return Response({
	'status': 400,
	'data': 'Could not create one or more subscription(s), aborting'
	})
	return Response({'data': 'Success'})

	def get_authenticated_user(self, request):
	auth_header = request.META.get('HTTP_AUTHORIZATION')
	if not auth_header:
	return None

		from api.views import bad_request


		def get_client_ip(request):

Conversation

GergiH commented Aug 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GergiH Aug 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GergiH Aug 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GergiH Aug 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GergiH commented Aug 12, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

GergiH commented Aug 4, 2020 •

edited

Loading

GergiH Aug 6, 2020 •

edited

Loading

GergiH Aug 7, 2020 •

edited

Loading

GergiH Aug 11, 2020 •

edited

Loading