From cfe0f6b56d8c353ee82305e020aafacf343dd9ba Mon Sep 17 00:00:00 2001 From: Eeshan Garg Date: Wed, 20 Feb 2019 19:59:36 -0330 Subject: [PATCH] webhooks/clubhouse: Account for bug in GitHub branch payloads. This change was prompted by a possible bug on Clubhouse's end. In general, if a branch is added, it also prompts a workflow state transition in its primary story. However, our webhook error logs show an erroneous payload where the same branch is added to another story, but there is no workflow state transition. Also, both the stories are grouped in the same payload, which confused/broke our code. Ideally, this shouldn't happen and is most likely a bug on Clubhouse's end. In most cases, changes included in Clubhouse payloads never pertain to more than one parent entity (stories) simultaneously, and we usually operate under the assumption that the changes included therein are related to each other in terms of their parent object (story or epic) and not a child object (the GitHub branch). --- zerver/webhooks/clubhouse/view.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/zerver/webhooks/clubhouse/view.py b/zerver/webhooks/clubhouse/view.py index b30db3d639..bcc7fb0c8d 100644 --- a/zerver/webhooks/clubhouse/view.py +++ b/zerver/webhooks/clubhouse/view.py @@ -325,18 +325,16 @@ def get_reference_by_id(payload: Dict[str, Any], ref_id: int) -> Dict[str, Any]: return ref -def get_story(payload: Dict[str, Any]) -> Dict[str, Any]: - story = {} # type: Dict[str, Any] - for a in payload['actions']: - if a['entity_type'] == 'story': - story = a - - return story - def get_story_create_github_entity_body(payload: Dict[str, Any], entity: str) -> str: action = get_action_with_primary_id(payload) - story = get_story(payload) + + story = {} # type: Dict[str, Any] + for a in payload['actions']: + if (a['entity_type'] == 'story' and + a['changes'].get('workflow_state_id') is not None): + story = a + new_state_id = story['changes']['workflow_state_id']['new'] old_state_id = story['changes']['workflow_state_id']['old'] new_state = get_reference_by_id(payload, new_state_id)['name']