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

AddPaymentSourcesToWallet "randomly" changing the default #4004

Open
adammathys opened this issue Mar 22, 2021 · 2 comments
Open

AddPaymentSourcesToWallet "randomly" changing the default #4004

adammathys opened this issue Mar 22, 2021 · 2 comments
Labels
type:bug Error, flaw or fault

Comments

@adammathys
Copy link
Contributor

adammathys commented Mar 22, 2021

As of the introduction of #2913, we no longer set the default payment to one that was used on the current order. Instead, we implicitly pick the one with the biggest ID from all sources in a user's wallet. This can result in inconsistent behaviour where placing an order will change the default payment from the one the customer used and/or the one a customer has explicitly set as their default.

Solidus Version:
v2.11+

To Reproduce

  1. Add two payment sources to a users wallet. Make the one with the smallest ID the default payment source.
  2. Complete checkout using the default payment source.

Current behavior
After placing an order with the default payment source, the default will be changed to the wallet source with the largest ID. (Because order.user.wallet_payment_sources.last should implicitly order by ID.) Meaning we changed the user's default payment source without them ever selecting the new default.

Expected behavior
Based on previous behaviour, default payment source should be unchanged after placing the order. (Given the steps used to reproduce.)

@jarednorman
Copy link
Member

Am I misunderstanding that the previous behaviour was for the reusable source on the order with the highest ID would previously have been selected as the default. That's how I'm reading this.

@adammathys
Copy link
Contributor Author

Correct, the previous behaviour would be to pick the source with the largest Spree::Source#id that was part of a valid payment on that order.

The new logic instead picks the wallet source with the largest Spree::WalletPaymentSource#id from all of the sources in the wallet.

RyanofWoods added a commit to RyanofWoods/solidus that referenced this issue Nov 1, 2021
Fixes issue solidusio#4004 [1]

PR solidusio#2913 [2] changed how the new default WalletPaymentSource was set,
from using the last one found/created * within the Order's
PaymentSources to the user's last WalletPaymentSource. A subtle
difference which assumes that the user's last WalletPaymentSource was
just created by add_to_wallet.

However, if the Order's PaymentSource was from the User's wallet default
and add_to_wallet is called, a new WalletPaymentSource would not be
created * and make_default would try to set the new default as the last
WalletPaymentSource. This would change the default if the last
WalletPaymentSource was not the default.

Now, the WalletPaymentSources are scoped to the order's PaymentSources
and the last one is given to Wallet#default_wallet_payment_source=.
Meaning that in this scenario, the default would given and ignored as it
is already the default.

[1] solidusio#4004
[2] solidusio#2913
* Wallet#add creates or finds the WalletPaymentSource if exists.
@kennyadsl kennyadsl added the type:bug Error, flaw or fault label Aug 23, 2022
RyanofWoods added a commit to RyanofWoods/solidus that referenced this issue Oct 31, 2022
PR solidusio#2913 [1] changed how the new default WalletPaymentSource was set.
Before:
- using the last one found/created by the Order's PaymentSources

After:
- using the user's last WalletPaymentSource.

This is problematic in the scenario where the user uses their default
WalletPaymentSource for an order, but their default source is not their
most recent WalletPaymentSource.

Fixes issue solidusio#4004 [2]

Now, #make_default uses WalletPaymentSources found or created by the
order, it means it will keep the default payment source if used. This
is because wallet#add finds or creates.

[1] solidusio#2913
[2] solidusio#4004
RyanofWoods added a commit to RyanofWoods/solidus that referenced this issue Nov 1, 2022
PR solidusio#2913 [1] changed how the new default WalletPaymentSource was set.
Before:
- using the last one found/created by the Order's PaymentSources

After:
- using the user's last WalletPaymentSource.

This is problematic in the scenario where the user uses their default
WalletPaymentSource for an order, but their default source is not their
most recent WalletPaymentSource.

Fixes issue solidusio#4004 [2]

Now, #make_default uses WalletPaymentSources found or created by the
order, it means it will keep the default payment source if used. This
is because wallet#add finds or creates.

