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

Add Bolt as a Payment Method Option #4425

Conversation

piyushswain
Copy link
Contributor

@piyushswain piyushswain commented Jun 15, 2022

This PR adds bolt as a payment method option during solidus installation.
The install_generator.rb file has been updated to add Bolt as an optional payment method during solidus installation.

The Bolt option in the payment method installs the solidus_bolt gem.

Ref solidus_bolt#109

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

@piyushswain piyushswain marked this pull request as draft June 15, 2022 08:11
Copy link
Contributor

@waiting-for-dev waiting-for-dev left a comment

Choose a reason for hiding this comment

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

Thanks, @piyushswain!

I left a comment. Also, if you rebase from master CI will be green.

@@ -148,6 +137,21 @@ def install_payment_method
end
end

def install_auth_plugin
# Doesn't need to add solidus_auth_devise if solidus_bolt is being installed as it already has the gem
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should rely on a transitive dependency. Although it's unlikely, in the case of solidus_bolt stopping depending on solidus_auth_devise, we want the host application to keep using it.

@piyushswain piyushswain force-pushed the ps/add-bolt-as-a-payment-method-option-during-installation branch from 76f458a to 5fb1b68 Compare July 21, 2022 04:58
@piyushswain
Copy link
Contributor Author

piyushswain commented Jul 25, 2022

Successfully installed on:

Rails Ruby Solidus Branch
6.1.6.1 2.7.4 v3.1
6.1.6.1 2.7.4 v3.0
6.1.6.1 2.7.4 v2.11

Change required to make in work:
In config/application.rb addded the following line

config.active_record.yaml_column_permitted_classes = [Symbol]

Otherwise got the following error on bin/rails generate solidus:install:

Psych::DisallowedClass: Tried to load unspecified class: Symbol
/Users/piyushswain/solidusio/solidus/sample/db/samples/payment_methods.rb:3:in `<main>'
/Users/piyushswain/solidusio/solidus/sample/lib/spree/sample.rb:17:in `load_sample'
/Users/piyushswain/solidusio/solidus/sample/lib/spree_sample.rb:12:in `load_samples'
/Users/piyushswain/solidusio/solidus/sample/lib/tasks/sample.rake:21:in `block (2 levels) in <main>'

@DanielePalombo

@piyushswain
Copy link
Contributor Author

piyushswain commented Jul 25, 2022

Failing on:

Rails Ruby Solidus Branch
7.0.3.1 2.7.4 master

Related Error:
on bin/rails generate solidus:install

uninitialized constant Spree::SocialConfiguration (NameError)

      ::Spree::SocialConfig = ::Spree::SocialConfiguration.new

@DanielePalombo

@piyushswain piyushswain changed the base branch from master to v3.1 July 25, 2022 12:59
This commit adds bolt as a payment method option during solidus installation.
@piyushswain piyushswain force-pushed the ps/add-bolt-as-a-payment-method-option-during-installation branch from 5fb1b68 to 27ec86f Compare July 26, 2022 10:42
@piyushswain piyushswain changed the base branch from v3.1 to master July 26, 2022 10:43
@waiting-for-dev
Copy link
Contributor

Failing on:
Rails Ruby Solidus Branch
7.0.3.1 2.7.4 master

Related Error: on bin/rails generate solidus:install

uninitialized constant Spree::SocialConfiguration (NameError)

      ::Spree::SocialConfig = ::Spree::SocialConfiguration.new

@DanielePalombo

It looks like solidus_social needs to be adapted so that it no references constants in initializers. See the Rails guides for context.

@DanielePalombo
Copy link
Contributor

@piyushswain can you take care of it?

@waiting-for-dev
Copy link
Contributor

As it contains code referencing solidus_frontend, we need to move it to the solidus_frontend installer.

@piyushswain
Copy link
Contributor Author

@waiting-for-dev Could you please explain a little more on why we need to move the bolt installation to the solidus_frontend installer.

From what I understand solidus installation should install any selected payment_method and solidus_frontend should install the new frontend.

I don't understand the reason why we need to move this to solidus_frontend.

@waiting-for-dev
Copy link
Contributor

waiting-for-dev commented Aug 17, 2022

Sure, @piyushswain. Sorry for not being clear enough. Since #4490, the Solidus installer prompts the user to select solidus_frontend or solidus_starter_frontend as storefront. The solidus_bolt extension injects code to solidus_frontend, so the extension won't work when users select solidus_starter_frontend. As the installer delegates to the solidus frontend installer when that's selected, we need to add that logic there.

We're now recommending extensions not to add storefront code. We're still missing a discussion about how we should tackle this kind of built-in integration for the new recommended frontend, though.

Does it make sense?

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