[PATCH 2/3] tests: Add tests for 'patch-relation-changed' events

[Stephen Finucane]

On Wed, 2020-10-07 at 23:34 +1100, Daniel Axtens wrote:
> Stephen Finucane <stephen at that.guru> writes:
> 
> > This event is rather odd. If you have two patches then the way a
> > relation is created is by creating a 'PatchRelation' instance and then
> > setting the 'related' attribute on the first patch followed by the
> > second patch. Because the event uses the 'Patch' model's 'pre_save'
> > signal, we'll only see events for the patch being currently saved. This
> > means no event will be raised for the first patch and only one event,
> > the one for the second patch, will be raised when the second patch is
> > being added to the relationship.
> > 
> > In hindsight, the structure of the event is off. We should have had
> > something like a 'patch-added-to-relationship' and a
> > 'patch-removed-from-relationship' event, both with the same fields:
> > 'project', 'actor', 'patch' and 'related', the latter of which would
> > have listed all of the _other_ patches in the relationship. Sadly, this
> > is an API change which means we can't do it now. We may well wish to do
> > so in the future though.
> 
> D'oh.
> 
> Given that we're bumping the major version of patchwork, couldn't we
> also bump the API version? I don't think there are many users of the
> relations events so we might as well try to get this cleaned up before
> they get popular.

Thinking about this more, even that wouldn't work. The linked PatchRelation
entry isn't idempotent. If you create a two patch relation, an event will be
created for the second patch and the the 'current_relation' entry will show the
two patches. If you then add a third patch to this relation, another event entry
will be created but the previous one will also have its 'current_relation' value
updated to show three patches. There isn't really any way around this other than
embedding the information in a 'JSONField', which is not supported by MySQL-
based deployments without an extension. I think we should probably null this out
and remove the fields in a future major version bump of the API.

Stephen

> Kind regards,
> Daniel