search

Denhac / denhac-webhooks

This repo holds some of our membership automation stuff. It listens to webhooks from the main wordpress site
7 stars 3 forks source link

[Actions] Create AuthorizeEquipment action #45

Closed chungl closed 7 months ago

chungl commented 8 months ago

We should create an action to encapsulate the work of associating UserPlans to Users.

Since actions are replayable, we will be more resilient to failures that can arise from needing to post one request per user per plan.

This allows us to decouple the intent of a modal submission from the execution of potentially queue-able work. We can respond quickly to avoid timing out on the user's request, and we can retry as necessary if any failures occur while processing the work.

I know I wrote this part of the code, but one issue that's come up is that if this line fails, the Slack UI fails and no other authorizations happen. Because of this, I've been migrating things that use an API but that aren't mission critical to be done right that second in the dialog over to an Action that can be queued and retried.

php artisan make:action <name> to make one, and I'd put it in a new folder called "WordPress". I'm also starting to move things with "WooCommerce" in the name over to WordPress but that's a separate thing.

_Originally posted by @Jnesselr in https://github.com/Denhac/denhac-webhooks/pull/38#discussion_r1446021989_

chungl commented 8 months ago

@Jnesselr Do you have an intuition for whether the action should represent a batch of authorizations or a single one? From an API user's perspective I'd like there to be a single endpoint that captures all relevant information (who is authorizing, who they are authorizing, and on what equipment) as a single submission. From an execution perspective, it would make more to retry individual requests that failed rather than resubmit everything if a single item fails. Perhaps there should be a hybrid approach where one action dispatches many more?

chungl commented 8 months ago

Related, but separate question: Where would it make sense to do an authorization check that the acting member has permission to issue authorizations on the listed equipment? This seems like a thing that should ideally fail synchronously with action creation so that we can provide immediate feedback, but it's not clear that the actions have an inbuilt pre-queue hook. Perhaps this is a controller problem.

chungl commented 8 months ago

I'm going to tinker on this project and see if I can figure it out, but these are high-level architecture questions that would be great to get some guidance on from someone more familiar with this platform and its design.

Jnesselr commented 8 months ago

Do you have an intuition for whether the action should represent a batch of authorizations or a single one?

Yes, an action should almost never represent a batch by convention. It's almost always one to one, ie in this case "one action means one user is being trained on one piece of equipment".

Where would it make sense to do an authorization check that the acting member has permission to issue authorizations on the listed equipment?

That already happens. A user should NEVER see a piece of equipment that they are not a trainer for. If they do see a piece of equipment they're not a trainer for, we messed up somewhere else.