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

Support code reloading when configuring static preferences sources #4449

Conversation

waiting-for-dev
Copy link
Contributor

This is how we usually recommend configuring sources for static
preferences:

Spree.config do |config|
  config.static_model_preferences.add(
    AmazingStore::AmazingPaymentMethod,
    'amazing_payment_method_credentials',
    credentials: ENV['AMAZING_PAYMENT_METHOD_CREDENTIALS'],
    server: Rails.env.production? ? 'production' : 'test',
    test_mode: !Rails.env.production?
  )
end

However, it's no longer possible to directly reference an autoloadable
class from an initializer.

In this case, we can't change the #add method to take the class name
instead. The reason is that it ends up invoking a sanitization check on
the given preferences against the defined preferences, and that, of
course, needs access to the actual class.

Therefore, it's better to start recommending nesting the configuration
within a .to_prepare block, as they run every time the code is
reloaded:

Rails.application.config.to_prepare do
  Spree::Config.static_model_preferences.add(
    AmazingStore::AmazingPaymentMethod,
    'amazing_payment_method_credentials',
    credentials: ENV['AMAZING_PAYMENT_METHOD_CREDENTIALS'],
    server: Rails.env.production? ? 'production' : 'test',
    test_mode: !Rails.env.production?
  )
end

However, that means running the #add method multiple times, so we need
to change it to replace the definition instead of raising an error.

Closes #4070 & #4040

@aldesantis aldesantis requested a review from a team July 13, 2022 10:02
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Approved assuming specs fails for a different reason. I think this is an important change that we should highlight in the Changelog.

@waiting-for-dev
Copy link
Contributor Author

Approved assuming specs fails for a different reason. I think this is an important change that we should highlight in the Changelog.

Good point! I updated the Changelog accordingly.

This is how we usually recommend configuring sources for static
preferences [1]:

```ruby
Spree.config do |config|
  config.static_model_preferences.add(
    AmazingStore::AmazingPaymentMethod,
    'amazing_payment_method_credentials',
    credentials: ENV['AMAZING_PAYMENT_METHOD_CREDENTIALS'],
    server: Rails.env.production? ? 'production' : 'test',
    test_mode: !Rails.env.production?
  )
end
```

However, it's no longer possible to directly reference an autoloadable
class from an initializer [2].

In this case, we can't change the `#add` method to take the class name
instead. The reason is that it ends up invoking a sanitization check on
the given preferences against the defined preferences, and that, of
course, needs access to the actual class [3].

Therefore, it's better to start recommending nesting the configuration
within a `.to_prepare` block, as they run every time the code is
reloaded [4]:

```ruby
Rails.application.config.to_prepare do
  Spree::Config.static_model_preferences.add(
    AmazingStore::AmazingPaymentMethod,
    'amazing_payment_method_credentials',
    credentials: ENV['AMAZING_PAYMENT_METHOD_CREDENTIALS'],
    server: Rails.env.production? ? 'production' : 'test',
    test_mode: !Rails.env.production?
  )
end
```

However, that means running the `#add` method multiple times, so we need
to change it to replace the definition instead of raising an error.

[1] - https://github.com/solidusio/solidus_stripe/blob/3fab36511a9d02ebd64d03571704ac5a9031b63b/README.md#usage
[2] - https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#autoload-on-boot-and-on-each-reload
[3] - https://github.com/solidusio/solidus/blob/df51df62fa9b829216958b21b20514b7a3d87b30/core/lib/spree/preferences/static_model_preferences.rb#L13
[4] - https://guides.rubyonrails.org/configuring.html#initialization-events

Closes solidusio#4070 & solidusio#4040
We rearrange the CHANGELOG to accommodate the increased amount of
information.
@waiting-for-dev waiting-for-dev force-pushed the waiting-for-dev/accept_static_class_name_as_string branch from 27beeac to c9562c6 Compare July 15, 2022 04:16
@waiting-for-dev waiting-for-dev merged commit adbc70b into solidusio:master Jul 15, 2022
@waiting-for-dev waiting-for-dev deleted the waiting-for-dev/accept_static_class_name_as_string branch July 15, 2022 04:36
DanielePalombo pushed a commit to solidusio/solidus_bolt that referenced this pull request Aug 30, 2022
piyushswain added a commit to piyushswain/solidus_razorpay that referenced this pull request Mar 9, 2023
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.

3 participants