Resending invites #438
Resending invites #438
Conversation
…tus-changes' into hy/resend-invite
|
Test failure looks like a real migration issue |
hemant10yadav
force-pushed
the
hy/resend-invite
branch
from
d005800 to
d4cf965
Compare
# Conflicts: # commcare_connect/opportunity/tables.py
| @@ -115,6 +119,7 @@ def invite_user(user_id, opportunity_access_id): | |||
| opportunity_access=opportunity_access, | |||
| defaults={ | |||
| "message_sid": sms_status.sid, | |||
| "notification_date": now() if sms_status.sid else None, | |||
There was a problem hiding this comment.
What is the reason for this check?
There was a problem hiding this comment.
If the SID is not present, then the SMS is not sent, right? And if the SMS is not sent, we do not set the timestamp, so the user can trigger it again before 24 hours. Otherwise, if we set it to now() every time, the user won't be able to send the SMS again before 24 hours, even though the SMS was not sent earlier.
There was a problem hiding this comment.
The SID will be present for all messages, even when no message was sent. The only case where the SID might not be present is when Twilio fails to register the SMS. A better way to set this notification_date would be in the SMSStatusCallbackView when Twilio responds with a "Delivered" status response.
There was a problem hiding this comment.
Thanks changed 65235ea
| @@ -568,4 +570,23 @@ <h1 class="modal-title fs-5 text-white">{% translate "Import Catchment Areas" %} | |||
| </div> | |||
| </div> | |||
|
|
|||
| <div class="modal fade" id="resendInviteModal" tabindex="-1" aria-labelledby="resendInviteModalLabel" aria-hidden="true"> | |||
There was a problem hiding this comment.
Can we change the current modal implementation to bootstrap alerts to show any responses returned from the server? This will keep the UX similar across the website.
There was a problem hiding this comment.
I am happy to do that. I assume it would be shown at the top level, but the problem is that when the user scrolls down, they won't see the alert unless they manually scroll back up to the top, or we trigger the scroll.
|
Pawan's comments seem right and I would encourage you to follow them, but nothing blocking on my end. |
CCCT-547