Super Scaffold incoming webhooks test

[gazayas]

Closes #453

Joint PR

• Remove bullet train webhooks routing bullet_train#947

I was stuck on this one for a while for a couple of reasons:

1. I originally tried to write system tests but decided it would be more beneficial to write controller tests. Also, getting the webhook parameters right for this one took some fine-tuning.
2. Since we usually use Scaffolding::CompletelyConcrete::TangibleThing and such to transform strings, I initially set out to scaffold using those kinds of strings. It wasn't until I was working in the bullet_train-incoming_webhooks transformer itself that it dawned on me that we were already doing this with Webhooks::Incoming::BulletTrainWebhook 🤦‍♂️

Try it out

bin/super-scaffold incoming-webhooks SomeProvider

This will now scaffold a working controller test file.

[jagthedrummer]

@gazayas, I think we don't want this conditional here, if at all possible. After scaffolding an incoming webhook I'd want the tests for the controller to run, even if I'd set ENV['HIDE_THINGS'] to true. (I don't know much about how incoming webhooks work behind the scenes, so apologies if this isn't really possible. Or easy.)

[gazayas]

@jagthedrummer After looking through the code for a bit and seeing what type of event types we have, I could probably pull this off with a Membership or Invitation. Will try to see what I can do.

[gazayas]

@jagthedrummer I updated the tests with a Membership instead, but I'm not entirely satisfied with this. It's definitely hard to envision how we can accomplish this with a system test, but I think ultimately it makes the most sense to do it that way even if we're not sure HOW we're going to accomplish it.

In the meantime, I'm okay with merging this and then leaving the original issue open. I will need some time to just THINK about how to implement things with an ngrok tunnel though... There has to be a way to Super Scaffold the new webhook and ensure the actual flow of the process works.

I'm also not against requiring developers to setup an ngrok tunnel if we are making tests specifically for webhooks, and we can potentially add a warning message in test/system/super_scaffolding_test.rb letting them know that these tests aren't being run unless the tunnel is up.

[jagthedrummer]

It's definitely hard to envision how we can accomplish this with a system test, but I think ultimately it makes the most sense to do it that way even if we're not sure HOW we're going to accomplish it.

@gazayas can you help me understand why you think a system test is better than a controller test for this? I feel like I might be missing some context or something. I don't see any reason to try to test through a tunnel. That's just testing the tunnel setup, and that's really out of scope for this kind of test, in my opinion. I think the main thing we want to test is just "if a properly formed request lands at this endpoint, do we handle it correctly?" which I think this accomplishes. Am I missing something?

[gazayas]

@jagthedrummer Yeah, there's one main reason here as to why I think testing as a part of the Super Scaffolding system test would be a good idea.

As Andrew stated in #453,

we should test end-to-end that when someone runs bin/super-scaffold incoming-webhooks the resulting controller and model work as expected including being able to receive a webhook.

So the main reason would be to make sure the Super Scaffolded model and routes are scaffolded properly. Just like with the other models in test/bin/setup-super-scaffolding-system-test, adding a model with bin/super-scaffold incoming-webhooks will allow us to ensure we have all the correct files, it can check if the strings we need to change are transformed properly, and also if the routing works. Yes, we technically aren't doing anything on the browser side, but it is in line with how we test other Super Scaffolded models. Maybe a system test isn't something we're looking for, but something that verifies the validity of the model we're scaffolding (a controller test that only runs if we have if defined?(Webhooks::Incoming::SomeProviderWebhook) ... wrapped around it?).

As far as the logic of the webhooks themselves, like you said we're only seeing if the endpoint works, so as I mentioned before I'm okay with just merging this unit test for the time being, but I'm trying to reconcile how we can actually test the scaffolding process and anything else inbetween that we might need as opposed to just the endpoint.

[jagthedrummer]

@gazayas, thanks for the additional context! I think I'm with you now. I'm going to type out my new understanding mainly as a way of organizing my thoughts. I think you're already way ahead of me on this.

So, I think we have two separate but extremely closely related things that we're trying to solve.

1. When we generate code into a project to handle an incoming webhook, we want to also generate tests for that code.
2. We also want to test the process by which we generate the code and the tests mentioned above.

I think this PR completely addresses the first point.

And the second point is what you're getting at with the idea that we need a system test. Something where in the test process we end up running a super scaffolding command for an incoming webhook and then testing that the resulting stuff works as expected, like we do for some of the other super scaffolding stuff.