raft-tech / TANF-app

Repo for development of a new TANF Data Reporting System
Other
17 stars 4 forks source link

1337 email owner as system admin group (un)assigned #3253

Open raftmsohani opened 3 weeks ago

raftmsohani commented 3 weeks ago

Summary of Changes

Provide a brief summary of changes Pull request closes #1337 _

How to Test

task up
  1. Open http://localhost:3000/ and sign in.
  2. Proceed to admin page and create a group called: "System Owner"
  3. Then add this group to one of the users
  4. Change another user's group from/to: "OFA System Admin"
  5. Check the user with system owner group receives email

The following screenshot shows an example of email template for assignment:

Screenshot 2024-11-01 at 8 48 50 AM

Deliverables

More details on how deliverables herein are assessed included here.

Deliverable 1: Accepted Features

Checklist of ACs:

Deliverable 2: Tested Code

Deliverable 3: Properly Styled Code

Deliverable 4: Accessible

Deliverable 5: Deployed

Deliverable 6: Documented

Deliverable 7: Secure

Deliverable 8: User Research

Research product(s) clearly articulate(s):

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 90.24390% with 4 lines in your changes missing coverage. Please review.

Project coverage is 91.47%. Comparing base (2b07c68) to head (307042a). Report is 3 commits behind head on develop.

Files with missing lines Patch % Lines
tdrs-backend/tdpservice/email/tasks.py 66.66% 2 Missing :warning:
tdrs-backend/tdpservice/users/signals.py 89.47% 1 Missing and 1 partial :warning:
Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253/graphs/tree.svg?width=650&height=150&src=pr&token=BA04YXPAL9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech)](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech) ```diff @@ Coverage Diff @@ ## develop #3253 +/- ## =========================================== - Coverage 91.48% 91.47% -0.01% =========================================== Files 297 298 +1 Lines 8433 8474 +41 Branches 611 615 +4 =========================================== + Hits 7715 7752 +37 - Misses 605 608 +3 - Partials 113 114 +1 ``` | [Flag](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech) | Coverage Δ | | |---|---|---| | [dev-backend](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech) | `91.33% <90.24%> (-0.01%)` | :arrow_down: | | [dev-frontend](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech) | `92.51% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files with missing lines](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech) | Coverage Δ | | |---|---|---| | [tdrs-backend/tdpservice/email/email\_enums.py](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253?src=pr&el=tree&filepath=tdrs-backend%2Ftdpservice%2Femail%2Femail_enums.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech#diff-dGRycy1iYWNrZW5kL3RkcHNlcnZpY2UvZW1haWwvZW1haWxfZW51bXMucHk=) | `100.00% <100.00%> (ø)` | | | [...nd/tdpservice/email/helpers/admin\_notifications.py](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253?src=pr&el=tree&filepath=tdrs-backend%2Ftdpservice%2Femail%2Fhelpers%2Fadmin_notifications.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech#diff-dGRycy1iYWNrZW5kL3RkcHNlcnZpY2UvZW1haWwvaGVscGVycy9hZG1pbl9ub3RpZmljYXRpb25zLnB5) | `92.59% <100.00%> (+6.87%)` | :arrow_up: | | [tdrs-backend/tdpservice/users/apps.py](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253?src=pr&el=tree&filepath=tdrs-backend%2Ftdpservice%2Fusers%2Fapps.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech#diff-dGRycy1iYWNrZW5kL3RkcHNlcnZpY2UvdXNlcnMvYXBwcy5weQ==) | `100.00% <100.00%> (ø)` | | | [tdrs-backend/tdpservice/email/tasks.py](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253?src=pr&el=tree&filepath=tdrs-backend%2Ftdpservice%2Femail%2Ftasks.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech#diff-dGRycy1iYWNrZW5kL3RkcHNlcnZpY2UvZW1haWwvdGFza3MucHk=) | `79.78% <66.66%> (-0.90%)` | :arrow_down: | | [tdrs-backend/tdpservice/users/signals.py](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253?src=pr&el=tree&filepath=tdrs-backend%2Ftdpservice%2Fusers%2Fsignals.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech#diff-dGRycy1iYWNrZW5kL3RkcHNlcnZpY2UvdXNlcnMvc2lnbmFscy5weQ==) | `89.47% <89.47%> (ø)` | | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech). Last update [455ca37...307042a](https://app.codecov.io/gh/raft-tech/TANF-app/pull/3253?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=raft-tech).

🚨 Try these New Features:

jtimpe commented 1 week ago

@raftmsohani one note i forgot to make: the description says "system admins" should receive the email, but i believe it should say "system owners"

also, the addition of the new System Owners group means that the frontend no longer shows the expected links (kibana, grafana, admin)

raftmsohani commented 1 week ago

@raftmsohani one note i forgot to make: the description says "system admins" should receive the email, but i believe it should say "system owners"

also, the addition of the new System Owners group means that the frontend no longer shows the expected links (kibana, grafana, admin)

@jtimpe I don't see anything wrong, basically when system admin group is being (un)assigned then system owner should receive email

ADPennington commented 1 week ago

Proceed to admin page and create a group called: "System Owner" Then add this group to one of the users Change another user's group from/to: "OFA System Admin" Check the user with system owner group receives email

@raftmsohani is it expected for sys admins to create this new user group? if so, why? Additionally, what permissions does this user group need?

raftmsohani commented 1 week ago

Proceed to admin page and create a group called: "System Owner" Then add this group to one of the users Change another user's group from/to: "OFA System Admin" Check the user with system owner group receives email

@raftmsohani is it expected for sys admins to create this new user group? if so, why? Additionally, what permissions does this user group need?

Instead of creating the group in migrations, we decided to create it one manually in prod, and then pull all group creations out of migrations and add it to load data command.

I thought the new SystemOwner group is what the ticket is asking. I assume same permissions as OFAAdmin??

ADPennington commented 1 week ago

Proceed to admin page and create a group called: "System Owner" Then add this group to one of the users Change another user's group from/to: "OFA System Admin" Check the user with system owner group receives email

@raftmsohani is it expected for sys admins to create this new user group? if so, why? Additionally, what permissions does this user group need?

Instead of creating the group in migrations, we decided to create it one manually in prod, and then pull all group creations out of migrations and add it to load data command.

I thought the new SystemOwner group is what the ticket is asking. I assume same permissions as OFAAdmin??

okay; is this approach simpler? if so, can you attach a video demonstrating how to do this? @raftmsohani

raftmsohani commented 1 week ago

Proceed to admin page and create a group called: "System Owner" Then add this group to one of the users Change another user's group from/to: "OFA System Admin" Check the user with system owner group receives email

@raftmsohani is it expected for sys admins to create this new user group? if so, why? Additionally, what permissions does this user group need?

Instead of creating the group in migrations, we decided to create it one manually in prod, and then pull all group creations out of migrations and add it to load data command. I thought the new SystemOwner group is what the ticket is asking. I assume same permissions as OFAAdmin??

okay; is this approach simpler? if so, can you attach a video demonstrating how to do this? @raftmsohani

It is basically manually creating a user group, but I will create a video

raftmsohani commented 1 week ago

Here is the video for creating "System Owner" user group

https://github.com/user-attachments/assets/939330a3-ff63-4095-848f-9f7ed9c0d832

raftmsohani commented 6 days ago

@ADPennington please watch the video, but you might also want to define what we need from "System Owner" group and based on that we can assign permissions