manual rubocop fixes (#1163)

.rubocop.yml

@@ -22,3 +22,8 @@ Style/Documentation:
 
 Style/EmptyMethod:
   EnforcedStyle: expanded
+
+# I like this cop, but occasionally code is more readable without a guard clause,
+# and I don't want to write rubocop:disable comments every time that happens
+Style/GuardClause:
+  Enabled: false

app/controllers/maps_controller.rb

@@ -2,7 +2,9 @@
 
 class MapsController < ApplicationController
   before_action :require_user, only: %i[create update destroy events follow unfollow]
-  before_action :set_map, only: %i[show conversation update destroy contains events export follow unfollow unfollow_from_email]
+  before_action :set_map, only: %i[show conversation update destroy
+                                   contains events export
+                                   follow unfollow unfollow_from_email]
   after_action :verify_authorized
 
   # GET maps/:id

app/helpers/topics_helper.rb

@@ -17,7 +17,7 @@ module TopicsHelper
 
         rtype: is_map ? 'map' : 'topic',
         inmaps: is_map ? [] : t.inmaps(current_user),
-        inmapsLinks: is_map ? [] : t.inmapsLinks(current_user),
+        inmapsLinks: is_map ? [] : t.inmaps_links(current_user),
         type: is_map ? metamap_metacode.name : t.metacode.name,
         typeImageURL: is_map ? metamap_metacode.icon : t.metacode.icon,
         mapCount: is_map ? 0 : t.maps.count,

app/models/event.rb

@@ -11,7 +11,7 @@ class Event < ApplicationRecord
   belongs_to :map
   belongs_to :user
 
-  scope :chronologically, -> { order('created_at asc') }
+  scope :chronologically, (-> { order('created_at asc') })
 
   after_create :notify_webhooks!, if: :map
 

app/models/follow.rb

@@ -11,13 +11,11 @@ class Follow < ApplicationRecord
 
   after_create :add_subsetting
 
-  scope :active, -> {
+  scope :active, (-> { where(muted: false) })
-    where(muted: false)
-  }
 
   private
 
   def add_subsetting
-    follow_reason = FollowReason.create!(follow: self)
+    FollowReason.create!(follow: self)
   end
 end

app/models/map.rb

@@ -153,11 +153,10 @@ class Map < ApplicationRecord
   end
 
   def after_updated_async
-    if ATTRS_TO_WATCH.any? { |k| changed_attributes.key?(k) }
+    return unless ATTRS_TO_WATCH.any? { |k| changed_attributes.key?(k) }
-      FollowService.follow(self, updated_by, 'contributed')
+    FollowService.follow(self, updated_by, 'contributed')
-      # NotificationService.notify_followers(self, 'map_updated', changed_attributes)
+    # NotificationService.notify_followers(self, 'map_updated', changed_attributes)
-      # or better yet publish an event
+    # or better yet publish an event
-    end
   end
   handle_asynchronously :after_updated_async
 

app/models/mapping.rb

@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
 class Mapping < ApplicationRecord
-  scope :topicmapping, -> { where(mappable_type: :Topic) }
+  scope :topicmapping, (-> { where(mappable_type: :Topic) })
-  scope :synapsemapping, -> { where(mappable_type: :Synapse) }
+  scope :synapsemapping, (-> { where(mappable_type: :Synapse) })
 
   belongs_to :mappable, polymorphic: true
   belongs_to :map, class_name: 'Map', foreign_key: 'map_id', touch: true
@@ -53,17 +53,17 @@ class Mapping < ApplicationRecord
   handle_asynchronously :after_created_async
 
   def after_updated
-    if (mappable_type == 'Topic') && (xloc_changed? || yloc_changed?)
+    return unless (mappable_type == 'Topic') && (xloc_changed? || yloc_changed?)
-      meta = { 'x': xloc, 'y': yloc, 'mapping_id': id }
+    meta = { 'x': xloc, 'y': yloc, 'mapping_id': id }
-      Events::TopicMovedOnMap.publish!(mappable, map, updated_by, meta)
+    Events::TopicMovedOnMap.publish!(mappable, map, updated_by, meta)
-      ActionCable.server.broadcast 'map_' + map.id.to_s, type: 'topicMoved', id: mappable.id, mapping_id: id, x: xloc, y: yloc
+    ActionCable.server.broadcast('map_' + map.id.to_s, type: 'topicMoved',
-    end
+                                                       id: mappable.id, mapping_id: id,
+                                                       x: xloc, y: yloc)
   end
 
   def after_updated_async
-    if (mappable_type == 'Topic') && (xloc_changed? || yloc_changed?)
+    return unless (mappable_type == 'Topic') && (xloc_changed? || yloc_changed?)
-      FollowService.follow(map, updated_by, 'contributed')
+    FollowService.follow(map, updated_by, 'contributed')
-    end
   end
   handle_asynchronously :after_updated_async
 

app/models/metacode.rb

@@ -6,13 +6,11 @@ class Metacode < ApplicationRecord
   has_many :topics
 
   # This method associates the attribute ":aws_icon" with a file attachment
-  has_attached_file :aws_icon, styles: {
+  has_attached_file :aws_icon, styles: { ninetysix: ['96x96#', :png] },
-    ninetysix: ['96x96#', :png]
-  },
                                default_url: 'https://s3.amazonaws.com/metamaps-assets/metacodes/generics/96px/gen_wildcard.png'
 
   # Validate the attached icon is image/jpg, image/png, etc
-  validates_attachment_content_type :aws_icon, content_type: /\Aimage\/.*\Z/
+  validates_attachment_content_type :aws_icon, content_type: %r{\Aimage/.*\Z}
 
   validate :aws_xor_manual_icon
   validate :manual_icon_https
@@ -31,15 +29,13 @@ class Metacode < ApplicationRecord
   def as_json(options = {})
     default = super(options)
     default[:icon] = icon
-    default.except('aws_icon_file_name', 'aws_icon_content_type', 'aws_icon_file_size', 'aws_icon_updated_at', 'manual_icon')
+    default.except(
+      'aws_icon_file_name', 'aws_icon_content_type', 'aws_icon_file_size', 'aws_icon_updated_at',
+      'manual_icon'
+    )
   end
 
-  def hasSelected(user)
+  def in_metacode_set(metacode_set)
-    return true if user.settings.metacodes.include? id.to_s
-    false
-  end
-
-  def inMetacodeSet(metacode_set)
     return true if metacode_sets.include? metacode_set
     false
   end
@@ -56,10 +52,9 @@ class Metacode < ApplicationRecord
   end
 
   def manual_icon_https
-    if manual_icon.present?
+    return if manual_icon.blank?
-      unless manual_icon.starts_with? 'https'
+    unless manual_icon.starts_with? 'https'
-        errors.add(:base, 'Manual icon must begin with https')
+      errors.add(:base, 'Manual icon must begin with https')
-      end
     end
   end
 end

app/models/permitted_params.rb

@@ -1,14 +1,18 @@
 # frozen_string_literal: true
 
-class PermittedParams < Struct.new(:params)
+class PermittedParams
   %w[map synapse topic mapping token].each do |kind|
     define_method(kind) do
       permitted_attributes = send("#{kind}_attributes")
-      params.require(kind).permit(*permitted_attributes)
+      @params.require(kind).permit(*permitted_attributes)
     end
     alias_method :"api_#{kind}", kind.to_sym
   end
 
+  def initialize(params)
+    @params = params
+  end
+
   alias read_attribute_for_serialization send
 
   def token_attributes

app/models/synapse.rb

@@ -22,9 +22,9 @@ class Synapse < ApplicationRecord
 
   validates :category, inclusion: { in: ['from-to', 'both'], allow_nil: true }
 
-  scope :for_topic, ->(topic_id = nil) {
+  scope :for_topic, (lambda do |topic_id = nil|
     where(topic1_id: topic_id).or(where(topic2_id: topic_id))
-  }
+  end)
 
   before_create :set_perm_by_defer
   after_create :after_created_async
@@ -73,7 +73,7 @@ class Synapse < ApplicationRecord
   protected
 
   def set_perm_by_defer
-    permission = defer_to_map.permission if defer_to_map
+    defer_to_map&.permission
   end
 
   def after_created_async
@@ -83,15 +83,15 @@ class Synapse < ApplicationRecord
   handle_asynchronously :after_created_async
 
   def after_updated
-    if ATTRS_TO_WATCH.any? { |k| changed_attributes.key?(k) }
+    return unless ATTRS_TO_WATCH.any? { |k| changed_attributes.key?(k) }
-      new = attributes.select { |k| ATTRS_TO_WATCH.include?(k) }
+
-      old = changed_attributes.select { |k| ATTRS_TO_WATCH.include?(k) }
+    new = attributes.select { |k| ATTRS_TO_WATCH.include?(k) }
-      meta = new.merge(old) # we are prioritizing the old values, keeping them
+    old = changed_attributes.select { |k| ATTRS_TO_WATCH.include?(k) }
-      meta['changed'] = changed_attributes.keys.select { |k| ATTRS_TO_WATCH.include?(k) }
+    meta = new.merge(old) # we are prioritizing the old values, keeping them
-      Events::SynapseUpdated.publish!(self, updated_by, meta)
+    meta['changed'] = changed_attributes.keys.select { |k| ATTRS_TO_WATCH.include?(k) }
-      maps.each do |map|
+    Events::SynapseUpdated.publish!(self, updated_by, meta)
-        ActionCable.server.broadcast 'map_' + map.id.to_s, type: 'synapseUpdated', id: id
+    maps.each do |map|
-      end
+      ActionCable.server.broadcast 'map_' + map.id.to_s, type: 'synapseUpdated', id: id
     end
   end
 

app/models/topic.rb

@@ -39,13 +39,13 @@ class Topic < ApplicationRecord
     topics1.or(topics2)
   end
 
-  scope :relatives, ->(topic_id = nil, user = nil) {
+  scope :relatives, (lambda do |topic_id = nil, user = nil|
     # should only see topics through *visible* synapses
     # e.g. Topic A (commons) -> synapse (private) -> Topic B (commons) must be filtered out
     topic_ids = Pundit.policy_scope(user, Synapse.where(topic1_id: topic_id)).pluck(:topic2_id)
     topic_ids += Pundit.policy_scope(user, Synapse.where(topic2_id: topic_id)).pluck(:topic1_id)
     where(id: topic_ids.uniq)
-  }
+  end)
 
   delegate :name, to: :user, prefix: true
 
@@ -65,13 +65,13 @@ class Topic < ApplicationRecord
     Pundit.policy_scope(user, maps).map(&:name)
   end
 
-  def inmapsLinks(user)
+  def inmaps_links(user)
     Pundit.policy_scope(user, maps).map(&:id)
   end
 
   def as_json(options = {})
     super(methods: %i[user_name user_image collaborator_ids])
-      .merge(inmaps: inmaps(options[:user]), inmapsLinks: inmapsLinks(options[:user]),
+      .merge(inmaps: inmaps(options[:user]), inmapsLinks: inmaps_links(options[:user]),
              map_count: map_count(options[:user]), synapse_count: synapse_count(options[:user]))
   end
 
@@ -127,14 +127,9 @@ class Topic < ApplicationRecord
         end
       end
     end
-    if output_format == 'array'
+    return output if output_format == 'array'
-      return output
+    return output.join('; ') if output_format == 'text'
-    elsif output_format == 'text'
+    raise 'invalid argument to synapses_csv'
-      return output.join('; ')
-    else
-      raise 'invalid argument to synapses_csv'
-    end
-    output
   end
 
   def topic_autocomplete_method
@@ -144,7 +139,7 @@ class Topic < ApplicationRecord
   protected
 
   def set_perm_by_defer
-    permission = defer_to_map.permission if defer_to_map
+    self.permission = defer_to_map.permission if defer_to_map
   end
 
   def create_metamap?
@@ -163,22 +158,22 @@ class Topic < ApplicationRecord
   handle_asynchronously :after_created_async
 
   def after_updated
-    if ATTRS_TO_WATCH.any? { |k| changed_attributes.key?(k) }
+    return unless ATTRS_TO_WATCH.any? { |k| changed_attributes.key?(k) }
-      new = attributes.select { |k| ATTRS_TO_WATCH.include?(k) }
+
-      old = changed_attributes.select { |k| ATTRS_TO_WATCH.include?(k) }
+    new = attributes.select { |k| ATTRS_TO_WATCH.include?(k) }
-      meta = new.merge(old) # we are prioritizing the old values, keeping them
+    old = changed_attributes.select { |k| ATTRS_TO_WATCH.include?(k) }
-      meta['changed'] = changed_attributes.keys.select { |k| ATTRS_TO_WATCH.include?(k) }
+    meta = new.merge(old) # we are prioritizing the old values, keeping them
-      Events::TopicUpdated.publish!(self, updated_by, meta)
+    meta['changed'] = changed_attributes.keys.select { |k| ATTRS_TO_WATCH.include?(k) }
-      maps.each do |map|
+    Events::TopicUpdated.publish!(self, updated_by, meta)
-        ActionCable.server.broadcast 'map_' + map.id.to_s, type: 'topicUpdated', id: id
+    maps.each do |map|
-      end
+      ActionCable.server.broadcast 'map_' + map.id.to_s, type: 'topicUpdated', id: id
     end
   end
 
   def after_updated_async
-    if ATTRS_TO_WATCH.any? { |k| changed_attributes.key?(k) }
+    return unless ATTRS_TO_WATCH.any? { |k| changed_attributes.key?(k) }
-      FollowService.follow(self, updated_by, 'contributed')
+
-    end
+    FollowService.follow(self, updated_by, 'contributed')
   end
   handle_asynchronously :after_updated_async
 

app/models/user.rb

@@ -52,24 +52,24 @@ class User < ApplicationRecord
   validates_attachment_content_type :image, content_type: %r{\Aimage/.*\Z}
 
   # override default as_json
-  def as_json(_options = {})
+  def as_json(options = {})
     json = { id: id,
              name: name,
              image: image.url(:sixtyfour),
              admin: admin }
-    if _options[:follows]
+    if options[:follows]
       json['follows'] = {
         topics: following.active.where(followed_type: 'Topic').to_a.map(&:followed_id),
         maps: following.active.where(followed_type: 'Map').to_a.map(&:followed_id)
       }
     end
-    if _options[:follow_settings]
+    if options[:follow_settings]
       json['follow_topic_on_created'] = settings.follow_topic_on_created == '1'
       json['follow_topic_on_contributed'] = settings.follow_topic_on_contributed == '1'
       json['follow_map_on_created'] = settings.follow_map_on_created == '1'
       json['follow_map_on_contributed'] = settings.follow_map_on_contributed == '1'
     end
-    json['email'] = email if _options[:email]
+    json['email'] = email if options[:email]
     json
   end
 
@@ -106,10 +106,14 @@ class User < ApplicationRecord
   end
 
   def most_used_metacodes
-    topics.to_a.each_with_object(Hash.new(0)) do |topic, memo|
+    metacode_counts = topics.to_a.each_with_object(Hash.new(0)) do |topic, list_so_far|
-      memo[topic.metacode_id] += 1
+      list_so_far[topic.metacode_id] += 1
-      memo
+      list_so_far
-    end.to_a.sort { |a, b| b[1] <=> a[1] }.map { |i| i[0] }.slice(0, 5)
+    end
+    id_count_pairs = metacode_counts.to_a
+    id_count_pairs.sort! { |a, b| b[1] <=> a[1] }
+    metacode_ids = id_count_pairs.map { |i| i[0] }
+    metacode_ids.slice(0, 5)
   end
 
   # generate a random 8 letter/digit code that they can use to invite people
@@ -132,11 +136,11 @@ class User < ApplicationRecord
   end
 
   def has_map_open(map)
-    latestEvent = Event.where(map: map, user: self)
+    latest_event = Event.where(map: map, user: self)
-                       .where(kind: %w[user_present_on_map user_not_present_on_map])
+                        .where(kind: %w[user_present_on_map user_not_present_on_map])
-                       .order(:created_at)
+                        .order(:created_at)
-                       .last
+                        .last
-    latestEvent && latestEvent.kind == 'user_present_on_map'
+    latest_event && latest_event.kind == 'user_present_on_map'
   end
 
   def has_map_with_synapse_open(synapse)

app/models/user_preference.rb

@@ -5,20 +5,24 @@ class UserPreference
                 :follow_map_on_created, :follow_map_on_contributed
 
   def initialize
-    array = []
+    @metacodes = init_metacodes.compact
-    %w[Action Aim Idea Question Note Wildcard Subject].each do |m|
+    @metacode_focus = @metacodes[0]
+    initialize_follow_settings
+  end
+
+  private
+
+  def init_metacodes
+    %w[Action Aim Idea Question Note Wildcard Subject].map do |m|
       begin
         metacode = Metacode.find_by(name: m)
-        array.push(metacode.id.to_s) if metacode
+        metacode.id.to_s if metacode
       rescue ActiveRecord::StatementInvalid
         if m == 'Action'
           Rails.logger.warn('TODO: remove this travis workaround in user_preference.rb')
         end
       end
-    end
+    end.compact
-    @metacodes = array
-    @metacode_focus = array[0]
-    initialize_follow_settings
   end
 
   def initialize_follow_settings

app/models/webhooks/slack/synapse_added_to_map.rb

@@ -3,6 +3,8 @@
 class Webhooks::Slack::SynapseAddedToMap < Webhooks::Slack::Base
   def text
     connector = eventable.desc.empty? ? '->' : eventable.desc
-    "\"*#{eventable.topic1.name}* #{connector} *#{eventable.topic2.name}*\" was added as a connection by *#{event.user.name}* to the map *#{view_map_on_metamaps}*"
+    "\"*#{eventable.topic1.name}* #{connector} *#{eventable.topic2.name}*\"" \
+      " was added as a connection by *#{event.user.name}*" \
+      " to the map *#{view_map_on_metamaps}*"
   end
 end

app/models/webhooks/slack/synapse_removed_from_map.rb

@@ -3,7 +3,11 @@
 class Webhooks::Slack::SynapseRemovedFromMap < Webhooks::Slack::Base
   def text
     connector = eventable.desc.empty? ? '->' : eventable.desc
+
     # TODO: express correct directionality of arrows when desc is empty
-    "\"*#{eventable.topic1.name}* #{connector} *#{eventable.topic2.name}*\" was removed by *#{event.user.name}* as a connection from the map *#{view_map_on_metamaps}*"
+
+    "\"*#{eventable.topic1.name}* #{connector} *#{eventable.topic2.name}*\"" \
+      " was removed by *#{event.user.name}* as a connection" \
+      " from the map *#{view_map_on_metamaps}*"
   end
 end

app/policies/map_policy.rb

@@ -23,7 +23,7 @@ class MapPolicy < ApplicationPolicy
   end
 
   def conversation?
-    show? && %w[connorturland@gmail.com devin@callysto.com chessscholar@gmail.com solaureum@gmail.com ishanshapiro@gmail.com].include?(user.email)
+    show? && is_tester(user)
   end
 
   def create?

app/policies/synapse_policy.rb

@@ -3,12 +3,21 @@
 class SynapsePolicy < ApplicationPolicy
   class Scope < Scope
     def resolve
-      return scope.where(permission: %w[public commons]) unless user
+      return authenticated_scope if user
+      unauthenticated_scope
+    end
 
+    private
+
+    def authenticated_scope
       scope.where(permission: %w[public commons])
            .or(scope.where(defer_to_map_id: user.all_accessible_maps.map(&:id)))
            .or(scope.where(user_id: user.id))
     end
+
+    def unauthenticated_scope
+      scope.where(permission: %w[public commons])
+    end
   end
 
   def index?

app/policies/topic_policy.rb

@@ -3,12 +3,21 @@
 class TopicPolicy < ApplicationPolicy
   class Scope < Scope
     def resolve
-      return scope.where(permission: %w[public commons]) unless user
+      return authenticated_scope if user
+      unauthenticated_scope
+    end
 
+    private
+
+    def authenticated_scope
       scope.where(permission: %w[public commons])
            .or(scope.where(defer_to_map_id: user.all_accessible_maps.map(&:id)))
            .or(scope.where(user_id: user.id))
     end
+
+    def unauthenticated_scope
+      scope.where(permission: %w[public commons])
+    end
   end
 
   def index?

app/serializers/api/v2/user_serializer.rb

@@ -17,7 +17,6 @@ module Api
         object.image.url(:sixtyfour)
       end
 
-      # rubocop:disable Style/PredicateName
       def is_admin
         object.admin
       end

app/services/follow_service.rb

@@ -18,20 +18,20 @@ class FollowService
 
     def unfollow(entity, user)
       follow = Follow.where(followed: entity, user: user).first
-      if follow
+      return unless follow
-        unless follow.update(muted: true)
+
-          raise follow.errors.full_messages.join("\n")
+      unless follow.update(muted: true)
-        end
+        raise follow.errors.full_messages.join("\n")
       end
     end
 
     def remove_reason(entity, user, reason)
       return unless FollowReason::REASONS.include?(reason)
       follow = Follow.where(followed: entity, user: user).first
-      if follow
+      return unless follow
-        follow.follow_reason.update_attribute(reason, false)
+
-        follow.destroy unless follow.follow_reason.has_reason
+      follow.follow_reason.update_attribute(reason, false)
-      end
+      follow.destroy unless follow.follow_reason.has_reason
     end
 
     protected
@@ -40,17 +40,11 @@ class FollowService
       follow = Follow.where(followed: entity, user: user).first
       return false if follow && follow.muted
       if entity.class == Topic
-        if reason == 'created'
+        return user.settings.follow_topic_on_created == '1' if reason == 'created'
-          return user.settings.follow_topic_on_created == '1'
+        return user.settings.follow_topic_on_contributed == '1' if reason == 'contributed'
-        elsif reason == 'contributed'
-          return user.settings.follow_topic_on_contributed == '1'
-        end
       elsif entity.class == Map
-        if reason == 'created'
+        return user.settings.follow_map_on_created == '1' if reason == 'created'
-          return user.settings.follow_map_on_created == '1'
+        return user.settings.follow_map_on_contributed == '1' if reason == 'contributed'
-        elsif reason == 'contributed'
-          return user.settings.follow_map_on_contributed == '1'
-        end
       end
     end
   end

app/services/map_activity_service.rb

@@ -5,7 +5,7 @@ class MapActivityService
     'Activity on map ' + map.name
   end
 
-  def self.summarize_data(map, user, until_moment = DateTime.now)
+  def self.summarize_data(map, user, until_moment = DateTime.current)
     results = {
       stats: {}
     }
@@ -18,7 +18,7 @@ class MapActivityService
     message_count = Message.where(resource: map)
                            .where('created_at > ? AND created_at < ?', since, until_moment)
                            .where.not(user: user).count
-    results[:stats][:messages_sent] = message_count if message_count > 0
+    results[:stats][:messages_sent] = message_count if message_count.positive?
 
     moved_count = Event.where(kind: 'topic_moved_on_map', map: map)
                        .where('created_at > ? AND created_at < ?', since, until_moment)
@@ -42,7 +42,9 @@ class MapActivityService
     topics_added_events.each do |ta|
       num_adds = topics_added_events.where(eventable_id: ta.eventable_id).count
       num_removes = topics_removed_events.where(eventable_id: ta.eventable_id).count
-      topics_added_to_include[ta.eventable_id] = ta if num_adds > num_removes && scoped_topic_ids.include?(ta.eventable.id)
+      if num_adds > num_removes && scoped_topic_ids.include?(ta.eventable.id)
+        topics_added_to_include[ta.eventable_id] = ta
+      end
     end
     unless topics_added_to_include.keys.empty?
       results[:stats][:topics_added] = topics_added_to_include.keys.length
@@ -53,7 +55,9 @@ class MapActivityService
     topics_removed_events.each do |ta|
       num_adds = topics_added_events.where(eventable_id: ta.eventable_id).count
       num_removes = topics_removed_events.where(eventable_id: ta.eventable_id).count
-      topics_removed_to_include[ta.eventable_id] = ta if num_removes > num_adds && TopicPolicy.new(user, ta.eventable).show?
+      if num_removes > num_adds && TopicPolicy.new(user, ta.eventable).show?
+        topics_removed_to_include[ta.eventable_id] = ta
+      end
     end
     unless topics_removed_to_include.keys.empty?
       results[:stats][:topics_removed] = topics_removed_to_include.keys.length
@@ -74,7 +78,9 @@ class MapActivityService
     synapses_added_events.each do |ta|
       num_adds = synapses_added_events.where(eventable_id: ta.eventable_id).count
       num_removes = synapses_removed_events.where(eventable_id: ta.eventable_id).count
-      synapses_added_to_include[ta.eventable_id] = ta if num_adds > num_removes && scoped_synapse_ids.include?(ta.eventable.id)
+      if num_adds > num_removes && scoped_synapse_ids.include?(ta.eventable.id)
+        synapses_added_to_include[ta.eventable_id] = ta
+      end
     end
     unless synapses_added_to_include.keys.empty?
       results[:stats][:synapses_added] = synapses_added_to_include.keys.length
@@ -85,7 +91,9 @@ class MapActivityService
     synapses_removed_events.each do |ta|
       num_adds = synapses_added_events.where(eventable_id: ta.eventable_id).count
       num_removes = synapses_removed_events.where(eventable_id: ta.eventable_id).count
-      synapses_removed_to_include[ta.eventable_id] = ta if num_removes > num_adds && SynapsePolicy.new(user, ta.eventable).show?
+      if num_removes > num_adds && SynapsePolicy.new(user, ta.eventable).show?
+        synapses_removed_to_include[ta.eventable_id] = ta
+      end
     end
     unless synapses_removed_to_include.keys.empty?
       results[:stats][:synapses_removed] = synapses_removed_to_include.keys.length

app/services/notification_service.rb

@@ -7,7 +7,7 @@ class NotificationService
   include ActionView::Helpers::SanitizeHelper
 
   def self.renderer
-    renderer ||= ApplicationController.renderer.new(
+    @renderer ||= ApplicationController.renderer.new(
       http_host: ENV['MAILER_DEFAULT_URL'],
       https:     Rails.env.production? ? true : false
     )
@@ -70,19 +70,25 @@ class NotificationService
 
   def self.access_request(request)
     subject = access_request_subject(request.map)
-    body = renderer.render(template: 'map_mailer/access_request', locals: { map: request.map, request: request }, layout: false)
+    body = renderer.render(template: 'map_mailer/access_request',
+                           locals: { map: request.map, request: request },
+                           layout: false)
     request.map.user.notify(subject, body, request, false, MAP_ACCESS_REQUEST, true, request.user)
   end
 
   def self.access_approved(request)
     subject = access_approved_subject(request.map)
-    body = renderer.render(template: 'map_mailer/access_approved', locals: { map: request.map }, layout: false)
+    body = renderer.render(template: 'map_mailer/access_approved',
+                           locals: { map: request.map },
+                           layout: false)
     request.user.notify(subject, body, request, false, MAP_ACCESS_APPROVED, true, request.map.user)
   end
 
   def self.invite_to_edit(user_map)
     subject = invite_to_edit_subject(user_map.map)
-    body = renderer.render(template: 'map_mailer/invite_to_edit', locals: { map: user_map.map, inviter: user_map.map.user }, layout: false)
+    body = renderer.render(template: 'map_mailer/invite_to_edit',
+                           locals: { map: user_map.map, inviter: user_map.map.user },
+                           layout: false)
     user_map.user.notify(subject, body, user_map, false, MAP_INVITE_TO_EDIT, true, user_map.map.user)
   end
 

app/views/metacode_sets/_form.html.erb

@@ -35,7 +35,7 @@
        <% $i = 0 %>
        <% @m = Metacode.order("name").all %>
        <% while $i < (Metacode.all.length / 4) do %>
-            <li id="<%= @m[$i].id %>" <% if not @m[$i].inMetacodeSet(@metacode_set) %>class="toggledOff"<% end %> 
+            <li id="<%= @m[$i].id %>" <% if not @m[$i].in_metacode_set(@metacode_set) %>class="toggledOff"<% end %> 
               onclick="Metamaps.Admin.liClickHandler.call(this);">
               <img src="<%= asset_path @m[$i].icon %>" alt="<%= @m[$i].name %>" />
               <p><%= @m[$i].name.downcase %></p>
@@ -46,7 +46,7 @@
       </ul>
       <ul id="filters-two">
        <% while $i < (Metacode.all.length / 4 * 2) do %>
-            <li id="<%= @m[$i].id %>" <% if not @m[$i].inMetacodeSet(@metacode_set) %>class="toggledOff"<% end %>
+            <li id="<%= @m[$i].id %>" <% if not @m[$i].in_metacode_set(@metacode_set) %>class="toggledOff"<% end %>
               onclick="Metamaps.Admin.liClickHandler.call(this);">
               <img src="<%= asset_path @m[$i].icon %>" alt="<%= @m[$i].name %>" />
               <p><%= @m[$i].name.downcase %></p>
@@ -57,7 +57,7 @@
       </ul>
       <ul id="filters-three">         
        <% while $i < (Metacode.all.length / 4 * 3) do %>
-            <li id="<%= @m[$i].id %>" <% if not @m[$i].inMetacodeSet(@metacode_set) %>class="toggledOff"<% end %>
+            <li id="<%= @m[$i].id %>" <% if not @m[$i].in_metacode_set(@metacode_set) %>class="toggledOff"<% end %>
               onclick="Metamaps.Admin.liClickHandler.call(this);">
               <img src="<%= asset_path @m[$i].icon %>" alt="<%= @m[$i].name %>" />
               <p><%= @m[$i].name.downcase %></p>
@@ -68,7 +68,7 @@
       </ul>
       <ul id="filters-four">         
        <% while $i < Metacode.all.length do %>
-            <li id="<%= @m[$i].id %>" <% if not @m[$i].inMetacodeSet(@metacode_set) %>class="toggledOff"<% end %>
+            <li id="<%= @m[$i].id %>" <% if not @m[$i].in_metacode_set(@metacode_set) %>class="toggledOff"<% end %>
               onclick="Metamaps.Admin.liClickHandler.call(this);">
               <img src="<%= asset_path @m[$i].icon %>" alt="<%= @m[$i].name %>" />
               <p><%= @m[$i].name.downcase %></p>

app/views/metacode_sets/edit.html.erb

@@ -6,7 +6,7 @@
 
 <script>
 <% Metacode.all.each do |m| %>
-  <% if m.inMetacodeSet(@metacode_set) %>
+  <% if m.in_metacode_set(@metacode_set) %>
     Metamaps.Admin.selectMetacodes.push("<%= m.id %>");
   <% end %>
   Metamaps.Admin.allMetacodes.push("<%= m.id %>");

app/views/metacode_sets/new.html.erb

@@ -6,7 +6,7 @@
 
 <script>
 <% Metacode.all.each do |m| %>
-  <% if m.inMetacodeSet(@metacode_set) %>
+  <% if m.in_metacode_set(@metacode_set) %>
     Metamaps.Admin.selectMetacodes.push("<%= m.id %>");
   <% end %>
   Metamaps.Admin.allMetacodes.push("<%= m.id %>");

config/environments/development.rb

@@ -25,7 +25,7 @@ Rails.application.configure do
   # Print deprecation notices to the Rails logger
   config.active_support.deprecation = :log
 
-  config.action_mailer.preview_path = "#{Rails.root}/spec/mailers/previews"
+  config.action_mailer.preview_path = Rails.root.join('spec', 'mailers', 'previews')
 
   # Expands the lines which load the assets
   config.assets.debug = false

config/environments/production.rb

@@ -4,7 +4,9 @@
 Rails.application.configure do
   # Settings specified here will take precedence over those in config/application.rb
 
-  config.action_cable.allowed_request_origins = ['https://metamaps.herokuapp.com', 'http://metamaps.herokuapp.com', 'https://metamaps.cc']
+  config.action_cable.allowed_request_origins = [
+    'https://metamaps.herokuapp.com', 'http://metamaps.herokuapp.com', 'https://metamaps.cc'
+  ]
 
   # log to stdout
   logger = Logger.new(STDOUT)

config/initializers/delayed_job.rb

@@ -11,4 +11,4 @@ Delayed::Worker.class_eval do
   prepend ExceptionNotifierInDelayedJob
 end
 
-Delayed::Worker.logger = Logger.new(File.join(Rails.root, 'log', 'delayed_job.log'))
+Delayed::Worker.logger = Logger.new(Rails.root.join('log', 'delayed_job.log'))

config/initializers/internal_testers.rb

@@ -1,5 +1,8 @@
 # frozen_string_literal: true
 
 def is_tester(user)
-  user && %w[connorturland@gmail.com devin@callysto.com chessscholar@gmail.com solaureum@gmail.com ishanshapiro@gmail.com].include?(user.email)
+  user && %w[
+    connorturland@gmail.com devin@callysto.com chessscholar@gmail.com solaureum@gmail.com
+    ishanshapiro@gmail.com
+  ].include?(user.email)
 end

spec/controllers/explore_controller_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe ExploreController, type: :controller do
         expect(JSON.parse(response.body)).to eq []
       end
       it 'with 1 record' do
-        map = create(:map)
+        create(:map)
         get :active, format: :json
         expect(JSON.parse(response.body).class).to be Array
       end

spec/controllers/synapses_controller_spec.rb

@@ -40,7 +40,10 @@ RSpec.describe SynapsesController, type: :controller do
     context 'with private topics' do
       it 'redirects to /' do
         post :create, format: :json, params: {
-          synapse: valid_attributes.merge(topic1_id: create(:topic, permission: 'private'), topic2_id: create(:topic, permission: 'private'))
+          synapse: valid_attributes.merge(
+            topic1_id: create(:topic, permission: 'private'),
+            topic2_id: create(:topic, permission: 'private')
+          )
         }
         expect(response.status).to eq 302
         expect(response).to redirect_to('/')

spec/mailers/previews/map_activity_mailer_preview.rb

@@ -69,17 +69,20 @@ class MapActivityMailerPreview < ActionMailer::Preview
   end
 
   def generate_user
-    User.create(name: Faker::Name.name, email: Faker::Internet.email, password: 'password', password_confirmation: 'password', joinedwithcode: 'qwertyui')
+    User.create(name: Faker::Name.name, email: Faker::Internet.email,
+                password: 'password', password_confirmation: 'password',
+                joinedwithcode: 'qwertyui')
   end
 
   def generate_map
-    Map.create(name: Faker::HarryPotter.book, permission: 'commons', arranged: false, user: generate_user)
+    Map.create(name: Faker::HarryPotter.book, permission: 'commons',
+               arranged: false, user: generate_user)
   end
 
   def topic_added_to_map(map)
     user = generate_user
     topic = Topic.create(name: Faker::Friends.quote, permission: 'commons', user: user)
-    mapping = Mapping.create(map: map, mappable: topic, user: user)
+    Mapping.create(map: map, mappable: topic, user: user)
   end
 
   def topic_moved_on_map(mapping)
@@ -95,8 +98,9 @@ class MapActivityMailerPreview < ActionMailer::Preview
 
   def synapse_added_to_map(map, topic1, topic2)
     user = generate_user
-    topic = Synapse.create(desc: 'describes', permission: 'commons', user: user, topic1: topic1, topic2: topic2)
+    topic = Synapse.create(desc: 'describes', permission: 'commons',
-    mapping = Mapping.create(map: map, mappable: topic, user: user)
+                           user: user, topic1: topic1, topic2: topic2)
+    Mapping.create(map: map, mappable: topic, user: user)
   end
 
   def synapse_removed_from_map(mapping)

spec/rails_helper.rb

@@ -11,13 +11,13 @@ if Rails.env.production?
 end
 
 # require all support files
-Dir[Rails.root.join('spec/support/**/*.rb')].each { |f| require f }
+Dir[Rails.root.join('spec', 'support', '**', '*.rb')].each { |f| require f }
 
 ActiveRecord::Migration.maintain_test_schema!
 
 RSpec.configure do |config|
   # Remove this line if you're not using ActiveRecord or ActiveRecord fixtures
-  config.fixture_path = "#{::Rails.root}/spec/fixtures"
+  config.fixture_path = ::Rails.root.join('spec', 'fixtures')
 
   config.use_transactional_fixtures = true
 

spec/services/map_activity_service_spec.rb

@@ -16,7 +16,8 @@ RSpec.describe MapActivityService do
   describe 'topics added to map' do
     it 'includes a topic added within the last 24 hours' do
       topic = create(:topic)
-      mapping = create(:mapping, user: other_user, map: map, mappable: topic, created_at: 6.hours.ago)
+      create(:mapping, user: other_user, map: map, mappable: topic, created_at: 6.hours.ago)
+
       event = Event.find_by(kind: 'topic_added_to_map', eventable_id: topic.id)
       event.update_columns(created_at: 6.hours.ago)
       response = MapActivityService.summarize_data(map, email_user)
@@ -47,26 +48,33 @@ RSpec.describe MapActivityService do
       mapping.destroy
       Event.find_by(kind: 'topic_removed_from_map', eventable_id: topic.id).update_columns(created_at: 6.hours.ago)
       mapping2 = create(:mapping, user: other_user, map: map, mappable: topic, created_at: 5.hours.ago)
-      Event.where(kind: 'topic_added_to_map').where("meta->>'mapping_id' = ?", mapping2.id.to_s).first.update_columns(created_at: 5.hours.ago)
+      Event.where(kind: 'topic_added_to_map').where("meta->>'mapping_id' = ?", mapping2.id.to_s)
+           .first
+           .update_columns(created_at: 5.hours.ago)
       response = MapActivityService.summarize_data(map, email_user)
       expect(response).to eq empty_response
     end
 
     it 'excludes a topic added outside the last 24 hours' do
       topic = create(:topic)
-      mapping = create(:mapping, user: other_user, map: map, mappable: topic, created_at: 25.hours.ago)
+      create(:mapping, user: other_user, map: map, mappable: topic, created_at: 25.hours.ago)
+
       Event.find_by(kind: 'topic_added_to_map', eventable_id: topic.id).update_columns(created_at: 25.hours.ago)
+
       response = MapActivityService.summarize_data(map, email_user)
       expect(response).to eq empty_response
     end
 
     it 'excludes topics added by the user who will receive the data' do
       topic = create(:topic)
-      topic2 = create(:topic)
+      create(:mapping, user: other_user, map: map, mappable: topic, created_at: 5.hours.ago)
-      mapping = create(:mapping, user: other_user, map: map, mappable: topic, created_at: 5.hours.ago)
+
       event = Event.find_by(kind: 'topic_added_to_map', eventable_id: topic.id)
       event.update_columns(created_at: 5.hours.ago)
-      mapping2 = create(:mapping, user: email_user, map: map, mappable: topic2, created_at: 5.hours.ago)
+
+      topic2 = create(:topic)
+      create(:mapping, user: email_user, map: map, mappable: topic2, created_at: 5.hours.ago)
+
       Event.find_by(kind: 'topic_added_to_map', eventable_id: topic2.id).update_columns(created_at: 5.hours.ago)
       response = MapActivityService.summarize_data(map, email_user)
       expect(response[:stats][:topics_added]).to eq(1)
@@ -166,7 +174,7 @@ RSpec.describe MapActivityService do
   describe 'synapses added to map' do
     it 'includes a synapse added within the last 24 hours' do
       synapse = create(:synapse)
-      mapping = create(:mapping, user: other_user, map: map, mappable: synapse, created_at: 6.hours.ago)
+      create(:mapping, user: other_user, map: map, mappable: synapse, created_at: 6.hours.ago)
       event = Event.find_by(kind: 'synapse_added_to_map', eventable_id: synapse.id)
       event.update_columns(created_at: 6.hours.ago)
       response = MapActivityService.summarize_data(map, email_user)
@@ -197,14 +205,16 @@ RSpec.describe MapActivityService do
       mapping.destroy
       Event.find_by(kind: 'synapse_removed_from_map', eventable_id: synapse.id).update_columns(created_at: 6.hours.ago)
       mapping2 = create(:mapping, user: other_user, map: map, mappable: synapse, created_at: 5.hours.ago)
-      Event.where(kind: 'synapse_added_to_map').where("meta->>'mapping_id' = ?", mapping2.id.to_s).first.update_columns(created_at: 5.hours.ago)
+      Event.where(kind: 'synapse_added_to_map').where("meta->>'mapping_id' = ?", mapping2.id.to_s)
+           .first
+           .update_columns(created_at: 5.hours.ago)
       response = MapActivityService.summarize_data(map, email_user)
       expect(response).to eq empty_response
     end
 
     it 'excludes a synapse added outside the last 24 hours' do
       synapse = create(:synapse)
-      mapping = create(:mapping, user: other_user, map: map, mappable: synapse, created_at: 25.hours.ago)
+      create(:mapping, user: other_user, map: map, mappable: synapse, created_at: 25.hours.ago)
       Event.find_by(kind: 'synapse_added_to_map', eventable_id: synapse.id).update_columns(created_at: 25.hours.ago)
       response = MapActivityService.summarize_data(map, email_user)
       expect(response).to eq empty_response
@@ -212,11 +222,14 @@ RSpec.describe MapActivityService do
 
     it 'excludes synapses added by the user who will receive the data' do
       synapse = create(:synapse)
-      synapse2 = create(:synapse)
+      create(:mapping, user: other_user, map: map, mappable: synapse, created_at: 5.hours.ago)
-      mapping = create(:mapping, user: other_user, map: map, mappable: synapse, created_at: 5.hours.ago)
+
       event = Event.find_by(kind: 'synapse_added_to_map', eventable_id: synapse.id)
       event.update_columns(created_at: 5.hours.ago)
-      mapping2 = create(:mapping, user: email_user, map: map, mappable: synapse2, created_at: 5.hours.ago)
+
+      synapse2 = create(:synapse)
+      create(:mapping, user: email_user, map: map, mappable: synapse2, created_at: 5.hours.ago)
+
       Event.find_by(kind: 'synapse_added_to_map', eventable_id: synapse2.id).update_columns(created_at: 5.hours.ago)
       response = MapActivityService.summarize_data(map, email_user)
       expect(response[:stats][:synapses_added]).to eq(1)
@@ -262,7 +275,9 @@ RSpec.describe MapActivityService do
       mapping2.destroy
       event = Event.find_by(kind: 'synapse_removed_from_map', eventable_id: synapse.id)
       event.update_columns(created_at: 5.hours.ago)
-      Event.find_by(kind: 'synapse_removed_from_map', eventable_id: synapse2.id).update_columns(created_at: 5.hours.ago)
+      Event
+        .find_by(kind: 'synapse_removed_from_map', eventable_id: synapse2.id)
+        .update_columns(created_at: 5.hours.ago)
       response = MapActivityService.summarize_data(map, email_user)
       expect(response[:stats][:synapses_removed]).to eq(1)
       expect(response[:synapses_removed]).to eq([event])
@@ -297,7 +312,8 @@ RSpec.describe MapActivityService do
       old_topic = create(:topic, permission: 'commons', user: other_user)
       old_topic_mapping = create(:mapping, map: map, mappable: old_topic, user: other_user)
       old_private_topic = create(:topic, permission: 'private', user: other_user)
-      old_private_topic_mapping = create(:mapping, map: map, mappable: old_private_topic, user: other_user)
+      old_private_topic_mapping = create(:mapping, map: map, mappable: old_private_topic,
+                                                   user: other_user)
     end
     Timecop.return
 
@@ -347,7 +363,8 @@ RSpec.describe MapActivityService do
       old_synapse = create(:synapse, permission: 'commons', user: other_user)
       old_synapse_mapping = create(:mapping, map: map, mappable: old_synapse, user: other_user)
       old_private_synapse = create(:synapse, permission: 'private', user: other_user)
-      old_private_synapse_mapping = create(:mapping, map: map, mappable: old_private_synapse, user: other_user)
+      old_private_synapse_mapping = create(:mapping, map: map, mappable: old_private_synapse,
+                                                     user: other_user)
     end
     Timecop.return
 

spec/support/schema_matcher.rb

@@ -2,11 +2,10 @@
 
 RSpec::Matchers.define :match_json_schema do |schema_name|
   match do |response|
-    schema_directory = Rails.root.join('doc', 'api', 'schemas').to_s
+    schema_path = Rails.root.join('doc', 'api', 'schemas', "#{schema_name}.json").to_s
-    schema = "#{schema_directory}/#{schema_name}.json"
 
     # schema customizations
-    schema = JSON.parse(File.read(schema))
+    schema = JSON.parse(File.read(schema_path))
     schema = update_file_refs(schema)
 
     data = JSON.parse(response.body)
@@ -15,7 +14,7 @@ RSpec::Matchers.define :match_json_schema do |schema_name|
 end
 
 def get_json_example(resource)
-  filepath = "#{Rails.root}/doc/api/examples/#{resource}.json"
+  filepath = Rails.root.join('doc', 'api', 'examples', "#{resource}.json")
   OpenStruct.new(body: File.read(filepath))
 end
 
@@ -25,7 +24,7 @@ def update_file_refs(schema)
     schema[key] = if value.is_a? Hash
                     update_file_refs(value)
                   elsif key == '$ref'
-                    "#{Rails.root}/doc/api/schemas/#{value}"
+                    Rails.root.join('doc', 'api', 'schemas', value).to_s
                   else
                     value
                   end