Skip to content

Commit

Permalink
remove version checks now that we are officially on rails 5 (#349)
Browse files Browse the repository at this point in the history
  • Loading branch information
yuenmichelle1 authored Aug 2, 2024
1 parent 2b385df commit df9459f
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 102 deletions.
10 changes: 1 addition & 9 deletions app/services/concerns/talk_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,7 @@ def permitted_params
end

def rooted_params
# Parameters in Rails 5+ behave differently, because they no longer inherit from HashWithIndifferentAccess
# methods like slice will behave differently in Rails 4 than Rails 5
# In Rails 5, we will need to first to apply to_h and then slice.
# TODO: Can Remove version comparison once Prod is on Rails 5+
if Rails.version.starts_with?('4.2')
permitted_params.slice model_class.table_name
else
permitted_params.to_h.slice model_class.table_name
end
permitted_params.to_h.slice model_class.table_name
end

def unrooted_params
Expand Down
7 changes: 1 addition & 6 deletions spec/controllers/users_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,7 @@
it{ is_expected.to be_successful }

it 'should return the usernames' do
# TODO: Remove version check once on Rails 5
# In Rails versions < 5, PG results when casted to_a,
# numeric types will get converted to string.
# After Rails 5, there is no typecasting, unless stated.
# See https://github.com/rails/rails/commit/c51f9b61ce1e167f5f58f07441adcfa117694301
expected_user_id = Rails.version.starts_with?('5') ? user.id : user.id.to_s
expected_user_id = user.id
expect(response.json[:usernames]).to match_array [{
'id' => expected_user_id,
'login' => user.login,
Expand Down
16 changes: 3 additions & 13 deletions spec/mailers/notification_mailer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,34 +99,24 @@
before(:each){ mailer.instance_variable_set :@user, user1 }
let(:categories){ mailer.find_categories_for SubscriptionPreference.email_digests[frequency] }

# TODO: Once on Rails 5, Can Remove Version Check.
# Rails 5 introduces a change to ActiveRecord:Enum where it typecasts enum attributes to its string value.
# See: https://github.com/rails/rails/commit/c51f9b61ce1e167f5f58f07441adcfa117694301
# Luckily we can still send int values when doing a where clause on categories AND by Rails 5, we can also do a string array search of categories. So we do not need to update `find_notifications_for`
# Regardless, we will test thoroughly in Staging Canary When Up.
# Version Checks are on L112, L120, L128

context 'with immediate' do
let(:frequency){ :immediate }
subject{ categories }
# TODO: When on Rails 5, Remove Version Checks (See L102 For Details)
expected_categories = Rails.version.starts_with?('4.2') ? Subscription.categories.values_at(:mentions, :group_mentions, :system, :moderation_reports) : ['mentions', 'group_mentions', 'system', 'moderation_reports']
expected_categories = ['mentions', 'group_mentions', 'system', 'moderation_reports']
it{ is_expected.to match_array expected_categories }
end

context 'with daily' do
let(:frequency){ :daily }
subject{ categories }
# TODO: When on Rails 5, Remove Version Checks(See L102 For Details)
expected_categories = Rails.version.starts_with?('4.2') ? Subscription.categories.values_at(:participating_discussions, :followed_discussions) : ['participating_discussions', 'followed_discussions']
expected_categories = ['participating_discussions', 'followed_discussions']
it{ is_expected.to match_array expected_categories }
end

context 'with weekly' do
let(:frequency){ :weekly }
subject{ categories }
# TODO: When on Rails 5, Remove Version Checks(See L102 For Details)
expected_categories = Rails.version.starts_with?('4.2') ? Subscription.categories.values_at(:messages, :started_discussions) : ['messages', 'started_discussions']
expected_categories = ['messages', 'started_discussions']
it{ is_expected.to match_array expected_categories }
end

Expand Down
24 changes: 5 additions & 19 deletions spec/models/comment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -554,16 +554,8 @@ def mention(subject)
let(:comment){ create :comment }

it 'should queue the notification' do
# TODO: Once on Rails 5, Can Remove this Version Check
# In Rails Versions < 5, commit callbacks are not getting called in transactional tests.
# See https://stackoverflow.com/a/30901628/15768801 for more details.
if Rails.version.starts_with?('5')
allow(CommentSubscriptionWorker).to receive(:perform_async)
expect(CommentSubscriptionWorker).to have_received(:perform_async).with comment.id
else
expect(CommentSubscriptionWorker).to receive(:perform_async).with comment.id
comment.run_callbacks :commit
end
allow(CommentSubscriptionWorker).to receive(:perform_async)
expect(CommentSubscriptionWorker).to have_received(:perform_async).with comment.id
end
end

Expand Down Expand Up @@ -655,15 +647,9 @@ def mention(subject)

describe '#publish_to_event_stream_later' do
it 'should be triggered after a commit' do
# TODO: Once on Rails 5, Can Remove this Version Check
if Rails.version.starts_with?('5')
new_comment = build(:comment)
expect(new_comment).to receive :publish_to_event_stream_later
new_comment.save!
else
expect(comment).to receive :publish_to_event_stream_later
comment.run_callbacks :commit
end
new_comment = build(:comment)
expect(new_comment).to receive :publish_to_event_stream_later
new_comment.save!
end

it 'should enqueue the publish worker' do
Expand Down
17 changes: 4 additions & 13 deletions spec/models/data_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,19 +75,10 @@

describe '#spawn_worker' do
it 'should run the worker' do
# TODO: Once on Rails 5, Can Remove this Version Check
# In Rails Versions < 5, commit callbacks are not getting called in transactional tests.
# See https://stackoverflow.com/a/30901628/15768801 for more details.
if Rails.version.starts_with?('5')
allow(TagExportWorker).to receive(:perform_async)
data_request = build :tags_data_request
data_request.save!
expect(TagExportWorker).to have_received(:perform_async).with data_request.id
else
data_request = create :tags_data_request
expect(TagExportWorker).to receive(:perform_async).with data_request.id
data_request.run_callbacks :commit
end
allow(TagExportWorker).to receive(:perform_async)
data_request = build :tags_data_request
data_request.save!
expect(TagExportWorker).to have_received(:perform_async).with data_request.id
end
end

Expand Down
17 changes: 4 additions & 13 deletions spec/models/discussion_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,19 +191,10 @@ def create_discussion(position: nil)

describe '#notify_subscribers_later' do
it 'should queue the notification' do
# TODO: Once on Rails 5, Can Remove this Version Check
# In Rails Versions < 5, commit callbacks are not getting called in transactional tests.
# See https://stackoverflow.com/a/30901628/15768801 for more details.
if Rails.version.starts_with?('5')
allow(DiscussionSubscriptionWorker).to receive(:perform_async)
discussion = build :discussion
discussion.save!
expect(DiscussionSubscriptionWorker).to have_received(:perform_async).with discussion.id
else
discussion = create :discussion
expect(DiscussionSubscriptionWorker).to receive(:perform_async).with discussion.id
discussion.run_callbacks :commit
end
allow(DiscussionSubscriptionWorker).to receive(:perform_async)
discussion = build :discussion
discussion.save!
expect(DiscussionSubscriptionWorker).to have_received(:perform_async).with discussion.id
end
end

Expand Down
13 changes: 3 additions & 10 deletions spec/models/group_mention_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,9 @@

describe '#notify_later' do
it 'should queue the notification' do
# TODO: Once on Rails 5, Can Remove this Version Check
if Rails.version.starts_with?('5')
group_mention = build :group_mention
expect(GroupMentionWorker).to receive(:perform_async)
group_mention.save!
else
group_mention = create :group_mention
expect(GroupMentionWorker).to receive(:perform_async).with group_mention.id
group_mention.run_callbacks :commit
end
group_mention = build :group_mention
expect(GroupMentionWorker).to receive(:perform_async)
group_mention.save!
end
end

Expand Down
13 changes: 3 additions & 10 deletions spec/models/mention_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,9 @@

describe '#notify_later' do
it 'should queue the notification' do
# TODO: Once on Rails 5, Can Remove this Version Check
if Rails.version.starts_with?('5')
mention = build :mention, mentionable: focus
expect(MentionWorker).to receive(:perform_async)
mention.save!
else
mention = create :mention, mentionable: focus
expect(MentionWorker).to receive(:perform_async).with mention.id
mention.run_callbacks :commit
end
mention = build :mention, mentionable: focus
expect(MentionWorker).to receive(:perform_async)
mention.save!
end
end

Expand Down
12 changes: 3 additions & 9 deletions spec/models/moderation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,9 @@
include_context 'moderation subscriptions'

it 'should queue the notification' do
# TODO: Once on Rails 5, Can Remove this Version Check
if Rails.version.starts_with?('5')
new_moderation = build :moderation, section: 'project-1'
expect(ModerationNotificationWorker).to receive(:perform_async)
new_moderation.save!
else
expect(ModerationNotificationWorker).to receive(:perform_async).with moderation.id
moderation.run_callbacks :commit
end
new_moderation = build :moderation, section: 'project-1'
expect(ModerationNotificationWorker).to receive(:perform_async)
new_moderation.save!
end
end

Expand Down

0 comments on commit df9459f

Please sign in to comment.