-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Update filters disabled error to include specific action #6195
Update filters disabled error to include specific action #6195
Conversation
Is this change significant enough to warrant an entry in |
Hi! Sorry for the delay, I'd go with considering this a bug fix! |
@@ -3,7 +3,7 @@ module Filters | |||
|
|||
class Disabled < RuntimeError | |||
def initialize | |||
super "Can't remove a filter when filters are disabled. Enable filters with 'config.filters = true'" | |||
super "Can't add or remove a filter when filters are disabled. Enable filters with 'config.filters = true'" |
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 was thinking we could stop overriding initialize
, and do class Disabled < RuntimeError; end
, and then pass the correct message for each of the cases instead?
That's a worthwhile suggestion so at least the message is specific to the action taking place. @javawizard I'd accept that change if you still want to get this in. Thank you. |
ActiveAdmin::Filters::Disabled is raised both when attempting add a filter and when attempting to remove a filter when filters are disabled. Change the message to suit both use cases.
d6a32fa
to
653ab2f
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #6195 +/- ##
=======================================
Coverage 98.94% 98.94%
=======================================
Files 197 197
Lines 4950 4950
=======================================
Hits 4898 4898
Misses 52 52
☔ View full report in Codecov by Sentry. |
ActiveAdmin::Filters::Disabled
is raised both when attempting add a filter and when attempting to remove a filter when filters are disabled. Change the message to suit both use cases.