Shopify / shopify-api-ruby

ShopifyAPI is a lightweight gem for accessing the Shopify admin REST and GraphQL web services.
MIT License
1.06k stars 473 forks source link

Deprecate ShopifyAPI::Context in favour of instance-based configuration #1188

Closed sterrym closed 11 months ago

sterrym commented 1 year ago

Description

This is an experiment to allow all methods that use ShopifyAPI::Context to use an instance-based configuration instead. There is an attempt to maintain backwards-compatibility as all method parameters still default to ShopifyAPI::Context

At time of writing, these are the impacted methods:

Please, include a summary of what the PR is for:

There is a desire/need to create more than one Shopify App within a single Rails application.

We are attempting to have zero-impact on current users other than potentially a deprecation notification.

How has this been tested?

All existing tests continue to function. We added configuration parameter options to all tests as well. We also top-hatted these changes by using this branch to create and install a new Shopify App

Checklist:

lizkenyon commented 11 months ago

Hi there 👋

Could you give us a bit more context on the use case for this 👇

desire/need to create more than one Shopify App within a single Rails application.

sterrym commented 11 months ago

Hi there 👋

Could you give us a bit more context on the use case for this 👇

desire/need to create more than one Shopify App within a single Rails application.

Absolutely. This was initially an internal need that we figured other partners would be interested in as well. Specifically, we are building multiple payments apps within an existing Shopify rails app. In our specific case, we are actually building a single Rails engine that supports three different types of payments apps. Each of them needs to be configured in a different way.

Internally, we ended up going another direction and are just using internal Shopify tools but I still suspect there's a valid use-case here.

lizkenyon commented 11 months ago

Thanks @sterrym for the added context!

I am going to close this for now. But we may reach out for additional context if we want to move forward at a later date!