Make all users contacts by default #2710
Conversation
BCerki
force-pushed
the
2338-default-contacts
branch
from
a23f18b to
d5b307e
Compare
| user_operator.verified_by_id = admin_user_guid | ||
|
|
||
| if user_operator.status == UserOperator.Statuses.DECLINED: | ||
| # Set role to pending for now but we may want to add a new role for declined |
There was a problem hiding this comment.
I brought this over from the v1 version of the service
BCerki
force-pushed
the
2338-default-contacts
branch
from
9403b34 to
1b6360d
Compare
| subsequent_user_operator.user.get_full_name(), | ||
| subsequent_user_operator.user.email, | ||
| ) | ||
| # # Assert that the email notification was called |
There was a problem hiding this comment.
@andrea-williams , the email tests fail now that I've created a v2 version of the service. Does that make sense?
There was a problem hiding this comment.
hmmm I'm not sure. Will investigate this more tmr.
There was a problem hiding this comment.
tbh I'm feeling suspicious of this. I played around with a code a bit locally and was getting the same failure where it's not being called, but I can't think why. @Sepehr-Sobhani do you have time to take a quick look and see if you can spot what's going on?
There was a problem hiding this comment.
if it's not a quick debug/fix, I'm happy to leave it for now and call it a later problem, since we're not ready to implement the email notifications for Reg2 yet anyway.
There was a problem hiding this comment.
Instead of service.user_operator_service.send_operator_access_request_email use service.user_operator_service_v2.send_operator_access_request_email and it will work.
BCerki
force-pushed
the
2338-default-contacts
branch
from
c1031ed to
3efa6db
Compare
| subsequent_user_operator.user.get_full_name(), | ||
| subsequent_user_operator.user.email, | ||
| ) | ||
| # # Assert that the email notification was called |
There was a problem hiding this comment.
hmmm I'm not sure. Will investigate this more tmr.
| user_operator.verified_at = datetime.now(ZoneInfo("UTC")) | ||
| user_operator.verified_by_id = admin_user_guid |
There was a problem hiding this comment.
could use set_verification_columns() here instead
BCerki
force-pushed
the
2338-default-contacts
branch
from
fe65b29 to
63a770f
Compare
There was a problem hiding this comment.
Ran locally. Logged in as bc-cas-dev and confirmed that after approving bc-cas-dev-secondary as a user, they automatically appeared in the Contact table when they weren't there before.
Would recommend adding testing notes in either the PR or the card that basically explain that ^. Feedback received from POs is that they sometimes have to spend a long time figuring out how to test a PR before they can approve it, so testing notes like this could probably help.
Co-authored-by: Andrea Williams <[email protected]>
BCerki
force-pushed
the
2338-default-contacts
branch
from
63a770f to
e036fff
Compare
card: #2338
This PR:
I'd like to update the fixtures so that existing users have a contact record, but I need to wait for https://github.com/orgs/bcgov/projects/123/views/16?pane=issue&itemId=88758186&issue=bcgov%7Ccas-registration%7C2524 to get merged to avoid conflicts. If it's not merged before this PR is ready, I'll do a separate PR to update the mock dataI'll update mock data in https://github.com/orgs/bcgov/projects/123/views/16?pane=issue&itemId=93581679&issue=bcgov%7Ccas-registration%7C2680 actually, to avoid rework