mirror of
https://github.com/discourse/discourse.git
synced 2025-05-22 22:43:33 +08:00
DEV: Return 400 instead of 500 for invalid top period (#13828)
* DEV: Return 400 instead of 500 for invalid top period This change will prevent a fatal 500 error when passing in an invalid period param value to the `/top` route. * Check if the method exists first I couldn't get `ListController.respond_to?` to work, but was still able to check if the method exists with `ListController.action_methods.include?`. This way we can avoid relying on the `NoMethodError` exception which may be raised during the course of executing the method. * Just check if the period param value is valid * Use the new TopTopic.validate_period method
This commit is contained in:
@ -259,6 +259,7 @@ class ListController < ApplicationController
|
|||||||
options ||= {}
|
options ||= {}
|
||||||
period = params[:period]
|
period = params[:period]
|
||||||
period ||= ListController.best_period_for(current_user.try(:previous_visit_at), options[:category])
|
period ||= ListController.best_period_for(current_user.try(:previous_visit_at), options[:category])
|
||||||
|
TopTopic.validate_period(period)
|
||||||
public_send("top_#{period}", options)
|
public_send("top_#{period}", options)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -424,6 +424,23 @@ RSpec.describe ListController do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe 'Top' do
|
||||||
|
it 'renders top' do
|
||||||
|
get "/top"
|
||||||
|
expect(response.status).to eq(200)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'renders top with a period' do
|
||||||
|
get "/top?period=weekly"
|
||||||
|
expect(response.status).to eq(200)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'errors for invalid periods on top' do
|
||||||
|
get "/top?period=decadely"
|
||||||
|
expect(response.status).to eq(400)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe 'category' do
|
describe 'category' do
|
||||||
context 'in a category' do
|
context 'in a category' do
|
||||||
let(:category) { Fabricate(:category_with_definition) }
|
let(:category) { Fabricate(:category_with_definition) }
|
||||||
|
Reference in New Issue
Block a user