Make Solidus Braintree compatible with Starter Frontend #102
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gsmendoza
force-pushed
the
gsmendoza/91-ssf-dummy-app
branch
8 times, most recently
from
cdc518e to
2e76450
Compare
elia
reviewed
Feb 15, 2023
gsmendoza
commented
Feb 16, 2023
The extension currently modifies the address factory so that it can set a fixed zipcode for the addresses created by OrderWalkthrough. However, once we change the frontend of this gem to SolidusStarterFrontend and run this extension's specs together with the frontend specs, the address modification would cause the frontend's coupon code spec to fail. See https://app.circleci.com/pipelines/github/solidusio/solidus_braintree/239/workflows/06370bde-ef25-49cf-8fc2-1aefd5a2f86e/jobs/623/parallel-runs/0/steps/0-108. To prevent that, we're going to avoid modifying the address by having a local version of the OrderWalkthrough class.
We'll be changing the frontend of the extension to SolidusStarterFrontend in the following commits. In the resulting update, specs for this extension and specs for the frontend will be run together in a single dummy app. Based on my tests, it seems the factory modifications are causing the coupon code spec from the starter frontend spec suite to break. Instead of modifying the address factory, we're taking the route of creating a subclass of the factory that is namespaced to solidus_braintree. This makes it safer to merge the extension's specs with the starter frontend specs. Done: * Created solidus_braintree_address factory. * Created with_first_and_last_name trait. * Created with_fixed_zipcode trait. * Used with_fixed_zipcode trait in OrderWalkthrough class.
* Copy Rakefile and dependencies from SolidusPaypalCommercePlatform * Rename references to solidus_paypal_commerce_platform to solidus_braintree. * Ignore dummy-app directory.
Allows extension assets to be required after StarterFrontend assets.
Consolidate `checkouts` directories to app directory.
gsmendoza
force-pushed
the
gsmendoza/91-ssf-dummy-app
branch
4 times, most recently
from
a4c62e5 to
a3ee7ed
Compare
There was a problem hiding this comment.
Thanks, @gsmendoza. I left two comments about the need to monkey patch what the user owns. Also, did you try if the payment method is working without problems in the backend? See solidusio/solidus_stripe#165 for more context.
app/decorators/controllers/solidus_braintree/carts_controller_decorator.rb
Outdated
Show resolved
Hide resolved
app/decorators/controllers/solidus_braintree/checkout_controller_decorator.rb
Outdated
Show resolved
Hide resolved
gsmendoza
force-pushed
the
gsmendoza/91-ssf-dummy-app
branch
from
a3ee7ed to
803c246
Compare
gsmendoza
added a commit
that referenced
this pull request
Feb 17, 2023
Instead of monkeypatching the controllers, which the user already owns. #102 (comment)
|
@waiting-for-dev I am able to configure the SolidusBraintree payment method in the backend:
|
* Change OrdersControllerDecorator to CartsControllerDecorator. * Introduce "with prepended view fixtures" shared context. * Change PayPal checkout spec fixture to cart footer.
Postponing since we want to minimize the changes in this branch.
* apple_pay_button.js is already required by frontend.js * frontend.js is already required by solidus_braintree.js
Instead of monkeypatching the controllers, which the user already owns. #102 (comment)
gsmendoza
force-pushed
the
gsmendoza/91-ssf-dummy-app
branch
from
7ea093b to
08d2a32
Compare
|
Regarding #102 (comment), I also tried changing my location to a US server via a VPN, but I'm still getting the "Merchant account does not support payment instrument." error. |
This was referenced Feb 21, 2023
Also simplify markup of partial. Payment step UI was reorganized upstream at solidusio/solidus_starter_frontend#321.
gsmendoza
force-pushed
the
gsmendoza/91-ssf-dummy-app
branch
from
c9983bf to
492d47c
Compare
The fieldset is now always visible due to the payment step UI reorganization in solidusio/solidus_starter_frontend#321.
Payment method container was changed in solidusio/solidus_starter_frontend#321.
This is a workaround. Note that it's still possible to trigger the PayPal button iframe by tabbing to it and hitting enter. The hack works for the Venmo and PayPal buttons. The Apple Pay Button is also not yet tested as of this commit. It will be tested in #105.
gsmendoza
force-pushed
the
gsmendoza/91-ssf-dummy-app
branch
from
492d47c to
96a259c
Compare
|
I've updated the PR based on your feedback and recent changes. Please see the following commits for the changes:
@elia, regarding "Do not allow buttons in disabled payment-step to be clickable", it might be possible to disable the PayPal, Venmo, and Apple Pay buttons via JS. However, we'll still need to discuss whether to implement that in StarterFrontend or within each payment extension. For the meantime, the CSS workaround prevents the buttons from being clicked. Here's a quick demo of the payment page: 2023-02-22-payment.webm |
kennyadsl
approved these changes
Feb 22, 2023
|
@elia @waiting-for-dev Merging this now so that we can move forward with the #105 and #106 tickets. If you have additional feedback, I can work on them while working on #104. |
This was referenced Mar 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Closes #91.
Scope
This doesn't include updating the InstallGenerator to copy the frontend code to the user's app. That will be handled in #104 .
Likewise, updating InstallerGenerator to match the one from SolidusPaypalCommercePlatform will be done in #104.
Demo
screencast-bpconcjcammlapcogcnnelfmaeghhagj-2023.02.16-08_54_40.webm
Postponed tasks
[ ] Test Apple Pay checkout - Need iOS device to test.- Can't do for now. See Test SolidusBraintree SSF update with Apple Pay #105.[ ] Test Venmo checkout - Not successful in doing https://github.com/solidusio/solidus_braintree#testing in master (SolidusFrontend). See Make Solidus Braintree compatible with Starter Frontend #102 (comment).- Postponed this to Test SolidusBraintree SSF update with Venmo Pay #106.TODOs
PR updates - first round
Fix CI
Production environment
Development environment
Documentation
solidus_frontend.Testing
PR documentation
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: