Changeset View
Standalone View
bid_api/signals.py
| Show All 13 Lines | |||||
| UserModel = get_user_model() | UserModel = get_user_model() | ||||
| user_email_changed = Signal(providing_args=["user", "old_email"]) | user_email_changed = Signal(providing_args=["user", "old_email"]) | ||||
| USER_SAVE_INTERESTING_FIELDS = { | USER_SAVE_INTERESTING_FIELDS = { | ||||
| "email", | "email", | ||||
| "full_name", | "full_name", | ||||
| "public_roles_as_string", | "public_roles_as_string", | ||||
| "avatar", | "avatar", | ||||
| "date_deletion_requested", | |||||
| # FIXME(anna): add nickname | |||||
| } | } | ||||
| WEBHOOK_TIMEOUT_SECS = 5 | WEBHOOK_TIMEOUT_SECS = 5 | ||||
| def filter_user_save_hook(wrapped): | def filter_user_save_hook(wrapped): | ||||
| """Decorator for the webhook user-save signal handlers.""" | """Decorator for the webhook user-save signal handlers.""" | ||||
| @functools.wraps(wrapped) | @functools.wraps(wrapped) | ||||
| ▲ Show 20 Lines • Show All 106 Lines • ▼ Show 20 Lines | def modified_user_to_webhooks(sender, user: UserModel, **kwargs): | ||||
| # Do our own JSON encoding so that we can compute the HMAC using the hook's secret. | # Do our own JSON encoding so that we can compute the HMAC using the hook's secret. | ||||
| payload = { | payload = { | ||||
| "id": user.id, | "id": user.id, | ||||
| "old_email": old_email, | "old_email": old_email, | ||||
| "full_name": user.get_full_name(), | "full_name": user.get_full_name(), | ||||
| "email": user.email, | "email": user.email, | ||||
| "roles": sorted(user.public_roles()), | "roles": sorted(user.public_roles()), | ||||
| "avatar_changed": old_avatar != cur_avatar, | "avatar_changed": old_avatar != cur_avatar, | ||||
| "date_deletion_requested": user.date_deletion_requested.isoformat() | |||||
| if user.date_deletion_requested else None, | |||||
sybren: What's the reason the `date_deletion_requested` field is included at all when `user. | |||||
raillaAuthorUnsubmitted Not Done Inline ActionsI think I'm used to treating these as APIs, that is having a fixed schema, so adding or removing a field in the payload depending on data alone didn't cross my mind. railla: I think I'm used to treating these as APIs, that is having a fixed schema, so adding or… | |||||
sybrenUnsubmitted Not Done Inline ActionsThe benefit is that the payload doesn't change w.r.t. older versions of Blender ID. The downside of this is that it requires better documentation (which wouldn't be bad in itself at all) to tell receivers of this data that there is an optional field. Such optional fields in JSON come quite natural in Golang, where there is even an option called "omitempty" that causes this exact behaviour. Now that we have dataclasses in Python, I tend to gravitate towards using those to define what's in the JSON document, and then feed dataclasses.asdict(theinstance) to json.dumps(). One of the advantages of this is that you have a clear place where these JSON document structures are defined, rather than passing along dicts. It also helps for type declarations. I think that a dataclass would also just include the None value, though. I'll leave it up to you to make a final decision here, I don't feel that strongly about this. sybren: The benefit is that the payload doesn't change w.r.t. older versions of Blender ID. The… | |||||
raillaAuthorUnsubmitted Done Inline ActionsI'd prefer to keep it as it is now, considering that adding a new field usually doesn't cause problems: JSON validators/loaders, like Python's marshmallow or Golang's json.Unmarshal tend to ignore unknown fields in the payload. And it would indeed save having to explain why this behaviour exists in the first place. railla: I'd prefer to keep it as it is now, considering that adding a new field usually doesn't cause… | |||||
| # FIXME(anna): add nickname | |||||
| } | } | ||||
| json_payload = json.dumps(payload).encode() | json_payload = json.dumps(payload).encode() | ||||
| sess = models.webhook_session() | sess = models.webhook_session() | ||||
| for hook in hooks: | for hook in hooks: | ||||
| log.debug("Sending to %s, %s", hook, hook.url) | log.debug("Sending to %s, %s", hook, hook.url) | ||||
| hook.send(json_payload, sess) | hook.send(json_payload, sess) | ||||
What's the reason the date_deletion_requested field is included at all when user.date_deletion_requested is None? Wouldn't it be nicer to just leave out the field entirely in that case?