Setting a m2m relationship works but not right away

[BrockHallenbeck]

When i do object.m2mfield.set(), it ultimately works but it doesn’t seem to be available right way.

My model has a ‘last_assigned_to’ field which is a many to many field to the User model. It gets set manually in save_model(). If I create a new record through the admin, the field itself does get set, this can be verified on the change_list and in the change_form.

However, I am trying to send emails to each user in that m2m relationship in save_model() immediately after assigning users using instance.last_assigned_to.set(), and found that the manager returns an empty queryset when accessed. I noticed the emails weren’t being sent, and backtracked to where I was setting the m2m relationship. I raise an exception just to see the stack trace and can confirm that ‘x’ is in fact an empty queryset.

Initially I thought this was the classic case of needing to save the instance before doing m2m stuff, but shouldn’t the call to super().save_model() take care of this or have I misunderstood something?

    def save_model(self, request, obj, form, change):
        super().save_model(request, obj, form, change)
        try:
            obj.advance()
            self._notify_assignee(request, obj)
        except ImproperlyConfigured:
            self.message_user(request, 'This form has not been configured correctly. It has been auto denied. Please contact IT', messages.ERROR)

    def advance(self):
        workflow = self.workflow #the object that maps a form to a workflow
        
        steps = [
            (workflow.supervisor_required,    self.supervisor_approved,    self.created_by.supervisor,         FORM_STATUS.PENDING_SUPERVISOR),
            (workflow.div_head_required,      self.div_head_approved,      self.created_by.division_head,      FORM_STATUS.PENDING_DIV_HEAD),
            (workflow.dept_head_required,     self.dept_head_approved,     self.created_by.department_head,    FORM_STATUS.PENDING_DEPT_HEAD),
            (workflow.hr_contacts.all(),            self.hr_approved,            workflow.hr_contacts.all(),           FORM_STATUS.PENDING_HR),
            (workflow.town_manager_contacts.all(),  self.town_manager_approved,  workflow.town_manager_contacts.all(), FORM_STATUS.PENDING_TOWN_MANAGER),
        ]

        for required, approved, assignee, status in steps:
            if not required or not assignee:
                continue
            if approved is None: #Approval does not exist
                assignees = [assignee] if isinstance(assignee, User) else assignee
                self.last_assigned_to.set(assignees)
                x = self.last_assigned_to.all()
                raise('asdf')
                self.status = status
                self.save()

                # for some reason last_assigned_to isn't immediately available after .set(), 
                # so we return the emails here and pass them to _notify_assignee() manually
                return [assignee.email for assignee in assignees] 
            elif approved is False: #Approval was denied
                self.status = FORM_STATUS.DENIED
                self.save()
                return

        if not self.approval_set.exists():
            self.status = FORM_STATUS.DENIED
            self.save()
            raise ImproperlyConfigured('This form had nobody to sign off on it, that means there is no supervisor, division head, department head, HR Contact or Town Manager Contact for this form\'s workflow.')

        # all required steps approved
        self.status = FORM_STATUS.APPROVED

        self.save()

[KenWhitesell]

<conjecture>
I’m guessing that your print statement in the exception handler might be misleading.

Depending upon the larger context in which this is running, I might guess that the set function is being run in a transaction. Then, the query (self.last_assigned_to.all()) isn’t being evaluated at that time. (Querysets are lazy.)

Then, I would guess that the raise is rolling back the transaction, causing the set to be reversed, so that when the queryset is being evaluated, there’s nothing to return.
</conjecture>

If you wanted to confirm this, you could print(x) after the assignment instead of rasing an exception.

If that doesn’t show that it’s working and I had to diagnose this, I’d either be running this in the debugger or sprinkling a lot of print statements through the code to see exactly what data is being seen at each point. I’d also be logging all queries in the database to see what’s being issued.

I find your flow through here to be confusing - your steps list consists of five entries, but you’ve got return statements within your if conditions, which means the loop isn’t going to complete all 5 steps if either of the conditions with return are met. (Whether this is a mistake or not is not something I can determine from the information presented so far.)

You’re also returning the list of email addresses at one point from advance, but you’re not saving that return value where you’re calling it from save_model. Again, I can’t tell if that’s right or wrong, only point out that it seems odd.

[BrockHallenbeck]

Hey Ken,

advance() gets called in my admin’s save_model(), and then again in change_view when I catch a POST. Without going into the weeds of the business logic too much, a user submits a form, which is then passed along to people (assignees) to sign off on. When a user is assigned to a form (that is, request.user == instance.last_assigned_to), I render a button that when clicked opens a modal form for them to fill out (an approval form). I catch that POSTed form in change_view and call advance() again, which assigns it to the next person, until we reach the end of the approval ‘chain’. The return statements are intentional. The business logic dictates that a form should stop at each step and only be kicked forward if approved by the previous person. Maybe it’s clearer with screenshots

Just to be complete, here is my base form model definition which all my ‘form’ models inherit from and the referenced ‘workflow’ model definition:

#Map a form to a workflow
#Specify contacts
class FormWorkflow(MssModel):
    form_class = models.OneToOneField(ContentType, on_delete=models.CASCADE,limit_choices_to=get_form_content_types)
    supervisor_required = models.BooleanField(default=False)
    div_head_required = models.BooleanField(default=False)
    dept_head_required = models.BooleanField(default=False)
    hr_contacts = models.ManyToManyField(User, related_name='forms_as_hr_contact', blank=True, limit_choices_to=get_hr_contacts)
    town_manager_contacts = models.ManyToManyField(User, related_name='forms_as_town_manager_contact', blank=True, limit_choices_to=get_town_manager_contacts )

    def __str__(self):
        return "Workflow configuration for {}".format(self.form_class.model_class()._meta.verbose_name.title())



class FORM_STATUS(models.TextChoices):
    PENDING_SUPERVISOR = 'pending_supervisor', 'Pending Supervisor Approval'
    PENDING_DIV_HEAD = 'pending_div_head', 'Pending Div Head Approval'
    PENDING_DEPT_HEAD = 'pending_dept_head', 'Pending Dept Head Approval'
    PENDING_HR = 'pending_hr', 'Pending HR Approval'
    PENDING_TOWN_MANAGER = 'pending_town_manager', 'Pending Town Manager Approval'
    DENIED = 'denied', 'Denied'
    APPROVED = 'approved', 'Approved'

class ROLE_CHOICE(models.TextChoices):
    SUPERVISOR = 'supervisor', 'Supervisor'
    DIVISION_HEAD = 'division_head', 'Division Head'
    DEPARTMENT_HEAD = 'department_head', 'Department Head'
    HR = 'hr', 'HR'
    TOWN_MANAGERS_OFFICE = 'town_managers_office', "Town Manager's Office"

class ApprovalForm(MssModel):
    last_assigned_to = models.ManyToManyField(User, related_name='forms_assigned', blank=True)
    status = models.CharField(max_length=255, choices=FORM_STATUS, default = FORM_STATUS.PENDING_SUPERVISOR)

    def __str__(self):
        return "{}\'s {} - {}".format(self.created_by.full_name, self._meta.verbose_name.title(), self.created_at.strftime('%m/%d/%Y'))

    @cached_property
    def workflow(self):
        ct = ContentType.objects.get_for_model(self.__class__)
        try:
            return FormWorkflow.objects.get(form_class=ct)
        except FormWorkflow.DoesNotExist:
            raise ImproperlyConfigured(f'{self.__class__.__name__} has no FormWorkflow configured.')
        
    @property
    def supervisor_approved(self):
        if self.workflow.supervisor_required:
            try:
                response = self.approval_set.get(role=ROLE_CHOICE.SUPERVISOR)
            except Approval.DoesNotExist:
                return None
            else:
                return response.approved
        return None


    @property
    def div_head_approved(self):
        if self.workflow.div_head_required:
            try:
                response = self.approval_set.get(role=ROLE_CHOICE.DIVISION_HEAD)
            except Approval.DoesNotExist:
                return None
            else:
                return response.approved
        return None
        
    @property
    def dept_head_approved(self):
        if self.workflow.dept_head_required:
            try:
                response = self.approval_set.get(role=ROLE_CHOICE.DEPARTMENT_HEAD)
            except Approval.DoesNotExist:
                return None
            else:
                return response.approved
        return None
    
    @property
    def hr_approved(self):
        if self.workflow.hr_contacts:
            try:
                response = self.approval_set.get(role=ROLE_CHOICE.HR)
            except Approval.DoesNotExist:
                return None
            else:
                return response.approved
        return None

    @property
    def town_manager_approved(self):
        if self.workflow.town_manager_contacts:
            try:
                response = self.approval_set.get(role=ROLE_CHOICE.TOWN_MANAGERS_OFFICE)
            except Approval.DoesNotExist:
                return None
            else:
                return response.approved
        return None

