Skip to content

Conversation

@danjhd
Copy link
Contributor

@danjhd danjhd commented Sep 19, 2025

Details

A very simple PR to suggest a way to add status and error to the webhooks schema.

Jira Issue (if relevant)

Jira URL: n/a

Related PRs

#142
I am targeting this PR into the branch being used for that PR. Do not merge into any other branch.

Submitter PR Checks

(tick as appropriate)

  • PR completes task/fixes bug
  • API version has been incremented if necessary
  • ADR status has been updated, and ADR implementation has been recorded
  • Documentation updated (README, etc.)
  • PR added to Jira Issue (if relevant)
  • Follow-up stories added to Jira

Reviewer PR Checks

(tick as appropriate)

  • PR completes task/fixes bug
  • Design makes sense, and fits with our current code base
  • Code is easy to follow
  • PR size is sensible
  • Commit history is sensible and tidy

Info on PRs

The checks above are guidelines. They don't all have to be ticked, but they should all have been considered.

@danjhd danjhd requested a review from a team as a code owner September 19, 2025 13:20
"description": "Status of the Webhook. A disabled webhook will not POST events.",
"type": "string",
"default": "enabled",
"enum": [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this have "error" as an option to follow the pattern of Flow Delete Requests? I think the intention of that pattern is that the error object is only present when status is set to error. If we want to follow that pattern, and we want to make this user setable, we might want to think about the best way to indicate the values a client may set and the values a service instance may set. We should also probably describe what a client should do if a webhook enters the error status. e.g. could they set it back to enabled to try again if they fix an issue on the receiving end? Would there be any benefit to having the separate created and started states of the Flow Delete Requests here too? Or is it fair to assume the startup time is negligible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not think about it to this depth, apologies. You have valid points, my response is "yes"! The difference between created and started might be a way of indicating that at least one POST has occurred since creation? Just thinking out loud. I am certainly not too opinionated on the enum values. I just thought creating this PR was better than just leaving you to iron out the details ;)

@j616
Copy link
Contributor

j616 commented Sep 23, 2025

Merging into #142 to continue work on this there

@j616 j616 merged commit 210a94b into bbc:jamessa-webhooks Sep 23, 2025
5 of 6 checks passed
@j616 j616 mentioned this pull request Sep 23, 2025
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants