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

Allow configuring multiple instances with the client #56

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fsoleraracil
Copy link

No description provided.

@cb-sriramthiagarajan
Copy link
Collaborator

Hello @fsoleraracil, did you intend to create this PR with all these changes? 😅

Also, it's interesting to see the title Allow configuring multiple instances with the client! I'd be curious to learn more on the use case.

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@fsoleraracil
Copy link
Author

fsoleraracil commented Jul 25, 2024

Hello @cb-sriramthiagarajan!
Thanks for taking a look and sorry for the unpolished PR. We were in need of this changes and I pushed what we needed to move forward.

Now that I have more time I'd like to explain a bit more.
I'm working in a project that uses different Chargebee instances depending on the country/area. Therefore, we handle the connection to multiple Chargebee environments from our application.

At first, we assumed that using the clauses:

const chargebeeA = new Chargebee()
chargebeeA.configure({ site: 'a', api_key: 'a_key' })

const chargebeeB = new Chargebee()
chargebeeB.configure({ site: 'b', api_key: 'b_key' })

Would lead us to have two clients, each configured for their own sites. However, this wasn't the case. The information provided to the .configure(..) method is being held in a static variable in the Chargebee class. This variable is also using a common object retrieved using the require clause. Therefore, the client configuration is shared across all the Chargebee instances regardless of the new initialiser. That makes chargebeeA and chargebeeB be configured to point to site b.

Aside of being a bit misleading, it won't allow anyone to have connections to different Chargebee instances in the same application, which is quite inconvenient.

Thus, in order to allow this I removed the static code that made the configuration for the site shared across instances.
For this I:

  • removed the static modifier from the _env variable in the Chargebee class.
  • split the Model classes into the actual model and the API (following the SOLID principle of single-responsibility and allowing passing through the instance _env down the stream in the API constructor).
  • created the Api classes with the operations that were previously static methods in the models.
  • pointed the Chargebee client to the API classes instead of the resources.
  • cleaned some unused imports.

With this change I kept the compatibility with the current client API while allowing to connect to other instances if necessary.

I’ll be glad to help and polish the PR as much as needed to be included in the official client since we are going to use this for a long time and it’s quite a stopper for us.

Please reach out to me if you need more information. I’m also available for a call.

Thanks for your time

@cb-sriramthiagarajan
Copy link
Collaborator

cb-sriramthiagarajan commented Aug 26, 2024

Hi @fsoleraracil, my apologies for the delay in responding.

We have released a new beta version that supports creating
multiple instances of Chargebee (+ has other features like support for Edge runtimes etc.). It'd be great if you could give it a try and share your feedback.

You can install this using npm install chargebee@beta

Edit: We have a Migration guide for v3. README in the next branch also has more info on the usage.

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