feat: remind pending assignment functionality #325
Conversation
kiram15
force-pushed
the
kiram15/ENT-7691
branch
from
3e3524a to
c3f1d30
Compare
kiram15
force-pushed
the
kiram15/ENT-7691
branch
from
c3f1d30 to
fa33bfa
Compare
kiram15
force-pushed
the
kiram15/ENT-7691
branch
from
9aaca68 to
98b0bc8
Compare
| braze_trigger_properties["course_title"] = assignment.content_title | ||
| braze_trigger_properties["enrollment_deadline"] = course_metadata['normalized_metadata']['enroll_by_date'] | ||
| braze_trigger_properties["start_date"] = course_metadata['normalized_metadata']['start_date'] | ||
| braze_trigger_properties["course_partner"] = course_metadata['owners'][0]['name'] |
There was a problem hiding this comment.
This is only taking the first owner's name to send, is that an OK default to have? Or should I create a method that loops through if there's multiple and do "Harvard and Stanford" or something?
There was a problem hiding this comment.
I think that's fine for now. We can circle back on this with relative ease in the future.
There was a problem hiding this comment.
FWIW, we generally show all partners (when there are multiple) on the course cards in the UIs, comma-separated. I feel we should likely include all partner names in the email, for consistency with what's shown in the UI for admins/learners (similar to what you suggested, Kira). Main concern is that don't tackle this, we'd intentionally be favoring one course partner over another, which isn't great.
If supporting multiple partners is deferred for this PR, I feel it should be tackled as a follow-up for sure.
There was a problem hiding this comment.
Ah, that's a good point!
kiram15
force-pushed
the
kiram15/ENT-7691
branch
from
f1275b3 to
dc5fc17
Compare
enterprise_access/apps/api/v1/views/content_assignments/assignments_admin.py
Outdated
Show resolved
Hide resolved
enterprise_access/apps/api/v1/views/content_assignments/assignments_admin.py
Outdated
Show resolved
Hide resolved
| braze_trigger_properties["course_title"] = assignment.content_title | ||
| braze_trigger_properties["enrollment_deadline"] = course_metadata['normalized_metadata']['enroll_by_date'] | ||
| braze_trigger_properties["start_date"] = course_metadata['normalized_metadata']['start_date'] | ||
| braze_trigger_properties["course_partner"] = course_metadata['owners'][0]['name'] |
There was a problem hiding this comment.
FWIW, we generally show all partners (when there are multiple) on the course cards in the UIs, comma-separated. I feel we should likely include all partner names in the email, for consistency with what's shown in the UI for admins/learners (similar to what you suggested, Kira). Main concern is that don't tackle this, we'd intentionally be favoring one course partner over another, which isn't great.
If supporting multiple partners is deferred for this PR, I feel it should be tackled as a follow-up for sure.
kiram15
force-pushed
the
kiram15/ENT-7691
branch
from
c7d7d1f to
928d391
Compare
enterprise_access/apps/api/v1/views/content_assignments/assignments_admin.py
Outdated
Show resolved
Hide resolved
enterprise_access/apps/api/v1/views/content_assignments/assignments_admin.py
Outdated
Show resolved
Hide resolved
| if len(assignments) < len(request.data['assignment_uuids']): | ||
| return Response(status=status.HTTP_404_NOT_FOUND) |
There was a problem hiding this comment.
This seems like a check we'd want to do before calling cancel_assignments(), right? If this condition were true, it'd mean we couldn't find all of the assignments given the assignment uuids provided by the caller in our filter() above.
There was a problem hiding this comment.
No, because if we call it before then if any of the assignments are not found, it will just return before it hit the cancel rather than canceling the remaining ones.
There was a problem hiding this comment.
for posterity: we decided offline to leave the condition as-is, but change the response status to 422 here.
| if response.get('non_cancelable'): | ||
| return Response(status=status.HTTP_422_UNPROCESSABLE_ENTITY) |
There was a problem hiding this comment.
The assignments in response['non_cancelable'] aren't related to errors, really - they are just the assignments that are not in a cancellable state (e.g. they're already cancelled, or they're already accepted). I might change this around a little to be like
if not response.get('cancelled'):
return Response(status=422)There was a problem hiding this comment.
With this I know they're not related to errors, but wouldn't it mean that some of the uuids were unprocessable?
There was a problem hiding this comment.
for posterity: we decided offline to leave this condition and response as-is. If we need more fine-grained detail about un-cancellable assignments later, we can modify.
enterprise_access/apps/api/v1/views/content_assignments/assignments_admin.py
Outdated
Show resolved
Hide resolved
…ss into kiram15/ENT-7691
|
closing in favor of the other PR with commits all squashed down |
Description
Create a remind functionality that the admin of an enterprise learner credit wants to remind them of their pending assignment. The learner will receive a email with course details of their pending assignment. This PR is similar to Katrina's here but for the remind functionality
Solution
Add a celery task to the send out a braze email campaign to the learner to remind them
JIRA Ticket