From 6b70c2ca128eba5fa90b6177cf4a4137277d1787 Mon Sep 17 00:00:00 2001 From: aschmitz Date: Sun, 22 Oct 2017 20:27:48 -0500 Subject: [PATCH] Code review suggestions from akihikodaki Also fixed a syntax error in tests for AccountInteractions. --- app/javascript/mastodon/reducers/mutes.js | 2 +- app/models/concerns/account_interactions.rb | 3 +-- db/migrate/20170716191202_add_hide_notifications_to_mute.rb | 2 +- spec/controllers/api/v2/mutes_controller_spec.rb | 4 ++-- spec/models/concerns/account_interactions_spec.rb | 1 - 5 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/javascript/mastodon/reducers/mutes.js b/app/javascript/mastodon/reducers/mutes.js index 496e6846a..a96232dbd 100644 --- a/app/javascript/mastodon/reducers/mutes.js +++ b/app/javascript/mastodon/reducers/mutes.js @@ -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; } diff --git a/app/models/concerns/account_interactions.rb b/app/models/concerns/account_interactions.rb index a42b3939f..55ad812b2 100644 --- a/app/models/concerns/account_interactions.rb +++ b/app/models/concerns/account_interactions.rb @@ -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 diff --git a/db/migrate/20170716191202_add_hide_notifications_to_mute.rb b/db/migrate/20170716191202_add_hide_notifications_to_mute.rb index de7d2a4a2..2eeb31624 100644 --- a/db/migrate/20170716191202_add_hide_notifications_to_mute.rb +++ b/db/migrate/20170716191202_add_hide_notifications_to_mute.rb @@ -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 diff --git a/spec/controllers/api/v2/mutes_controller_spec.rb b/spec/controllers/api/v2/mutes_controller_spec.rb index eae5d889a..be9ff3327 100644 --- a/spec/controllers/api/v2/mutes_controller_spec.rb +++ b/spec/controllers/api/v2/mutes_controller_spec.rb @@ -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 diff --git a/spec/models/concerns/account_interactions_spec.rb b/spec/models/concerns/account_interactions_spec.rb index 10f25f835..a468549d8 100644 --- a/spec/models/concerns/account_interactions_spec.rb +++ b/spec/models/concerns/account_interactions_spec.rb @@ -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