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: dotmailer views #69

Merged
merged 14 commits into from
Jul 12, 2023
Merged

feat: dotmailer views #69

merged 14 commits into from
Jul 12, 2023

Conversation

cewei8483
Copy link
Contributor

@cewei8483 cewei8483 commented Jul 7, 2023

This change is Reviewable

@cewei8483 cewei8483 linked an issue Jul 7, 2023 that may be closed by this pull request
Copy link
Contributor

@KamilPawel KamilPawel left a comment

Choose a reason for hiding this comment

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

Reviewed 11 of 82 files at r1, 69 of 69 files at r2, all commit messages.
Reviewable status: 80 of 82 files reviewed, 8 unresolved discussions (waiting on @cewei8483)


backend/Pipfile line 39 at r2 (raw file):

google-cloud-container = "==2.3.0"
"django-anymail[amazon_ses]" = "==7.0.*"
django-cors-headers = "*"

Use pip freeze to get the current version of django-cors-headers and set it to that version :)


backend/portal/views/dotmailer.py line 16 at r2 (raw file):

import json

@csrf_exempt

I do think it is easier to disable csrf in settings rather than for every view?


backend/portal/views/dotmailer.py line 24 at r2 (raw file):

            validate_email(user_email)
        except ValidationError:
            return JsonResponse(status=200, data={'success': False})

If validation is false should possible return a bad request; code 400?


backend/portal/views/dotmailer.py line 32 at r2 (raw file):

@csrf_exempt

Same here


backend/portal/views/dotmailer.py line 48 at r2 (raw file):

            return JsonResponse(status=200, data={'success': True})
        
    return HttpResponse(status=405)

New empty line at the end of the file


backend/service/settings.py line 72 at r2 (raw file):

    "corsheaders.middleware.CorsMiddleware",
    "django.middleware.common.CommonMiddleware",
    "django.middleware.csrf.CsrfViewMiddleware",

if you comment this out, it will disable csrf

Code quote:

 "django.middleware.csrf.CsrfViewMiddleware",

frontend/src/app/api.ts line 10 at r2 (raw file):

  FetchBaseQueryError
} from '@reduxjs/toolkit/query';
import { useNavigate } from 'react-router-dom';

Why did you remove it and used window object?


frontend/src/features/footer/SignUp.tsx line 41 at r2 (raw file):

  const handleSubmit = (values: SignUpValues, { setSubmitting }: FormikHelpers<SignUpValues>): void => {
    setSubmitting(false);

setSubmitting should be done if the request fails, so in the catch block. Otherwise if we submit and something goes wrong and the form in some way relies on this, the user will not be able to submit

Copy link
Contributor

@KamilPawel KamilPawel left a comment

Choose a reason for hiding this comment

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

Reviewable status: 80 of 82 files reviewed, 9 unresolved discussions (waiting on @cewei8483)


frontend/src/app/api.ts line 15 at r2 (raw file):

const baseQuery = fetchBaseQuery({
  // baseUrl: process.env.REACT_APP_API_BASE_URL
  baseUrl: 'http://localhost:8000/'

Should ideally overwrite value in the .env file 🤔

Code quote:

  // baseUrl: process.env.REACT_APP_API_BASE_URL
  baseUrl: 'http://localhost:8000/'

Copy link
Contributor Author

@cewei8483 cewei8483 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 77 of 82 files reviewed, 9 unresolved discussions (waiting on @KamilPawel)


backend/Pipfile line 39 at r2 (raw file):

Previously, KamilPawel wrote…

Use pip freeze to get the current version of django-cors-headers and set it to that version :)

Done.


backend/portal/views/dotmailer.py line 16 at r2 (raw file):

Previously, KamilPawel wrote…

I do think it is easier to disable csrf in settings rather than for every view?

This one is there in the old repo so I'll keep it.


backend/portal/views/dotmailer.py line 24 at r2 (raw file):

Previously, KamilPawel wrote…

If validation is false should possible return a bad request; code 400?

If the BE returns 400 it will be redirected to error pages, which is not our intended behaviour.


backend/portal/views/dotmailer.py line 32 at r2 (raw file):

Previously, KamilPawel wrote…

Same here

Done.


backend/portal/views/dotmailer.py line 48 at r2 (raw file):

Previously, KamilPawel wrote…

New empty line at the end of the file

Done.


backend/service/settings.py line 72 at r2 (raw file):

Previously, KamilPawel wrote…

if you comment this out, it will disable csrf

Got it. But I think we should keep it enabled on main branch?


frontend/src/app/api.ts line 10 at r2 (raw file):

Previously, KamilPawel wrote…

Why did you remove it and used window object?

If you use hook inside this, it will complain about Invalid hook call since RTK query is already using hook to wrap it up. I discussed with Stefan the other day about this and he suggested using window object to avoid hook.


frontend/src/app/api.ts line 15 at r2 (raw file):

Previously, KamilPawel wrote…

Should ideally overwrite value in the .env file 🤔

Done.


frontend/src/features/footer/SignUp.tsx line 41 at r2 (raw file):

Previously, KamilPawel wrote…

setSubmitting should be done if the request fails, so in the catch block. Otherwise if we submit and something goes wrong and the form in some way relies on this, the user will not be able to submit

I thought once the form is submitted, isSubmitted should be set to false?

Copy link
Contributor

@KamilPawel KamilPawel left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 82 files at r1, 2 of 3 files at r3, all commit messages.
Reviewable status: 80 of 82 files reviewed, 1 unresolved discussion (waiting on @cewei8483)


backend/service/settings.py line 72 at r2 (raw file):

Previously, cewei8483 (Chi-En Wei) wrote…

Got it. But I think we should keep it enabled on main branch?

Yea for dev purposes :)


frontend/src/features/footer/SignUp.tsx line 41 at r2 (raw file):

Previously, cewei8483 (Chi-En Wei) wrote…

I thought once the form is submitted, isSubmitted should be set to false?

I was thinking that once the submission is done, then you would set submission to false right ?

Copy link
Contributor

@KamilPawel KamilPawel left a comment

Choose a reason for hiding this comment

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

Reviewable status: 80 of 82 files reviewed, 1 unresolved discussion (waiting on @cewei8483)


frontend/src/app/api.ts line 15 at r2 (raw file):

Previously, cewei8483 (Chi-En Wei) wrote…

Done.

👍

Copy link
Contributor Author

@cewei8483 cewei8483 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 76 of 82 files reviewed, 1 unresolved discussion (waiting on @KamilPawel)


frontend/src/features/footer/SignUp.tsx line 41 at r2 (raw file):

Previously, KamilPawel wrote…

I was thinking that once the submission is done, then you would set submission to false right ?

Done.

Copy link
Contributor

@KamilPawel KamilPawel left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 3 files at r3, 1 of 1 files at r4, 4 of 4 files at r5.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on @cewei8483)

Copy link
Contributor

@KamilPawel KamilPawel left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @cewei8483)


backend/portal/urls.py line 9 at r5 (raw file):

    url(r"^api/news_signup/$", process_newsletter_form, name="process_newsletter_form"),
    url(r"^api/consent_form/$", dotmailer_consent_form, name="consent_form"),
    url(r".*", render_react, name="react_app"),

I tested the api views, and the react one is still being called when pulling?

Copy link
Contributor

@KamilPawel KamilPawel left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @cewei8483)


frontend/src/app/api.ts line 49 at r5 (raw file):

        url: 'news_signup/',
        method: 'POST',
        body: payload

Did we say we are going with JSON or QS encoding?

Copy link
Contributor

@KamilPawel KamilPawel left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 5 of 5 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @cewei8483)

@KamilPawel KamilPawel merged commit 24692fd into development Jul 12, 2023
@KamilPawel KamilPawel deleted the 18-dotmailer-views branch July 12, 2023 09:45
faucomte97 pushed a commit to faucomte97/codeforlife-portal-backend that referenced this pull request Dec 17, 2024
# 1.0.0 (2024-12-06)

### Bug Fixes

* 105 new portal edits loginteacher ([ocadotechnology#113](ocadotechnology#113)) ([80339ed](ocadotechnology@80339ed))
* 2044 register page ([ocadotechnology#26](ocadotechnology#26)) ([78477e2](ocadotechnology@78477e2))
* 2045 home page ([ocadotechnology#6](ocadotechnology#6)) ([681e86e](ocadotechnology@681e86e))
* 2054 home learning page ([ocadotechnology#8](ocadotechnology#8)) ([5cbb7d5](ocadotechnology@5cbb7d5))
* 2059 http error pages ([ocadotechnology#9](ocadotechnology#9)) ([006f31f](ocadotechnology@006f31f))
* 91 new portal edits student  ([ocadotechnology#120](ocadotechnology#120)) ([3d2bab5](ocadotechnology@3d2bab5))
* 93 new portal edits teacherdashboardaccount ([ocadotechnology#123](ocadotechnology#123)) ([e27cea2](ocadotechnology@e27cea2))
* 99 new portal edit teacherdashboardclassesab123movestudentids=1 ([ocadotechnology#128](ocadotechnology#128)) ([6467467](ocadotechnology@6467467))
* about us page ([ocadotechnology#3](ocadotechnology#3)) ([4215826](ocadotechnology@4215826))
* add new logic ([ocadotechnology#79](ocadotechnology#79)) ([20ef876](ocadotechnology@20ef876))
* add NewsLetter page ([ocadotechnology#10](ocadotechnology#10)) ([656de69](ocadotechnology@656de69))
* add self as a media source ([ocadotechnology#121](ocadotechnology#121)) ([be6f39a](ocadotechnology@be6f39a))
* added padding teacher backup token ([ocadotechnology#141](ocadotechnology#141)) ([d589833](ocadotechnology@d589833))
* added spacings and the copy to clipboard functionality  ([ocadotechnology#127](ocadotechnology#127)) ([d95ca66](ocadotechnology@d95ca66))
* altered the padding to match prod ([ocadotechnology#138](ocadotechnology#138)) ([931c681](ocadotechnology@931c681))
* changed the header from 'Welcome' to 'Log in as a teacher' ([ocadotechnology#149](ocadotechnology#149)) ([6c4947f](ocadotechnology@6c4947f))
* CodingClub page ([ocadotechnology#5](ocadotechnology#5)) ([ba8f2d6](ocadotechnology@ba8f2d6))
* error pages ([22f0802](ocadotechnology@22f0802))
* fix padding ([ocadotechnology#129](ocadotechnology#129)) ([0a973e8](ocadotechnology@0a973e8)), closes [#1](https://github.com/ocadotechnology/codeforlife-portal-backend/issues/1)
* footer ([669e592](ocadotechnology@669e592))
* footer feedback ([fd618e2](ocadotechnology@fd618e2))
* general navigation ([01b7638](ocadotechnology@01b7638))
* get involved feedback ([1fb6d37](ocadotechnology@1fb6d37))
* get involved page ([ocadotechnology#4](ocadotechnology#4)) ([2a6a109](ocadotechnology@2a6a109))
* header ([bfdb306](ocadotechnology@bfdb306))
* home learning page feedback ([51295b8](ocadotechnology@51295b8))
* images contribute feedback ([55dfc63](ocadotechnology@55dfc63))
* install cfl package dev extra ([ocadotechnology#298](ocadotechnology#298)) ([552fbfe](ocadotechnology@552fbfe))
* new About us page edits ([ocadotechnology#104](ocadotechnology#104)) ([8226f01](ocadotechnology@8226f01))
* new portal edits - /student/dashboard/independent/join ([ocadotechnology#114](ocadotechnology#114)) ([1a47514](ocadotechnology@1a47514))
* new portal edits - /teacher/dashboard/account/setup-2fa ([ocadotechnology#119](ocadotechnology#119)) ([7c59b88](ocadotechnology@7c59b88))
* New portal edits - Header ([ocadotechnology#102](ocadotechnology#102)) ([0476c1a](ocadotechnology@0476c1a))
* new portal edits - table greys ([ocadotechnology#80](ocadotechnology#80)) ([845cba1](ocadotechnology@845cba1))
* New portal edits - teacher/2FA ([ocadotechnology#139](ocadotechnology#139)) ([beb2ac6](ocadotechnology@beb2ac6))
* padding and default ticked levels ([ocadotechnology#126](ocadotechnology#126)) ([0d7c5f9](ocadotechnology@0d7c5f9)), closes [ocadotechnology#2](ocadotechnology#2)
* pipeline tests ([9b8de6a](ocadotechnology@9b8de6a))
* spacing ([ocadotechnology#75](ocadotechnology#75)) ([4facbd8](ocadotechnology@4facbd8))
* themedbox and close menu on details button click ([078e968](ocadotechnology@078e968))
* tsconfig ([0afbb17](ocadotechnology@0afbb17))
* update cfl js package version ([ocadotechnology#148](ocadotechnology#148)) ([9664ec4](ocadotechnology@9664ec4))
* update password ([ocadotechnology#314](ocadotechnology#314)) ([52268fc](ocadotechnology@52268fc))
* Update URLs in cron jobs ([ocadotechnology#168](ocadotechnology#168)) ([7519ee0](ocadotechnology@7519ee0))
* use recursive path generator and normalize path attributes ([de837e8](ocadotechnology@de837e8))
* use recursive path generator and normalize path attributes ([ocadotechnology#38](ocadotechnology#38)) ([3137425](ocadotechnology@3137425))
* y overflow ([ce6fe5d](ocadotechnology@ce6fe5d))

### Features

* 158 teacher teach edit student details ([ocadotechnology#185](ocadotechnology#185)) ([1435e1f](ocadotechnology@1435e1f)), closes [#1](https://github.com/ocadotechnology/codeforlife-portal-backend/issues/1) [ocadotechnology#2](ocadotechnology#2) [ocadotechnology#2](ocadotechnology#2) [ocadotechnology#2](ocadotechnology#2) [ocadotechnology#3](ocadotechnology#3) [ocadotechnology#3](ocadotechnology#3) [ocadotechnology#4](ocadotechnology#4) [ocadotechnology#5](ocadotechnology#5) [ocadotechnology#5](ocadotechnology#5)
* 16 organisation views ([ocadotechnology#147](ocadotechnology#147)) ([2d8219f](ocadotechnology@2d8219f))
* 17 email views ([ocadotechnology#76](ocadotechnology#76)) ([15986b8](ocadotechnology@15986b8))
* 19 api views ([ocadotechnology#118](ocadotechnology#118)) ([7271f19](ocadotechnology@7271f19))
* 2039 teachers page ([ocadotechnology#32](ocadotechnology#32)) ([b2794e7](ocadotechnology@b2794e7))
* 2042 studnents page ([ocadotechnology#7](ocadotechnology#7)) ([a6fcab7](ocadotechnology@a6fcab7))
* 2055 privacy notice page ([ocadotechnology#34](ocadotechnology#34)) ([855fc66](ocadotechnology@855fc66))
* 2056 terms of use page ([ocadotechnology#31](ocadotechnology#31)) ([7fdc868](ocadotechnology@7fdc868))
* 2058 students + independents auth pages ([ocadotechnology#30](ocadotechnology#30)) ([20b6157](ocadotechnology@20b6157))
* 35 teachers onboarding ([ocadotechnology#36](ocadotechnology#36)) ([3f9213b](ocadotechnology@3f9213b))
* adapted home.py to work with our current frontend ([ocadotechnology#77](ocadotechnology#77)) ([628aebb](ocadotechnology@628aebb))
* added 3 tabs on teachers dashboard page ([ocadotechnology#29](ocadotechnology#29)) ([ad04ead](ocadotechnology@ad04ead))
* adding get-students-endpoint ([ocadotechnology#184](ocadotechnology#184)) ([f842ad7](ocadotechnology@f842ad7)), closes [#1](https://github.com/ocadotechnology/codeforlife-portal-backend/issues/1) [ocadotechnology#2](ocadotechnology#2)
* Bundle front end and add Django backend ([ocadotechnology#27](ocadotechnology#27)) ([48b1c95](ocadotechnology@48b1c95))
* cypress plus codecov ([ocadotechnology#2](ocadotechnology#2)) ([91dc301](ocadotechnology@91dc301))
* dotmailer views ([ocadotechnology#69](ocadotechnology#69)) ([24692fd](ocadotechnology@24692fd))
* integrated the score view for the students and student login ([ocadotechnology#150](ocadotechnology#150)) ([2bf397a](ocadotechnology@2bf397a)), closes [#1](https://github.com/ocadotechnology/codeforlife-portal-backend/issues/1)
* login page ([ocadotechnology#28](ocadotechnology#28)) ([638179a](ocadotechnology@638179a))
* otp deploy ([ocadotechnology#370](ocadotechnology#370)) ([46e68b3](ocadotechnology@46e68b3))
* page agnostic features ([ocadotechnology#37](ocadotechnology#37)) ([8eb3c9a](ocadotechnology@8eb3c9a))
* teacher student management ([ocadotechnology#42](ocadotechnology#42)) ([5521a95](ocadotechnology@5521a95))
* teacher teach delete class ([ocadotechnology#183](ocadotechnology#183)) ([f7f9ef9](ocadotechnology@f7f9ef9))
* teacher teach delete student ([ocadotechnology#186](ocadotechnology#186)) ([a71c881](ocadotechnology@a71c881))
* teacher views classes dashboard ([ocadotechnology#180](ocadotechnology#180)) ([985bcc1](ocadotechnology@985bcc1))
* teacher views dashboard ([ocadotechnology#173](ocadotechnology#173)) ([df0dbcb](ocadotechnology@df0dbcb))
* update details, delete account, disable 2fa ([ocadotechnology#179](ocadotechnology#179)) ([a4a8e59](ocadotechnology@a4a8e59)), closes [#1](https://github.com/ocadotechnology/codeforlife-portal-backend/issues/1) [ocadotechnology#2](ocadotechnology#2) [ocadotechnology#3](ocadotechnology#3) [ocadotechnology#3](ocadotechnology#3) [ocadotechnology#4](ocadotechnology#4)
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.

dotmailer views
2 participants