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

postAccounts Breaking Stripe Update #14

Closed
sophiadmcgowan opened this issue Jan 3, 2024 · 13 comments
Closed

postAccounts Breaking Stripe Update #14

sophiadmcgowan opened this issue Jan 3, 2024 · 13 comments

Comments

@sophiadmcgowan
Copy link

sophiadmcgowan commented Jan 3, 2024

Stripe recently pushed a change that allows Account.settings.bacs_debit_payments.display_name to be a nullable(string).

Stripe Changelog

image

This appears to have broken postAccounts in some cases with the following error:

Error in $.settings['bacs_debit_payments']['display_name']: parsing Text failed, expected String, but encountered Null

Is it possible to update the code according to the latest Stripe OpenAPI spec?

@joel-bach
Copy link
Member

Thank you for the report, I'll try to do an update soon 👍

@joel-bach
Copy link
Member

Hey @sophiadmcgowan , I pushed the updated to a branch (see #15), could you try it out and tell me if it fixes your problem?

@sophiadmcgowan
Copy link
Author

Hi @joel-bach thanks for pushing the branch. I did some testing today and it doesn't seem to have fixed our issue. Looking at the PR, I don't see any changes that aren't purely-formatting/whitespace changes. Is that in line with what you're seeing too?

@joel-bach
Copy link
Member

Hi @sophiadmcgowan , you are absolutely right, I pushed the wrong thing, sorry about that 🙈
I've updated the branch and now I see a lot of actual changes. Could you try again? 🙏

@sophiadmcgowan
Copy link
Author

Hi @joel-bach thanks for the update. Our code is failing to compile since we're stuck on an older version of Text.

src/StripeAPI/Operations/PostTreasuryFinancialAccountsFinancialAccount.hs:88:84: error:
    Not in scope: ‘Data.Text.Internal.pack’
    Perhaps you meant one of these:
      ‘Data.Text.Internal.safe’ (imported from Data.Text.Internal),
      data constructor ‘Data.Text.Internal.Text’ (imported from Data.Text.Internal),
      ‘Data.Text.Internal.text’ (imported from Data.Text.Internal)
    Module ‘Data.Text.Internal’ does not export ‘pack’.

It looks like Data.Text.Internal.pack is only exported in newer versions of Text, but all versions support Data.Text.pack. Would it be possible to change usages of Data.Text.Internal.pack to Data.Text.pack? This should make the library strictly more compatible. Thank you!

@joel-bach
Copy link
Member

Good point, I did another update. Since the generation is done through template-haskell it is not easy to control what the output is, so we use import ... as ... instead to import the regular module with the name of the internal module (just in case you are wondering why the code still contains references to Data.Text.Internal). This way the internal module is not actually used. Does this solve your compilation issue?

@joel-bach
Copy link
Member

@sophiadmcgowan did you get a chance to try again with the new version?

@sophiadmcgowan
Copy link
Author

Hi @joel-bach This version looks promising but I'm still working on testing it with our code since there are substantial changes in PostPaymentIntentsIntentConfirm.hs. I will keep you updated!

@sophiadmcgowan
Copy link
Author

sophiadmcgowan commented Jan 18, 2024

Hi @joel-bach is there a particular reason that the StripeAPI.Types.NotificationEventData.Extra module was removed? We currently depend on this module

@joel-bach
Copy link
Member

Yes, the reason being that this was a custom developed module which is not automatically generated and a lot of the underlying structure has changed which broke it. At this point, I do not have the capacity to maintain custom modules if they are going to break with every update, but I'd be happy to include it if you create a PR to add an adjusted version. Since this module was completely built on top of the generated code you can also just copy the previous version and adjust it within your code base to fit your needs as there were no dependencies to this module within this package itself.

@joel-bach
Copy link
Member

@sophiadmcgowan I would curious to hear if you have any updates on this 🙂 Were you able to resolve the issues you were facing?

@sophiadmcgowan
Copy link
Author

Hi @joel-bach, my colleague @tysonzero has taken over this work and can provide an update. Sorry for the delay!

@joel-bach
Copy link
Member

I've merged the update and will close this issue for now. Let me know if it does not work as expected.

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

No branches or pull requests

2 participants