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

Notifications model and goods report notification #1036

Merged
merged 17 commits into from
Sep 20, 2023

Conversation

a-gleeson
Copy link
Contributor

@a-gleeson a-gleeson commented Sep 14, 2023

TP2000-975 Notifications model and goods report notification

Why

The purpose of the story was to implement a notification to send to channel islands onces a goods report has been reviewed. This is a synchronously sent notification that should only be sent once, which means we need some sort of way to track if a notification has been triggered. The current implmenetation of notifications made this difficult.
We will also be expanding notificaitons as we bring in other users and implement a larger workflow.

What

  • Notify module for gov notify specific functions
  • Introduced a generic Notfiication model which implements
    • Has field notified_object_pk (ok of object which notification relates to)
    • Has field notification_type which is a test choices fields for the different type of notificaitons
    • send_notification_emails() sends the email via the notify module and creates notification logs tied to this notification object
    • return_subclass_instance() returns the subclass instance of the notfication object
    • synchronous_send_emails() triggers celery task directly
    • schedule_send_emails() triggers celery task asyncly
  • Proxy Notification Sub classes implemented for each type of notifiaction
    • has it's own init to set notification type
    • has it's own method to retrieve linked object
    • has it's own email personalistation which it gets details from linked object
    • maintains it's own gov notify template id
    • has it's own notfied_user() function to retrieve it's email list
    • GoodsSuccessfulImportNotification get_personalisation() calls notify.prepare_link_to_file() to prepare attachment on email
  • Tests to cover this model and the celery task trigger
  • A new conformation screen to confirm notification for good report sent
  • A view and button on the goods report tab to send the notification
  • If an import is a commodity import disable the send to packaging button until a goods report notification is sent
  • Tests to cover the views

NotificationProxyModel(1)
image
image
image
Button disappears after notfiication sent
image
image
image

Checklist

  • Requires migrations? - Ues

Links to relevant material
See: Options for notification model

@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2023

Codecov Report

Patch coverage: 83.52% and project coverage change: -0.07% ⚠️

Comparison is base (6833b1b) 92.73% compared to head (fb7b5dc) 92.66%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1036      +/-   ##
==========================================
- Coverage   92.73%   92.66%   -0.07%     
==========================================
  Files         453      458       +5     
  Lines       33126    33532     +406     
  Branches     2540     2558      +18     
==========================================
+ Hits        30719    31073     +354     
- Misses       1918     1969      +51     
- Partials      489      490       +1     
Files Changed Coverage Δ
importer/urls.py 100.00% <ø> (ø)
notifications/forms.py 100.00% <ø> (ø)
publishing/tests/conftest.py 100.00% <ø> (ø)
publishing/tests/test_envelope_queue_views.py 81.30% <ø> (ø)
notifications/notify.py 36.66% <36.66%> (ø)
notifications/admin.py 68.96% <65.38%> (-2.91%) ⬇️
workbaskets/tests/test_views.py 96.07% <65.51%> (-3.20%) ⬇️
notifications/models.py 77.30% <75.97%> (-13.61%) ⬇️
workbaskets/views/ui.py 85.49% <87.50%> (+0.04%) ⬆️
conftest.py 94.51% <96.15%> (+0.11%) ⬆️
... and 14 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

notifications/models.py Outdated Show resolved Hide resolved
notifications/models.py Outdated Show resolved Hide resolved
notifications/models.py Outdated Show resolved Hide resolved
workbaskets/tests/test_views.py Show resolved Hide resolved
workbaskets/tests/test_views.py Show resolved Hide resolved
Copy link
Collaborator

@CPrich905 CPrich905 left a comment

Choose a reason for hiding this comment

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

Nice work! Looks good to me but might want a second look over as it's quite a big piece of work

@a-gleeson a-gleeson merged commit bec000a into master Sep 20, 2023
3 checks passed
@a-gleeson a-gleeson deleted the EXPERIMENTAL--notifications-reimagined branch September 20, 2023 15:39
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.

4 participants