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.

Sign up for GitHub

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

Jump to bottom

DWPF-704 update dw usage of feedback package and implement custom notifications #433

Merged

nicopicchio merged 24 commits into main from DWPF-704-update-dw-usage-of-feedback-package-and-implement-custom-notifications

Sep 7, 2023

Contributor

nicopicchio commented Aug 29, 2023

No description provided.

nicopicchio added 8 commits

August 22, 2023 12:09


          updates django-feedback-govuk to latest

7d3ae76


          Merge branch 'main' into DWPF-704-update-dw-usage-of-feedback-package…

d95cf6c

…-and-implement-custom-notifications


          creates method to retrieve feedback from the last 24hrs

7aa0c24


          linter changes

491d9bd


          updates django-feedback-govuk version in requirements.txt

3ce3502


          defines method to send email notifications, adds env variables and task

bc42c78


          linter fix

ff40ae1


          schedules feedback email notification task to be sent daily at 06:00

39afe4f

nicopicchio marked this pull request as ready for review

August 30, 2023 08:38

nicopicchio requested a review from CamLamb

August 30, 2023 08:38

CamLamb reviewed

View reviewed changes

src/feedback/utils.py Outdated Show resolved Hide resolved

CamLamb reviewed

View reviewed changes

src/feedback/utils.py Outdated Show resolved Hide resolved

CamLamb reviewed

View reviewed changes

src/feedback/utils.py Outdated Show resolved Hide resolved

CamLamb reviewed

View reviewed changes

src/feedback/utils.py Outdated Show resolved Hide resolved

CamLamb reviewed

View reviewed changes

Contributor

CamLamb left a comment

When we merge PRs, their title become the commit message on the main branch.
Since this is the case, I like to make sure the ticket number is in the format that will be picked up by Jira:

Dwpf 704 > DWPF-704

nicopicchio changed the title ~~Dwpf 704 update dw usage of feedback package and implement custom notifications~~ DWPF-704-update-dw-usage-of-feedback-package-and-implement-custom-notifications

Contributor Author

nicopicchio commented Aug 30, 2023

When we merge PRs, their title become the commit message on the main branch. Since this is the case, I like to make sure the ticket number is in the format that will be picked up by Jira:

Dwpf 704 > DWPF-704

I am not sure why the title gets changed. I assumed the PR was getting the same name I give to the branch.

nicopicchio added 2 commits

August 30, 2023 11:20


          makes changes based on Cam's suggestions

939dfcb


          Merge branch 'main' into DWPF-704-update-dw-usage-of-feedback-package…

fc95bf4

…-and-implement-custom-notifications

nicopicchio changed the title ~~DWPF-704-update-dw-usage-of-feedback-package-and-implement-custom-notifications~~ DWPF-704 update dw usage of feedback package and implement custom notifications

nicopicchio and others added 10 commits

September 4, 2023 15:32


          adds tests to check that feedback_received_within() works as expected

911d93e


          renames method in test file

14765d3


          linter fix

0d65dea


          updates WAGTAIL_BASE_URL in .env.ci and .env.example files

c8f2c68


          Merge branch 'main' into DWPF-704-update-dw-usage-of-feedback-package…

48369ab

…-and-implement-custom-notifications


          adds code and tests to check for settings and email recipients

32c42c2


          Merge remote-tracking branch 'origin/DWPF-704-update-dw-usage-of-feed…

6f8b609

…back-package-and-implement-custom-notifications' into DWPF-704-update-dw-usage-of-feedback-package-and-implement-custom-notifications


          shuffles code and moves comments inside test methods

bb9e938


          send_feedback_notification refactor and adds more tests

929b946


          adds test to check that send_email_notification is called once per email

1805f09

nicopicchio and others added 2 commits

September 6, 2023 17:49


          renames calls to expected_calls

32d7388


          Merge branch 'main' into DWPF-704-update-dw-usage-of-feedback-package…

9c21fa3

…-and-implement-custom-notifications

nicopicchio requested a review from marcelkornblum

September 7, 2023 09:01

marcelkornblum approved these changes

View reviewed changes

Contributor

marcelkornblum left a comment

Looks good! There are a few things in the tests I'd ideally improve but no blockers

src/feedback/test/test_utils.py Show resolved Hide resolved

src/feedback/test/test_utils.py Show resolved Hide resolved

src/feedback/test/test_utils.py

		freezer.move_to("2023-09-03")
		assert not feedback_received_within()

Contributor

marcelkornblum Sep 7, 2023

the following ones are all fine, but very repetitive - if you want you could consider parameterising them to make it a bit more concise and (IMO) easier to see what's happening between the tests. There's examples of this in DW somewhere if you search around.

nicopicchio added 2 commits

September 7, 2023 12:27


          modifies tests for conciseness

4e14c87


          linter fix

44c675c

nicopicchio merged commit 46d7e3a into main

1 check passed

nicopicchio deleted the DWPF-704-update-dw-usage-of-feedback-package-and-implement-custom-notifications branch

September 7, 2023 11:46

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet