🧠 🌴 Cleanup a bit braintree customer form data #3167
Conversation
josemigallas
changed the title
Co-authored-by: Thales Miguel <thales.miguel@gmail.com>
…t so much more convenient Co-authored-by: Thales Miguel <thales.miguel@gmail.com>
| named_route = [:admin, :account, merchant_account.payment_gateway_type] | ||
| polymorphic_path(named_route, url_params) | ||
| payment_gateway_type = merchant_account.payment_gateway_type | ||
| return '' if [:bogus, nil].include?(payment_gateway_type) |
There was a problem hiding this comment.
is there a test for this. It just looks a little risky what type is payment_gateway_type
There was a problem hiding this comment.
This was added 11 years ago.. you seriously asking? LOL The whole thing is a bit of a mess so I wouldn't expect none of this is properly tested.
This were the "tests" that were implemented. At least there were a TODO 😂
There was a problem hiding this comment.
precisely will be very useful to have an example when this bogus value may appear. I remember seeing that before and it is unnerving.
There was a problem hiding this comment.
Right. I think it's only enabled in development env, that's probably why the didn't bother testing it. Anyway, I don't think it is this PR's responsibility but I will take a good look into this and add some more tests.
| for (const key in billingAddress) { | ||
| // @ts-expect-error FIXME | ||
| if (billingAddress[key] === null) { | ||
| // @ts-expect-error FIXME | ||
| billingAddress[key] = '' | ||
| } |
There was a problem hiding this comment.
This has been removed in favor of server side transform_values { |value| value.presence || '' }.
| end | ||
|
|
||
| def edit_payment_details(merchant_account = site_account) | ||
| return "" if ["bogus" ,""].include?(merchant_account.payment_gateway_type.to_s) | ||
| polymorphic_url([:edit, :admin, :account, merchant_account.payment_gateway_type]) | ||
| payment_gateway_type = merchant_account.payment_gateway_type |
There was a problem hiding this comment.
Some further opportunities for refactoring:
- rename
edit_payment_detailstoedit_payment_details_path
And by the way, searching for this name (to check if there is any name clash) I stumbled on this helper: https://github.com/3scale/porta/blob/master/lib/developer_portal/lib/developer_portal/controller_methods/payment_paths_methods.rb#L11-L23
Which also implements another thing I was going to suggest: extracting [:bogus, nil].include?(payment_gateway_type) to a separate method, to avoid duplication of logic.
There was a problem hiding this comment.
I don't think #edit_payment_details is used anymore, I will remove it altogether. No need to refactor this.
Note: the method #unacceptable_payment_gateway? should be moved to Account::Gateway but I'm gonna refactor that in a following PR.
josemigallas
dismissed stale reviews from akostadinov and thalesmiguel
via
76760c2
Upgrade Postgresql to version 13 in Dockerfile allow easier caching of oracle gems Now if one wants to cache only oracle gems when building images so that gems are available in image but not installed so only free bits are available in the image, then can do ``` BUNDLE_WITHOUT: "assets:test:development:default:preview:production" bundle package ``` THREESCALE-8990: Add CMS API to the API Docs (#3136) THREESCALE-8990: CMS access token scope: make public (#3135) * Clean up code from the CMS API rolling update * Access Tokens: remove support for private APIs * Access Token scopes: cleanup for private apis Update thinking_sphinx.yml configuration file: - add batch_size configuration (see below) - remove the environments development, test, preview that are not needed in the container image - group the different settings according to what they configure (thinking sphinx, searchd, etc.) batch_size setting is required for reindexing in SaaS, as with the default value 1000 reindexing with `rake ts:rt:index` fails with the following error: ``` ThinkingSphinx::QueryLengthError: The supplied SphinxQL statement is 10266420 characters long. The maximum allowed length is 8388603. ``` Probably that's because of indexing CMS contents that can be quite long. Fix link on Top Applications table (#3161) prevents initial rerender by using ref instead of state 🧠 🌴 Cleanup a bit braintree customer form data (#3167) * 🧠 🌴 extracts braintree customer form data from template * ⚰️ removes unused method from PaymentDetailsHelper Co-authored-by: Thales Miguel <thales.miguel@gmail.com> ♻️ dry: moves some logic to Account::Gateway module fix pgsql and oracle tests relying on record order is flaky use common helper 🐛 removes duplicated flash message CircleCI: Remove 3scale-ninjabot key (#3159) ♻️ extracts braintree form validation CMS: Enforce params security for templates (#3152) ⚰️ removes unused methods from payment details helper
No description provided.