search

thoughtbot / fake_stripe

A Stripe fake so that you can avoid hitting Stripe servers in tests.
MIT License
401 stars 78 forks source link

Renders stripe element based on name. #116

Closed mpmenne closed 4 years ago

mpmenne commented 4 years ago

This PR address issue #115. It will return a stripe.js (only for /v3/) that is able to individually create each Stripe element by name.

mpmenne commented 4 years ago

Kind of embarrassed that I submitted this in the first place...

composerinteralia commented 4 years ago

Oh no! Please don't be! I haven't had a chance to review this one yet, but if there is a problem you were trying to solve here that could be solved by the library then please feel free to keep this PR open, or open an issue if you are not happy with this PR. I can look into it when I get back from parental leave.

FWIW, I have opened lots of PRs that I felt a little embarrassed about once I had more information. https://github.com/rails/rails/pull/31208 comes to mind (it's funny to me that I still remember the embarrassment of this one several years later 😆), for example. It's part of learning!

mpmenne commented 4 years ago

Ha! Thanks for the kind words @composerinteralia! So two things bothered me here, let me know if you have any thoughts:

  1. Grasping for a way to test this. The repo has request specs, but would it be too heavy to have a system spec that pulls the JS into a dummy HTML to test the display and interactions. For example, I have another PR in progress right now that would validate the Stripe element fields. It is difficult to test with request specs, feels like it needs something a bit heavier. Would you entertain a system spec in this repo that could test the JS?
  2. I should probably clean up the code a bit :-)