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

Payment method preferences don't use default values if using static model preferences #3718

Open
seand7565 opened this issue Jul 21, 2020 · 3 comments
Labels
type:bug Error, flaw or fault

Comments

@seand7565
Copy link
Contributor

When setting up a payment methods preferences, you can define a default value if those preferences aren't explicitly set by the user. However, if the user is using static model preferences, these default values are not used.

Solidus Version:
2.10 - though likely an issue with more versions.

To Reproduce

  1. Add a preference with a default value to a payment method
  2. Create a static model preference source for the payment method that does not include the preference created in step 1
  3. Set the payment method to use the static model preference
  4. Try to access the new preference, get nil instead of the default value.

Current behavior
Accessing preferences not explicitly set in static model preferences returns nil instead of the default value

Expected behavior
Accessing preferences not explicitly set in static model preferences should return the default value

Screenshots
(default value with default preference source)
image
(default value with static preference source)
image

@igorbp
Copy link
Contributor

igorbp commented Oct 16, 2020

Hey @seand7565

Do you think there's a case when the user wouldn't want the defaults to be returned in the options hash? If we always return the preference defaults there will be no way to remove them with the static model preferences.

@seand7565
Copy link
Contributor Author

Hmm, that's an interesting question.

Ultimately, as a developer, I would expect something set as a default to always be returned as the default option (if nothing else is set). I think someone could make the case that setting static_model_preferences should override that expectation, but I'm not so sure - IMO it's just another way to set values to the preferences, in the same way that you'd set them on the payment_method.

I'm not sure that there's a use case for removing items from preferences - I think it makes more sense just to statically set them to something different (or just nil) if you don't want to use the default - but if there was a use-case for it, my initial feeling would be that we'd need an alternate way to remove the preferences - maybe from an initializer - rather than just not set the defaults when using static_model_preferences.

I guess another way of thinking about would be this: Currently, when you don't set a preference in static_model_preferences, it's true that it's not included in the preferences hash - but calling something that doesn't exist on a hash just returns nil - which is a value that can just be set in static_model_preferences if you don't want to use the preference for whatever reason. In short, you'd be able to achieve the same effect even if static_model_preferences did return the default value.

@kennyadsl kennyadsl added the type:bug Error, flaw or fault label Sep 6, 2022
@kennyadsl
Copy link
Member

Marking this as a bug, even if I'm not 100% convinced that either way it will be counterintuitive for someone.

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