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

Use translated model names in admin payment methods form #1975

Merged
merged 2 commits into from
Jul 19, 2017

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented May 29, 2017

This PR is one of many that follow to remove the confusion around payment methods, payment providers and gateways we have for a long time in our code base.

There is lots of confusing parts in administrating payment methods:

  • The payment method type is called a provider (although it may be only one payment method implementation of many from the same provider)
  • The payment method classes listed as providers are represented by its class names instead of a meaningful human readable name
  • Form params passed to the controller are confused by the STI nature of payment method classes instead of using one concise name
  • The active flag is represented by two radio buttons instead of a checkbox

Before

store credit - payment methods - before

After

store credit - payment methods - after

@tvdeyen
Copy link
Member Author

tvdeyen commented May 29, 2017

Need to rebase #1974 if we decide to make this change

@tvdeyen tvdeyen requested a review from jhawthorn May 29, 2017 12:30
@tvdeyen
Copy link
Member Author

tvdeyen commented May 29, 2017

Need to rebase after #2000 and #2001 were merged if we decide to make this change

@tvdeyen
Copy link
Member Author

tvdeyen commented Jun 22, 2017

Rebased, ready to review

@tvdeyen
Copy link
Member Author

tvdeyen commented Jun 23, 2017

Looks even better with #2040

preference fields - payment method - after

@tvdeyen tvdeyen force-pushed the cleanup-payment-methods-form branch from 302f504 to 9842111 Compare July 2, 2017 15:58
Currently the admin payment methods form uses a mix of payment_method and the payment methods class name params key. This is due to the STI nature of payment methods.

By using the :as feature from Rails' form_for helper we remove code from the controller that merges these to kind of params hashes into one and reduce clutter in the form.
@tvdeyen tvdeyen force-pushed the cleanup-payment-methods-form branch from 9842111 to 9b48a2c Compare July 19, 2017 14:34
@tvdeyen tvdeyen added UI type:enhancement Proposed or newly added feature labels Jul 19, 2017
@tvdeyen
Copy link
Member Author

tvdeyen commented Jul 19, 2017

Rebased again. Still ready for review

@jhawthorn jhawthorn self-assigned this Jul 19, 2017
@jhawthorn jhawthorn changed the title Enhancements for administration of payment methods Use translated model names in admin payment methods form Jul 19, 2017
- Use a checkbox in stead of two radio buttons to represent the active value.
- Move autocapture select above check boxes
- Use available space
@jhawthorn jhawthorn force-pushed the cleanup-payment-methods-form branch from 9b48a2c to a81f974 Compare July 19, 2017 18:24
@jhawthorn
Copy link
Contributor

We're going to backport and include this in 2.3.0. Since 2.3.0 includes the renames of the bogus payment method classes, it makes sense to include this and have them human readable (and also would avoid changing how they're displayed here in two separate consecutive releases).

@jhawthorn jhawthorn merged commit 4607e75 into solidusio:master Jul 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_backend Changes to the solidus_backend gem type:enhancement Proposed or newly added feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants