Code review suggestions from akihikodaki

Also fixed a syntax error in tests for AccountInteractions.
This commit is contained in:
aschmitz 2017-10-22 20:27:48 -05:00
parent 8c6ecd5616
commit 6b70c2ca12
5 changed files with 5 additions and 7 deletions

View file

@ -22,7 +22,7 @@ export default function mutes(state = initialState, action) {
state.setIn(['new', 'notifications'], true);
});
case MUTES_TOGGLE_HIDE_NOTIFICATIONS:
return state.setIn(['new', 'notifications'], !state.getIn(['new', 'notifications']));
return state.updateIn(['new', 'notifications'], (old) => !old);
default:
return state;
}

View file

@ -79,8 +79,7 @@ module AccountInteractions
mute = mute_relationships.create_with(hide_notifications: notifications).find_or_create_by!(target_account: other_account)
# When toggling a mute between hiding and allowing notifications, the mute will already exist, so the find_or_create_by! call will return the existing Mute without updating the hide_notifications attribute. Therefore, we check that hide_notifications? is what we want and set it if it isn't.
if mute.hide_notifications? != notifications
mute.hide_notifications = notifications
mute.save!
mute.update!(hide_notifications: notifications)
end
end

View file

@ -1,5 +1,5 @@
class AddHideNotificationsToMute < ActiveRecord::Migration[5.1]
def change
add_column :mutes, :hide_notifications, :boolean, default: false, null: false
add_column :mutes, :hide_notifications, :boolean, default: true, null: false
end
end

View file

@ -16,7 +16,7 @@ RSpec.describe Api::V2::MutesController, type: :controller do
get :index, params: { limit: 1 }
end
let(:mutes) { JSON.parse(response.body) }
let(:mutes) { body_as_json }
it 'returns http success' do
expect(response).to have_http_status(:success)
@ -27,7 +27,7 @@ RSpec.describe Api::V2::MutesController, type: :controller do
end
it 'returns whether the mute hides notifications' do
expect(mutes.first["hide_notifications"]).to be false
expect(mutes.first[:hide_notifications]).to be false
end
end
end

View file

@ -4,7 +4,6 @@ describe AccountInteractions do
describe 'muting an account' do
let(:me) { Fabricate(:account, username: 'Me') }
let(:you) { Fabricate(:account, username: 'You') }
end
context 'with the notifications option unspecified' do
before do