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

[Andrew Donnellan]

On 15/11/19 9:37 am, Johan Herland wrote:>>> 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.

OK. Personally I'd rather we not wire it up, as it's confusing to see 
something like that wired up and wonder why the field isn't being set.

It's also not clear how you should set the actor on email-triggered 
events as not every sender has a Patchwork account.


Andrew

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd at linux.ibm.com             IBM Australia Limited