launchpad-reviewers team mailing list archive

[Ines Almeida <mp+445031@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/bugs/subscribers/bugactivity.py b/lib/lp/bugs/subscribers/bugactivity.py
> index c036186..d1d1bc8 100644
> --- a/lib/lp/bugs/subscribers/bugactivity.py
> +++ b/lib/lp/bugs/subscribers/bugactivity.py
> @@ -66,6 +66,10 @@ def what_changed(object_modified_event):
>      after = object_modified_event.object
>      fields = object_modified_event.edited_fields
>      changes = {}
> +
> +    if not fields:

Looking at the test failures in http://lpbuildbot.canonical.com/builders/lp-devel-xenial/builds/3822/steps/shell/logs/summary, they seem to happen in a few situations (when someone subscribes to a bug, when a bug changes visibility, ...). I tested it locally and these do happen when you subscribe someone.

Overall, the `edited_fields` are added "manually" when creating a `ObjectModifiedEvent` (they are not computed automatically), so my assumption is that in some cases (like the subscription of someone) we want to trigger a modified event, but no fields did really change within the bug itself.

----
The difference added with the webhooks code change, is just that we call 'what_changed' and iterate over it when an object emits an object changed event. For context, the function `what_changed` hasn't changed with the webhooks, it simply wasn't called before in these situations, hence why it didn't failed before.

Thinking about it more after your comment, I'm not sure if this is an intended result (having an ObjectModifiedEvent without changed fields), so I'm look into it. What do you think?

> +        return changes
> +
>      for fieldname in fields:
>          # XXX 2011-01-21 gmb bug=705955:
>          #     Sometimes, something (webservice, I'm looking at you


-- 
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/445031
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad:add-bug-webhooks/fix-failing-tests into launchpad:master.