[1] solidusio#2913
[2] solidusio#4004
RyanofWoods added a commit to RyanofWoods/solidus that referenced this issue Nov 3, 2022
PR solidusio#2913 [1] changed how the new default WalletPaymentSource was set.
Before:
- using the last one found/created by the Order's PaymentSources

After:
- using the user's last WalletPaymentSource.

This is problematic in the scenario where the user uses their default
WalletPaymentSource for an order, but their default source is not their
most recent WalletPaymentSource.

Fixes issue solidusio#4004 [2]

Now, #make_default uses WalletPaymentSources found or created by the
order, it means it will keep the default payment source if used. This
is because wallet#add finds or creates.

[1] solidusio#2913
[2] solidusio#4004
RyanofWoods added a commit to nebulab/solidus that referenced this issue Dec 21, 2022
PR solidusio#2913 [1] changed how the new default WalletPaymentSource was set.
Before:
- using the last one found/created by the Order's PaymentSources

After:
- using the user's last WalletPaymentSource.

This is problematic when an order uses a default WalletPaymentSource
but it's not the most recent one.

Fixes issue solidusio#4004 [2]

Now, #make_default uses WalletPaymentSources found or created by the
order, which means it will keep the default payment source if used.

[1] solidusio#2913
[2] solidusio#4004
RyanofWoods added a commit to RyanofWoods/solidus that referenced this issue Jan 16, 2023
PR solidusio#2913 [1] changed how the new default WalletPaymentSource was set.
Before:
- using the last one found/created by the Order's PaymentSources

After:
- using the user's last WalletPaymentSource.

This is problematic when an order uses a default WalletPaymentSource
but it's not the most recent one.

Fixes issue solidusio#4004 [2]

Now, #make_default uses WalletPaymentSources found or created by the
order, which means it will keep the default payment source if used.

[1] solidusio#2913
[2] solidusio#4004
RyanofWoods added a commit to RyanofWoods/solidus that referenced this issue Jan 17, 2023
PR solidusio#2913 [1] changed how the new default WalletPaymentSource was set.
Before:
- using the last one found/created by the Order's PaymentSources

After:
- using the user's last WalletPaymentSource.

This is problematic when an order uses a default WalletPaymentSource
but it's not the most recent one.

Fixes issue solidusio#4004 [2]

Now, #make_default uses WalletPaymentSources found or created by the
order, which means it will keep the default payment source if used.

[1] solidusio#2913
[2] solidusio#4004
RyanofWoods added a commit to RyanofWoods/solidus that referenced this issue Jan 17, 2023
PR solidusio#2913 [1] changed how the new default WalletPaymentSource was set.
Before:
- using the last one found/created by the Order's PaymentSources

After:
- using the user's last WalletPaymentSource.

This is problematic when an order uses a default WalletPaymentSource
but it's not the most recent one.

Fixes issue solidusio#4004 [2]

Now, #make_default uses WalletPaymentSources found or created by the
order, which means it will keep the default payment source if used.

[1] solidusio#2913
[2] solidusio#4004
RyanofWoods added a commit to RyanofWoods/solidus that referenced this issue Jan 27, 2023
PR solidusio#2913 [1] changed how the new default WalletPaymentSource was set.
Before:
- using the last one found/created by the Order's PaymentSources

After:
- using the user's last WalletPaymentSource.

This is problematic when an order uses a default WalletPaymentSource
but it's not the most recent one.

Fixes issue solidusio#4004 [2]

Now, #make_default uses WalletPaymentSources found or created by the
order, which means it will keep the default payment source if used.

[1] solidusio#2913
[2] solidusio#4004
RyanofWoods added a commit to RyanofWoods/solidus that referenced this issue Feb 9, 2023
PR solidusio#2913 [1] changed how the new default WalletPaymentSource was set.
Before:
- using the last one found/created by the Order's PaymentSources

After:
- using the user's last WalletPaymentSource.

This is problematic when an order uses a default WalletPaymentSource
but it's not the most recent one.

Fixes issue solidusio#4004 [2]

Now, #make_default uses WalletPaymentSources found or created by the
order, which means it will keep the default payment source if used.

[1] solidusio#2913
[2] solidusio#4004
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Error, flaw or fault
Projects
None yet
Development

No branches or pull requests

3 participants