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
Add validation from wrong period type for the exchange rates + other improv. #1719
base: main
Are you sure you want to change the base?
Conversation
Small refactor or files_controller. Rename type to period_type for clarity.
|
||
it 'returns HTTP 400 bad request' do | ||
expect(response).to have_http_status(:bad_request) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the new behaviour that has been added.
When the period_type is wrong, it returns bad request 400.
filename = ExchangeRateFile.filename_for_download(type, format, year, month) | ||
validate_params! | ||
|
||
filename = ExchangeRateFile.filename_for_download(period_type, format, year, month) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed type
-> period_type
, for clarity.
period_year: year, | ||
period_month: month, | ||
format: format.to_s, | ||
).take | ||
end | ||
|
||
def validate_params! | ||
if period_type.nil? # && ExchangeRateFile::APPLICABLE_TYPES.include?(period_type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to remove this comment.
def type | ||
match_data = id.match(/^(monthly_csv_hmrc|monthly_csv|monthly_xml|average_csv|spot_csv)_/) | ||
def period_type | ||
regexp = Regexp.new("^(#{ExchangeRateFile::APPLICABLE_TYPES.join('|')})_") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am reusing the Const here so we avoid data duplication (model-controller)
def applicable_types | ||
where(type: APPLICABLE_TYPES) | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Jira link
https://transformuk.atlassian.net/browse/HOTT-4894
What?
Why?
To improve error messages and error status.
Make the code cleaner.