s/find_by_slug/friendly_find!/ and use everywhere

.rubocop.yml

@@ -59,12 +59,6 @@ Rails:
 Rails/Date:
   EnforcedStyle: strict
 
-Rails/DynamicFindBy:
-  Whitelist:
-  # TODO(glebm): Remove these in v0.9.0.
-  - find_by_slug
-  - find_by_slug!
-
 Rails/FindBy:
   Enabled: true
   Include:

CHANGELOG.md

@@ -9,6 +9,12 @@ This release contains new functionality and backwards-incompatible changes.
 * You can configure which notifiers are enabled: remove the email notifier totally, or add other notifiers (e.g. Pushover, possibly Slack) by adjusting the
  `Thredded.notifiers` configuration in your initializer. See the default initializer for examples.
 
+## Changed
+
+* Removed `Topic.find_by_slug` and `PrivateTopic.find_by_slug` methods.
+  Added `friendly_find!` to `Messageboard`, `Topic`, and `PrivateTopic` instead to avoid confusion with the Rails
+  dynamic finders.
+
 
 # v0.8.4
 

app/controllers/thredded/application_controller.rb

@@ -76,7 +76,7 @@ def authorize_creating(obj)
     end
 
     def update_user_activity
-      return if messageboard.nil? || !signed_in?
+      return if !messageboard_or_nil || !signed_in?
 
       Thredded::ActivityUpdaterJob.perform_later(
         thredded_current_user.id,
@@ -88,10 +88,14 @@ def pundit_user
       thredded_current_user
     end
 
+    # Returns the `@messageboard` instance variable.
+    # If `@messageboard` is not set, it first sets it to the messageboard with the slug or ID given by
+    # `params[:messageboard_id]`.
+    #
+    # @return [Thredded::Messageboard]
+    # @raise [Thredded::Errors::MessageboardNotFound] if the messageboard with the given slug does not exist.
     def messageboard
-      @messageboard ||= params[:messageboard_id].presence && Messageboard.friendly.find(params[:messageboard_id])
-    rescue ActiveRecord::RecordNotFound
-      raise Thredded::Errors::MessageboardNotFound
+      @messageboard ||= Messageboard.friendly_find!(params[:messageboard_id])
     end
 
     def messageboard_or_nil

app/controllers/thredded/messageboards_controller.rb

@@ -27,12 +27,12 @@ def create
     end
 
     def edit
-      @messageboard = Messageboard.friendly.find(params[:id])
+      @messageboard = Messageboard.friendly_find!(params[:id])
       authorize @messageboard, :update?
     end
 
     def update
-      @messageboard = Messageboard.friendly.find(params[:id])
+      @messageboard = Messageboard.friendly_find!(params[:id])
       authorize @messageboard, :update?
       if @messageboard.update(messageboard_params)
         redirect_to messageboard_topics_path(@messageboard), notice: I18n.t('thredded.messageboard.updated_notice')

app/controllers/thredded/private_topics_controller.rb

@@ -72,8 +72,13 @@ def current_page
       (params[:page] || 1).to_i
     end
 
+    # Returns the `@private_topic` instance variable.
+    # If `@private_topic` is not set, it first sets it to the topic with the slug or ID given by `params[:id]`.
+    #
+    # @return [Thredded::PrivateTopic]
+    # @raise [Thredded::Errors::PrivateTopicNotFound] if the topic with the given slug does not exist.
     def private_topic
-      @private_topic ||= Thredded::PrivateTopic.find_by_slug(params[:id])
+      @private_topic ||= Thredded::PrivateTopic.friendly_find!(params[:id])
     end
 
     def private_topic_params

app/controllers/thredded/topics_controller.rb

@@ -129,8 +129,13 @@ def follow_change_response(following:)
       end
     end
 
+    # Returns the `@topic` instance variable.
+    # If `@topic` is not set, it first sets it to the topic with the slug or ID given by `params[:id]`.
+    #
+    # @return [Thredded::Topic]
+    # @raise [Thredded::Errors::TopicNotFound] if the topic with the given slug does not exist.
     def topic
-      @topic ||= messageboard.topics.find_by_slug!(params[:id])
+      @topic ||= messageboard.topics.friendly_find!(params[:id])
     end
 
     def topic_params

app/models/thredded/messageboard.rb

@@ -81,6 +81,17 @@ def ensure_position
       order(topics_count: :desc)
     }
     # rubocop:enable Style/Lambda
