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.

Sign up for GitHub

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

Change RubyMoney bank to currencylayer #10520

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Change RubyMoney bank to currencylayer #10520

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a17813f
Change RubyMoney bank to currencylayer
danieljames-dj Jan 3, 2025
79ce66d
Review changes
danieljames-dj Jan 7, 2025
955dd78
Adding dummy API key in env
danieljames-dj Jan 14, 2025
b9ca875
Adding currencylayer only for production
danieljames-dj Jan 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .env.development
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@ AVATARS_PUBLIC_STORAGE=local
AVATARS_PRIVATE_STORAGE=local_private
DUMP_HOST=https://assets.worldcubeassociation.org
CRONJOB_POLLING_SECONDS=0
CURRENCY_LAYER_API_KEY=9d254ec37e1591eec964dac947fafa09
1 change: 1 addition & 0 deletions .env.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,4 @@ STAGING_OAUTH_SECRET=example-secret
AVATARS_PUBLIC_STORAGE=local
AVATARS_PRIVATE_STORAGE=local_private
DUMP_HOST=https://assets.worldcubeassociation.org
CURRENCY_LAYER_API_KEY=9d254ec37e1591eec964dac947fafa09
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these keys (here in test and same in dev) random dummy values or are they actual access keys?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are actual access keys from my test account. I understand that exposing actual access keys is risky, but I went ahead since my test account is a free account and I don't mind even if the account gets deactivated.

The reason why I added actual access key is that if I give a dummy access key, it will throw error in CI/CD.

I tried if there is any way to fix this with a dummy access key, but couldn't find any way. Can you please let me know whether you faced this problem while initializing env variables for paypal or stripe or anything similar?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually use access keys from testing or dummy accounts for Stripe and PayPal. Look into .env.test, you will see that a lot of PAYPAL_* variables exist. These are from "real" accounts, which means we have actual username/password to log in to PayPal dashboard. But if you do actually log in, you will see that these accounts are marked as test accounts by PayPal, and you cannot send actual, real money with them. Only sandbox money. So that's why the accounts are not real, they are only "real".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I think currencylayer doesn't have a test account as it doesn't do any sensitive things. So will it make sense to create two currencylayer accounts for WCA - one for test and one for production?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, now that I think about it, I have another idea...
What would you think about using the if Rails.env.test? check to use the EuCentralBank in local environments and switch on the CurrencyLayer in production only?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can introduce a flag in EnvConfig (file env_config.rb) where developers can switch on CurrencyLayer manually if they want, for local testing and stuff

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a great idea, I've now implemented it. :-)

1 change: 1 addition & 0 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ gem 'time_will_tell', github: 'thewca/time_will_tell'
gem 'redcarpet'
gem 'bootstrap-table-rails'
gem 'money-rails'
gem 'money-currencylayer-bank'
gem 'octokit'
gem 'stripe'
gem 'oauth2'
Expand Down
5 changes: 5 additions & 0 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,10 @@ GEM
money (~> 6.12)
money (6.16.0)
i18n (>= 0.6.4, <= 2)
money-currencylayer-bank (0.7.2)
json (>= 1.8)
monetize (~> 1.4)
money (~> 6.7)
money-rails (1.15.0)
activesupport (>= 3.0)
monetize (~> 1.9)
Expand Down Expand Up @@ -884,6 +888,7 @@ DEPENDENCIES
mail_form
mini_magick
momentjs-rails!
money-currencylayer-bank
money-rails
mysql2
newrelic_rpm
Expand Down
2 changes: 2 additions & 0 deletions app_secrets.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ def vault_file(secret_name, file_path, refresh: true)
vault :TNOODLE_PUBLIC_KEY
vault :WRC_WEBHOOK_USERNAME
vault :WRC_WEBHOOK_PASSWORD
vault :CURRENCY_LAYER_API_KEY

# To allow logging in to staging with your prod account
unless EnvConfig.WCA_LIVE_SITE?
Expand All @@ -106,6 +107,7 @@ def vault_file(secret_name, file_path, refresh: true)
mandatory :OIDC_SECRET_KEY, :string
mandatory :STAGING_OAUTH_CLIENT, :string
mandatory :STAGING_OAUTH_SECRET, :string
mandatory :CURRENCY_LAYER_API_KEY, :string

optional :AWS_ACCESS_KEY_ID, :string, ''
optional :AWS_SECRET_ACCESS_KEY, :string, ''
Expand Down
24 changes: 22 additions & 2 deletions config/initializers/ruby_money.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,28 @@
# frozen_string_literal: true

require 'money/bank/currencylayer_bank'

Money.locale_backend = :i18n

eu_bank = EuCentralBank.new
Money.default_bank = eu_bank
if Rails.env.test? || Rails.env.development?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is Rails.env.local? which is exactly an equivalent of "test or dev"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that's nice, updated.

eu_bank = EuCentralBank.new
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also update_rates the same way you're doing for the new CurrencyLayer bank below

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes, done.

Money.default_bank = eu_bank
else
mclb = Money::Bank::CurrencylayerBank.new
mclb.access_key = AppSecrets.CURRENCY_LAYER_API_KEY
mclb.currencylayer = true
mclb.ttl_in_seconds = 86_400
mclb.cache = proc.new do |payload|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be Proc.new (note the capitalization)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually rubocop asked to replace Proc.new with proc. After re-reading again, I realized it's proc and not proc.new. I cannot test it right now as my access key has expired. Once the budget is approved, I'll test again using a new test key created with WCA's account.

key = 'money:currencylayer_bank'
if payload
Rails.cache.write(key, payload)
else
Rails.cache.read(key)
end
end
mclb.update_rates
Money.default_bank = mclb
end

Money.default_currency = Money::Currency.new("USD")
Money.rounding_mode = BigDecimal::ROUND_HALF_UP
7 changes: 0 additions & 7 deletions lib/dues_calculator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
# frozen_string_literal: true

module DuesCalculator
def self.update_exchange_rates_if_needed
if !Money.default_bank.rates_updated_at || Money.default_bank.rates_updated_at < 1.day.ago
Money.default_bank.update_rates
end
end
gregorbg marked this conversation as resolved.
Show resolved Hide resolved

def self.dues_for_n_competitors(country_iso2, base_entry_fee_lowest_denomination, currency_code, n)
dues_per_competitor_in_usd_money = dues_per_competitor_in_usd(country_iso2, base_entry_fee_lowest_denomination, currency_code)
if dues_per_competitor_in_usd_money.present?
Expand All @@ -21,7 +15,6 @@ def self.dues_for_n_competitors(country_iso2, base_entry_fee_lowest_denomination
def self.dues_per_competitor_in_usd(country_iso2, base_entry_fee_lowest_denomination, currency_code)
country_band = CountryBand.find_by(iso2: country_iso2)&.number

DuesCalculator.update_exchange_rates_if_needed
input_money_us_dollars = Money.new(base_entry_fee_lowest_denomination, currency_code).exchange_to("USD")

registration_fee_dues_us_dollars = input_money_us_dollars * CountryBand.percent_registration_fee_used_for_due_amount(country_band)
Expand Down
Loading