FIX: handle boolean correctly on server #116
Conversation
|
What was broken before this? |
| custom_fields: { "enable_topic_voting" => true } | ||
| } | ||
| expect(Category.can_vote?(category.id)).to eq(true) | ||
| end |
There was a problem hiding this comment.
Nice nice, can you add a failing scenario please too?
There was a problem hiding this comment.
Sure, I think the failure should be with a simple false value. Reason being, we don't need to exercise ActiveRecord::Type::Boolean here right?
There was a problem hiding this comment.
Yup! Actually there's other cases where other custom_fields may exist after deleting enable_topic_voting. But that's alright. I trust you clicked through the scenarios.
There was a problem hiding this comment.
Actually, the following piece of code(with category_setting_attributes) should also be tested I agree. But I didn't test that coz it doesn't have to do anything with my fix. 😅
| fab!(:admin) { Fabricate(:user, admin: true) } | ||
|
|
||
| before do | ||
| SiteSetting.voting_enabled = true |
There was a problem hiding this comment.
Btw is this needed? (I don't see it used above)
There was a problem hiding this comment.
Indeed yes. Category.can_vote? will be false if the plugin is disabled.
The voting plugin's category setting is broken per https://meta.discourse.org/t/223880 and https://meta.discourse.org/t/223753
This PR fixes the reported issue.
Earlier the code was checking
params[:custom_fields].delete(:enable_topic_voting) == "true"which assumes we'll get a"true"string but we don't. This value is populated fromhttps://github.com/discourse/discourse-voting/blob/3d9f4c2996c98cca33380981fbac14bdcc702006/assets/javascripts/discourse/templates/connectors/category-custom-settings/feature-voting-settings.hbs#L5
and the controller converts this to a ruby boolean and hence the condition fails.