+
+    # Finds the messageboard by its slug or ID, or raises Thredded::Errors::MessageboardNotFound.
+    # @param slug_or_id [String]
+    # @return [Thredded::Messageboard]
+    # @raise [Thredded::Errors::MessageboardNotFound] if the messageboard with the given slug does not exist.
+    def self.friendly_find!(slug_or_id)
+      friendly.find(slug_or_id)
+    rescue ActiveRecord::RecordNotFound
+      raise Thredded::Errors::MessageboardNotFound
+    end
+
     def last_user
       last_topic.try(:last_user)
     end

app/models/thredded/private_topic.rb

@@ -48,8 +48,12 @@ class PrivateTopic < ActiveRecord::Base
 
     before_validation :ensure_user_in_private_users
 
-    def self.find_by_slug(slug)
-      friendly.find(slug)
+    # Finds the topic by its slug or ID, or raises Thredded::Errors::PrivateTopicNotFound.
+    # @param slug_or_id [String]
+    # @return [Thredded::PrivateTopic]
+    # @raise [Thredded::Errors::PrivateTopicNotFound] if the topic with the given slug does not exist.
+    def self.friendly_find!(slug_or_id)
+      friendly.find(slug_or_id)
     rescue ActiveRecord::RecordNotFound
       raise Thredded::Errors::PrivateTopicNotFound
     end

app/models/thredded/topic.rb

@@ -77,9 +77,12 @@ class Topic < ActiveRecord::Base
     after_commit :update_messageboard_last_topic, on: :update, if: -> { previous_changes.include?('moderation_state') }
     after_update :update_last_user_and_time_from_last_post!, if: -> { previous_changes.include?('moderation_state') }
 
-    # TODO(glebm): Rename this to friendly_find! in v0.9.0 to avoid confusion with the dynamic Rails finders.
-    def self.find_by_slug!(slug)
-      friendly.find(slug)
+    # Finds the topic by its slug or ID, or raises Thredded::Errors::TopicNotFound.
+    # @param slug_or_id [String]
+    # @return [Thredded::Topic]
+    # @raise [Thredded::Errors::TopicNotFound] if the topic with the given slug does not exist.
+    def self.friendly_find!(slug_or_id)
+      friendly.find(slug_or_id)
     rescue ActiveRecord::RecordNotFound
       raise Thredded::Errors::TopicNotFound
     end

app/views/thredded/private_topics/index.html.erb

@@ -11,7 +11,6 @@
       <%= render 'thredded/private_topics/no_private_topics' %>
     <% else -%>
       <%= render 'thredded/private_topics/form',
-        messageboard:  messageboard,
         private_topic: @new_private_topic,
         css_class:     'thredded--is-compact',
         placeholder:   t('thredded.private_topics.form.title_placeholder_start') %>

app/views/thredded/private_topics/new.html.erb

@@ -5,7 +5,6 @@
 <%= thredded_page do %>
   <section class="thredded--main-section">
     <%= render 'thredded/private_topics/form',
-               messageboard:  messageboard,
                private_topic: @private_topic,
                placeholder:   t('thredded.private_topics.form.title_placeholder_new') if @private_topic %>
   </section>

spec/models/thredded/topic_spec.rb

@@ -2,15 +2,15 @@
 require 'spec_helper'
 
 module Thredded
-  describe Topic, '.find_by_slug!' do
+  describe Topic, '.friendly_find!' do
     it 'finds the topic' do
       topic = create(:topic, title: 'Oh Hello')
 
-      expect(Topic.find_by_slug!('oh-hello')).to eq topic
+      expect(Topic.friendly_find!('oh-hello')).to eq topic
     end
 
     it 'raises Thredded::Errors::TopicNotFound error' do
-      expect { Topic.find_by_slug!('rubbish') }
+      expect { Topic.friendly_find!('rubbish') }
         .to raise_error(Thredded::Errors::TopicNotFound)
     end
   end