Webhook triggered on Order Cycle close

[lin-d-hop]

Description

As an API user I want to be notified when an OC closes.
As an API user I want to be able to turn notifications on and off

Acceptance Criteria & Tests

1. We need to be able to turn webhook posting on and off via the UI. API settings are at the User level in Developer Settings.
   The UI will exist in https://openfoodnetwork.org.uk/account#/developer_settings
   This UI is visible when super-admin enables 'API Key View' for the user.

2. This setting will be called 'Post webhook on Order Cycle Closed'. In line with other settings two radio buttons will denote enabled/disabled:
   

3. When 'enabled' a url is shown where the payload will be sent to.

4. The webhook payload will be triggered when an OC opens for any enterprises this user manages and owns. It is applicable for all distributor and coordinator enterprises (and not for one's for which the user supplies only).

5. A tooltip should be included saying 'This will trigger a webhook to post the following payload to the given URL when an order cycle closes:
   "id": STRING,
   "at": DATETIME,
   "type": "order_cycle_close",
   "data": {
   "id": INTEGER,
   "name": STRING,
   "open_at": DATETIME,
   "close_at": DATETIME,
   "enterprise_id": INTEGER,
   "enterprise_name": STRING
   "tags": [STRING]
   }"

6. The payload delivered should be:
   "id": STRING (ID of the webhook)
   "at": DATETIME (when payload sent)
   "type": "order_cycle_close",
   "data": {
   "id": INTEGER (ID of the OC)
   "name": STRING (name of the OC)
   "open_at": DATETIME (OC opening time)
   "close_at": DATETIME (OC closing time)
   "enterprise_id": INTEGER (enterprise ID in DB)
   "enterprise_name": STRING (enterprise name)
   "tags": [STRING] (tags on the OC)
   }

For example:
{
"id": "EV09374580",
"at": "2022-06-15T04:18:00-04:00",
"type": "order_cycle_close",
"data": {
"id": 79234596,
"name": "Aug 3rd"
"open_at": "2022-06-15T04:18:00-04:00",
"close_at": "2022-07-15T04:18:00-04:00",
"enterprise_id": 2093846
"enterprise_name": "Fictional Hub"
}
}

6. Unit Test coverage as standard
7. Translations as standard on the Enterprise Settings page.

Other useful Bits

This tutorial looks good:
https://keygen.sh/blog/how-to-build-a-webhook-system-in-rails-using-sidekiq/

[lin-d-hop]

This is basically a copy paste of OC Open, but on reading through and adjusting I think it is all relevant. Let me know if anything doesn't make sense or if you have any questions.

[RachL]

@openfoodfoundation/developers @openfoodfoundation/testers is this one a papercut size or bigger?

[dacook]

I think this could be bigger, because #9616 only includes provision for one webhook, so it won't be as simple as creating one more event type. I'll estimate:

1. Add event type field to webhook endpoints - 1hr
2. Add event type to user account form - up to 1day
   • TBC: can a user add more than one of each type?
3. [tech debt] Rename existing OrderCycleClosingJob to OrderCycleClosedJob - 1hr
   • it requires a safe transition for existing enqueud jobs
     (we chose the past tense for OrderCycleOpenedJob because it doesn't perform the opening, it happens after it has opened. We'd like to bring this existing job name in line with that. refer )
4. [fix] Change at to ISO date format (this was missed in the initial implementation) - 10min
5. Trigger webhook when order cycle closes (from OrderCycleClosedJob) - 1hr

[dacook]

Question: can a user add more than one of each type of webhook? (eg add Order Cycle Closed twice, once to Zapier and once to N8N).

I think yes, it should be allowed, to make life easier. Sure, you could probably re-post from one service to the next, but this might cost more or be a challenge for a non-technical user. There's a chance that someone could abuse it and add heaps of endpoints, but I'm sure there's plenty of worse ways to abuse the system (run a report for 12months of data.. )

A preview of #9616 for reference:

[mkllnk]

Rename existing OrderCycleClosingJob to OrderCycleClosedJob

You need compatibility for enqueued jobs to be found, right? You can always keep the old class for compatibility and use the new code:

# Temporary class for compatibility.
# Can be removed ten minutes after this change has been deployed.
class OrderCycleClosingJob < OrderCycleClosedJob; end

[mkllnk]

Add event type to user account form - up to 1day

Yes, the UI changes are the biggest part. But can we do it incrementally? Maybe we just duplicate what we have and have one field for opened and one field for closed. Supporting multiple webhooks would be good but can be another issue.

[dacook]

You can always keep the old class for compatibility and use the new code:

Thanks, I didn't think through if there might be other things to consider. I have updated the estimate.

Maybe we just duplicate what we have and have one field for opened and one field for closed. Supporting multiple webhooks would be good but can be another issue.

Maybe, but I suspect it will be just as easy to support multiple, as the data model already accepts multiple, and the form is already tabular. Maybe suggesting 1 day was over the top. Do you think it's fair to call this a papercut?

[audez]

Hi! Thanks for this <3
Will we be able to activate the feature as a superadmin?

Indeed, in FR we will mainly use this webhook to trigger sending a newsletter to a user's client at the end of each OC, so we need to enable the webhook for this user.

[dacook]

You're right, it will need to be enabled for the user (that is, the owner of the coordinator/distributor enterprise).
As it is currently, you need to log in as that user to access the Account > Developer Settings section. So no, you won't be able to activate it as a superadmin via Admin > Users > Edit.
Hopefully you could give them the instructions on how to enable it themselves.

[audez]

ok thanks! it will be another step to ask them (we already ask to create an account in a newsletter tool and send us the API key) but I guess they can live with it :)

[filipefurtad0]

https://github.com/orgs/openfoodfoundation/teams/developers https://github.com/orgs/openfoodfoundation/teams/testers is this one a papercut size or bigger?

Sorry for the delay in my reply. I was not able to estimate this one before in terms of testing, but now that we have the first bit in, I think there's a better understanding. Proposing:

Manual testing: 2 hrs
Writing VCR tests: 6 hrs (optional) I don't think we need this, as we only need to make sure that we send the correct payload; this is sent to another app and should not bounce back to OFN (which is the case for Stripe, for example.)

[dacook]

I think @mkllnk and I agreed on papercut (from memory this means under 1 day development time). I'm assuming 2hrs of testing doesn't change that.

I just realised that this doesn't include adding the 'test' function, which allows you to manually trigger the webhook and see the result in realtime. I think that would push this over a papercut size, and can probably wait to be a separate task.

[mkllnk]

papercut (from memory this means under 1 day development time)

I think that's our Fair Food definition. OFN uses a half day for a Ri dev (experienced). That can be a day for Shu and Ha devs.

[mkllnk]

Here is the last definition: https://community.openfoodnetwork.org/t/papercuts-revised-15-oct-2020/1765

I think that we are a bit relaxed with papercuts. Most papercuts take more than 5 minutes to discuss. 😉