Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrading package to M4 #11

Open
wants to merge 33 commits into
base: 4.0
Choose a base branch
from
Open

Upgrading package to M4 #11

wants to merge 33 commits into from

Conversation

girardinsamuel
Copy link

No description provided.

@girardinsamuel girardinsamuel self-assigned this Oct 29, 2021
@girardinsamuel girardinsamuel marked this pull request as ready for review October 30, 2021 10:22
@girardinsamuel
Copy link
Author

@josephmancuso
I have completed upgrading this package. You need to add STRIPE_SECRET env variable as github secrets for all tests to pass 😉 .

@josephmancuso josephmancuso changed the base branch from master to 4.0 October 31, 2021 03:43
@josephmancuso josephmancuso changed the base branch from 4.0 to master October 31, 2021 03:43
@josephmancuso
Copy link
Member

Hmm i added github secrets and added env to .github file but still getting the environment errors

@girardinsamuel
Copy link
Author

I partially fixed the tests but now there are some stripe requests errors. There is no such plan masonite-test with the STRIPE key you provided. I will let you check this, but it's on good track 😉

I solved two issues :

  • the config was not correctly loaded. Actually it was, BUT as we put some empty env var for STRIPE_SECRET in .env-example. Before the test it was copied over to .env, loaded into the environment by Masonite and overriding the env var define through Github Action job configuration. So to fix this I removed the var from .env-example. (Don't know if you have a better idea). We could also copy over github secret directly to .env file before tests, so that we keep the env vars in .env-example....
  • the migrations were not ran and we needed a subscriptions table. So now I ran the migrations before running the tests CI. I prefer this approach over committing the db file in the repo. Because locally any times we run tests it can change the db file and we are left with git changes in our repo after running tests...

Hope you're okay with this 😉

@josephmancuso
Copy link
Member

Sounds great I'll check which stripe test I have added for it. Maybe I didn't set up that plan for the test environment lol.

@josephmancuso josephmancuso self-assigned this Nov 1, 2021
@josephmancuso josephmancuso changed the base branch from master to 4.0 November 1, 2021 12:04
@girardinsamuel
Copy link
Author

Did you have time to check this ? I made a test stripe account and I got some tests working when using the correct plans 😉

@josephmancuso
Copy link
Member

@girardinsamuel fixed the issue with plan not existing but tests are still failing

@girardinsamuel
Copy link
Author

@josephmancuso From the tests logs some objects are still missing from the Stripe test account you set up:

  • no plan named : masonite-flash
  • no coupon named : 5-off

@josephmancuso
Copy link
Member

Ok just added those

@josephmancuso
Copy link
Member

Getting this error in the tests. Could be a Masonite ORM issue:

masoniteorm.exceptions.QueryException: Error binding parameter 0 - probably unsupported type.

@josephmancuso
Copy link
Member

@girardinsamuel Added a missing trial to the masonite-test plan and now many more are failing with an unsupported binding type .. Maybe we have to convert to a string? or we have to do something in Masonite ORM

@girardinsamuel
Copy link
Author

Yep looks like a Masonite ORM issue !

On first test test_can_use_coupon_on_charge() here is the query logs:

query = 'INSERT INTO "subscriptions" ("user_id", "plan", "plan_id", "plan_name", "trial_ends_at", "ends_at", "updated_at", "created_at") VALUES (?,?,?,?,?,?,?,?)'
bindings = [1, 'masonite-test', 'sub_1JtFWCLi7zk6is5VyAnhz6p1', 'Masonite Test', DateTime(2021, 12, 7, 17, 51, 2, 808387, tzinfo=Timezone('Etc/UTC')), None, ...]

Error binding parameter 4 - probably unsupported type.

The so called parameter 4 (if indexed from 0) here is a pendulum object. So with SQLite, does this binding is supported ?

The subscription migrations is using .timestamp() to create this field.

@girardinsamuel
Copy link
Author

@josephmancuso Only errors left are "real" errors I guess related to how stripe plans/coupons are configured ?
It seems that self.user.on_trial() is often returning False...

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