Skip to content
This repository has been archived by the owner on Apr 14, 2023. It is now read-only.

Rubocop -a on spec files #317

Merged
merged 1 commit into from
Mar 4, 2022
Merged

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Mar 4, 2022

Currently all master branch PRs fail because of this

@tvdeyen tvdeyen merged commit c8000c5 into solidusio:master Mar 4, 2022
@tvdeyen tvdeyen deleted the fix-rubocop-complaints branch March 4, 2022 16:58
gsmendoza added a commit that referenced this pull request Jul 19, 2022
…pecs

Summary
-------

When `spec/features/frontend/venmo_checkout_spec.rb` is enabled and ran against
a working Braintree sandbox, it will fail with the following error:

```
  1) Checkout with Venmo checkout with Venmo transactions meet's Braintree's acceptance criteria during checkout
     Failure/Error: expect(page).to have_content('Venmo Account: venmojoe')
       expected to find text "Venmo Account: venmojoe" in "LOGIN\nAll departments\nHOME\nCART: (EMPTY)\nYour order has been processed successfully\nLOGIN AS EXISTING CUSTOMER\nEmail\nPassword\nRemember me\nor Create a new account | Forgot Password?\nPowered by Solidus"
     # ./spec/features/frontend/venmo_checkout_spec.rb:111:in `block (4 levels) in <top (required)>'

Finished in 16.59 seconds (files took 3.87 seconds to load)
1 example, 1 failure
```

Expected spec behavior
-----------------------

Given I am logged in
And I am checking out an order
And I am paying for the order with Venmo
When I finalize the checkout
Then I should be redirected to the order page

Actual spec behavior before "Deprecate try_spree_current_user" commit in Solidus
--------------------------------------------------------------------------------

I am redirected to the order page as a guest.

This was tested in a test app with
the following setup:

* Rails 6.1.6.1
* Solidus 3.1.7
* SolidusAuthDevise - 2.5.4
* SolidusPaypalBraintree c8000c5 - Merge pull
  request #317 from tvdeyen/fix-rubocop-complaints
* Personal Braintree sandbox account
* `checkout/valid_venmo_transaction` cassettes updated to match personal
  Braintree sandbox account.

Actual spec behavior after "Deprecate try_spree_current_user" commit in Solidus
-------------------------------------------------------------------------------

I am not allowed to go to the order page and am redirected to the login page.

This was tested in a test app with
the following setup:

* Rails 7.0.3.1
* Solidus 3.2.0 alpha (8e5adb49eee121e3dad648bed9d6e834abcf450e)
* SolidusPaypalBraintree 4890572 - Fix specs to
  stub spree_current_user)
* SolidusAuthDevise 2.5.4
* "Temporarily disable failing Venmo specs" reverted
* Personal Braintree sandbox account
* `checkout/valid_venmo_transaction` cassettes updated to match personal
  Braintree sandbox account.

Cause
-----

`Spree::CheckoutController` uses
`Spree::CheckoutControllerDecorator#completion_route` in SolidusAuthDevise to
determine where the user is redirected after finalizing the order:

```rb
def completion_route
  return spree.order_path(@order) if spree_current_user

  spree.token_order_path(@order, @order.guest_token)
end
```

In the post-deprecation scenario, the Venmo specs stub
`Spree::CheckoutsController#spree_current_user`. As a result, when finalizing
the order, the user is redirected to the order path.

However, the Venmo specs do not stub
`Spree::OrdersController#spree_current_user`. Because of this, when the user
reaches the order page, he is identified as a guest and is forced to log in.

Why did it used to work before?
-------------------------------

In the pre-deprecation scenario, only
`Spree::CheckoutsController#try_spree_current_user` is stubbed and not
`Spree::CheckoutsController#spree_current_user`. As a result,
`completion_route` identifies the checkout user as a guest, and the user is
redirected to the token order path.

`Spree::OrdersController#show` then accepts the user as a guest because the
order token is included in the URL.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant