[PATCH v3 2/3] Include the responsible actor in applicable events

[Johan Herland]

On Thu, Nov 14, 2019 at 6:31 AM Andrew Donnellan <ajd at linux.ibm.com> wrote:
> On 17/10/19 9:44 am, Johan Herland wrote:
> > We want to use the events as an audit log. An important part of this is
> > recording _who_ made the changes that the events represent.
> >
> > To accomplish this, we need to know the current user (aka. request.user)
> > at the point where we create the Event instance. Event creation is
> > currently triggered by signals, but neither the signal handlers, nor the
> > model classes themselves have easy access to request.user.
> >
> > For Patch-based events (patch-created, patch-state-changed,
> > patch-delegated, patch-completed), we can do the following hack:
> > The relevant events are created in signal handlers that are all hooked
> > up to either the pre_save or post_save signals sent by Patch.save().
> > But before calling Patch.save(), Patchwork must naturally query
> > Patch.is_editable() to ascertain whether the patch can in fact be
> > changed by the current user. Thus, we only need a way to communicate
> > the current user from Patch.is_editable() to the signal handlers that
> > create the resulting Events. The Patch object itself is available in
> > both places, so we simply add an ._edited_by attribute to the instance
> > (which fortunately is not detected as a persistent db field by Django).
> >
> > The series-completed event is also triggered by Patch.save(), so uses
> > the same trick as above.
> >
> > For the check-created event the current user always happens to be the
> > same as the .user field recorded in the Check object itself.
> >
> > The remaining events (cover-created, series-created) are both triggered
> > by incoming emails, hence have no real actor as such, so we simply leave
> > the actor as None/NULL.
>
> How is cover-created different from patch-created?

In practice, it turns out there is no difference, really:

The patch-created event is triggered by a signal from the Patch model
which potentially carries the ._edited_by attribute. However, AFAICS,
when patches are created, there is no preceding call to
Patch.is_editable(), hence Patch._edited_by is not set, and we end up
passing actor=None to Event.objects.create().

The cover-created event is triggered by a signal from CoverLetter
which has no ._edited_by, hence we pass no actor to
Events.objects.create() and it defaults to None.

I still figured I'd wire up the logic for patch-created, just in case
we at some point were to start setting ._edited_by on Patch objects
when they are first created.

Hope this helps,

...Johan

-- 
Johan Herland, <johan at herland.net>
www.herland.net