(MssModel is just an extension of models.Model which adds some housekeeping fields (created by, created at, etc…)

And my change_view() function in my admin:

    def change_view(self, request, object_id, form_url='', extra_context=None):

        obj = self.get_object(request, object_id)

        if request.method == 'POST' and 'approval_action' in request.POST:
            approved = request.POST.get('approval_action') == 'approve'
            comments = request.POST.get('approval_comments', '')

            role = next(
                        (role for assignees, status, role in [
                            ([obj.created_by.supervisor] or [],              models.FORM_STATUS.PENDING_SUPERVISOR,   models.ROLE_CHOICE.SUPERVISOR),
                            ([obj.created_by.division_head] or [],           models.FORM_STATUS.PENDING_DIV_HEAD,     models.ROLE_CHOICE.DIVISION_HEAD),
                            ([obj.created_by.department_head] or [],         models.FORM_STATUS.PENDING_DEPT_HEAD,    models.ROLE_CHOICE.DEPARTMENT_HEAD),
                            (obj.workflow.hr_contacts.all(),           models.FORM_STATUS.PENDING_HR,           models.ROLE_CHOICE.HR),
                            (obj.workflow.town_manager_contacts.all(), models.FORM_STATUS.PENDING_TOWN_MANAGER, models.ROLE_CHOICE.TOWN_MANAGERS_OFFICE),
                        ] if request.user in assignees and status == obj.status),
                        None
                    )

            if role is None:
                raise PermissionDenied
            
            models.Approval.objects.create(created_by=request.user, last_updated_by=request.user, form=obj, role=role, approved=approved, comments=comments)

            try:
                obj.advance()
            except ImproperlyConfigured:
                self.message_user(request, 'Either this form or your user record has not been configured correctly. Submissions will be auto denied until this is resolved. Please contact IT')
                return redirect(reverse('admin:app_list', args=['forms']))
            
            if obj.status in [models.FORM_STATUS.APPROVED, models.FORM_STATUS.DENIED]:
                 self._notify_user(request,obj)
            else:
                self._notify_assignee(request,obj)

            level = messages.SUCCESS if approved else messages.WARNING
            msg = 'Form approved.' if approved else 'Form denied.'
            self.message_user(request, msg, level=level)
            return redirect(reverse('admin:app_list', args=['forms']))
        

        extra_context = extra_context or {}
        
        if obj:
            extra_context['can_approve'] = request.user in obj.last_assigned_to.all() and obj.status not in [models.FORM_STATUS.APPROVED, models.FORM_STATUS.DENIED]
        
        return super().change_view(request, object_id, form_url, extra_context)

The line that returns the list of emails was my workaround (that does work but just left me with a splinter in my mind). So, the fact that I’m not catching the return value in save_model() was just due to me pasting the code in the midst of a confused debug session (I was catching it before I decided to dive back in an see if I can make the original way work). My apologies for the confusion.

By design, users only have add permission for these forms, if forms are submitted in error, it has been decided they are to delete and resubmit. (it was deemed too annoying to deal with the case in which a user gets approved on one step, and then modifies their submission before the next person signs off)

All that to say, the assignment DOES work. You click ‘save’ and the record is created, and the last_assigned_to field IS set. I just can’t reference it to send my emails in save_model(). It is probably also worth noting that this only ‘fails’ on insert, that is when advance() is called from save_model(), the last_assigned_to field is blank at the time it is evaluated. However upon subsequent ‘approvals’, when advance() is called from inside change_view(), ‘last_assigned_to’ field returns correct information and emails are sent out accordingly.

I will start inserting print() statement everywhere to see if that illuminates anything. Logging database queries will be new for me, but I’m always open to learning new stuff!

I appreciate the response. Especially on a Saturday night!

EDIT: I apologize for the poor quality on the screenshot. I originally posted three screenshots, but it yelled at me saying I could only upload one, so I used an online tool to merge them together. Seems to not have worked that great.