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

Implement object_name_to_module so it will work for new objects in future #792

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

sb8244
Copy link
Contributor

@sb8244 sb8244 commented Apr 9, 2023

This fixes #790 #817 #791 in a way that will also work for the future.

fixes #790 #817 #791

@@ -34,6 +34,24 @@ defmodule Stripe.UtilTest do
assert object_name_to_module("transfer") == Stripe.Transfer
assert object_name_to_module("transfer_reversal") == Stripe.TransferReversal
assert object_name_to_module("token") == Stripe.Token

assert object_name_to_module("billing_portal.session") == Stripe.BillingPortal.Session
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These become a bit extra, but I figure that I'd add manual tests for any of the handwritten functions I removed

@doomspork
Copy link
Member

@sb8244 seems as a result of merging #790 we've caused a merge conflict. Would you mind taking a look at that?

@sb8244 sb8244 requested a review from a team as a code owner April 9, 2023 14:29
@sb8244
Copy link
Contributor Author

sb8244 commented Apr 9, 2023

@doomspork merged back with main 👍

@doomspork
Copy link
Member

Thank you @sb8244, appreciate the multiple PRs this weekend! 🙏 Random but noticed you're a fellow Atlantian (is that a thing?) 🎉

@snewcomer this change looks good to me but I'd like to defer to you for final review/approval.

@sb8244
Copy link
Contributor Author

sb8244 commented Apr 9, 2023

Wow unexpected! Not too many of us in the Elixir community.

@github-actions
Copy link

github-actions bot commented Oct 4, 2023

This pull request has been automatically marked as "stale:discard". If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated!.

@sb8244
Copy link
Contributor Author

sb8244 commented Oct 4, 2023

bump

@yordis
Copy link
Member

yordis commented Oct 9, 2023

@sb8244 I will be helping you move forward. Would you mind rebasing the PR? My apologies for the inconvenience.

@arosenb2
Copy link

arosenb2 commented Nov 28, 2023

Any chance this could get merged conflicts resolved and then merged + have a new release cut? I'm looking to use Financial Connections with webhooks and this PR seems like it would fix that.

@yordis yordis merged commit 212bef2 into beam-community:main Nov 28, 2023
16 checks passed
@sb8244 sb8244 deleted the object-name branch November 28, 2023 16:30
@sb8244
Copy link
Contributor Author

sb8244 commented Nov 28, 2023

Thanks for handling @yordis


def object_name_to_module("usage_record_summary"),
do: Stripe.UsageRecordSummary
def object_name_to_module("test_helpers.test_clock"), do: Stripe.TestClock
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! I believe there is a mistake made here and this should be Stripe.TestHelpers.TestClock. It is currently breaking api requests to retrieve test clocks. I've filed a bug report #825

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.

Stripe.BillingPortal.Configuration fails to load
5 participants