launchpad-reviewers team mailing list archive

[Guruprasad <mp+494890@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/soyuz/subscribers/archive.py b/lib/lp/soyuz/subscribers/archive.py
> index 9faf2b6..b7d2e77 100644
> --- a/lib/lp/soyuz/subscribers/archive.py
> +++ b/lib/lp/soyuz/subscribers/archive.py
> @@ -45,10 +45,14 @@ def package_status_change_webhook(upload, event):
>      # there are instances of rejected source package uploads which do not have
>      # any sources
>      if not upload.builds:
> -        if "status" in event.edited_fields and (
> -            upload.status == PackageUploadStatus.ACCEPTED
> -            or upload.status == PackageUploadStatus.REJECTED
> -            or upload.status == PackageUploadStatus.UNAPPROVED
> +        if (
> +            event.edited_fields

> the lazr.restful layer triggers the ObjectModifiedEvent
with a security-proxied PackageUpload object that has no edited_fields in the
modified event

In this context, wouldn't we need to use a check like `hasattr(event, 'edited_fields')` to avoid an error when the attribute does not exist? Or is it handled by some underlying layer?

> +            and "status" in event.edited_fields
> +            and (
> +                upload.status == PackageUploadStatus.ACCEPTED
> +                or upload.status == PackageUploadStatus.REJECTED
> +                or upload.status == PackageUploadStatus.UNAPPROVED
> +            )
>          ):
>              _trigger_source_package_status_change_webhook(
>                  upload,


-- 
https://code.launchpad.net/~vaishnavi-asawale/launchpad/+git/launchpad/+merge/494890
Your team Launchpad code reviewers is requested to review the proposed merge of ~vaishnavi-asawale/launchpad:fix-source-upload-webhooks into launchpad:master.