Rewrote issue patch tests and used them to refactor

.travis.yml

@@ -29,7 +29,7 @@ env:
 before_install:
     - export PLUGIN_NAME=redmine_tags
     - export REDMINE_PATH=$HOME/redmine
-    - svn co http://svn.redmine.org/redmine/tags/$REDMINE_VER $REDMINE_PATH
+    - git clone https://github.com/redmine/redmine.git --branch $REDMINE_VER --depth 1 $REDMINE_PATH
     - git clone https://github.com/ZitecCOM/redmine_testing_gems.git --depth 1 $REDMINE_PATH/plugins/redmine_testing_gems
     - ln -s $TRAVIS_BUILD_DIR $REDMINE_PATH/plugins/$PLUGIN_NAME
     - cp config/database-$DB-travis.yml $REDMINE_PATH/config/database.yml

init.rb

@@ -16,7 +16,7 @@
 # You should have received a copy of the GNU General Public License
 # along with redmine_tags.  If not, see <http://www.gnu.org/licenses/>.
 
-require_dependency 'redmine_tags'
+require 'redmine_tags'
 
 ActionDispatch::Callbacks.to_prepare do
   paths = '/lib/redmine_tags/{patches/*_patch,hooks/*_hook}.rb'

lib/redmine_tags/patches/issue_patch.rb

@@ -62,37 +62,29 @@ module ClassMethods
         #   * open_only - Boolean. Whenever search within open issues only.
         #   * name_like - String. Substring to filter found tags.
         def available_tags(options = {})
-          ids_scope = Issue.visible.select("#{ Issue.table_name }.id").joins(:project)
-          ids_scope = ids_scope.on_project(options[:project]) if options[:project]
-          ids_scope = ids_scope.open.joins(:status) if options[:open_only]
-          conditions = ['']
+          issues_scope = Issue.visible.select('issues.id').joins(:project)
+          issues_scope = issues_scope.on_project(options[:project]) if options[:project]
+          issues_scope = issues_scope.joins(:status).open if options[:open_only]
 
-          sql_query = ids_scope.to_sql
+          result_scope = ActsAsTaggableOn::Tag
+            .joins(:taggings)
+            .select('tags.id, tags.name, tags.taggings_count, COUNT(taggings.id) as count')
+            .group('tags.id, tags.name, tags.taggings_count')
+            .where(taggings: { taggable_type: 'Issue', taggable_id: issues_scope})
 
-          conditions[0] << <<-SQL
-            tag_id IN (
-              SELECT taggings.tag_id
-                FROM taggings
-              WHERE taggings.taggable_id IN (
-                  #{ sql_query }
-                )
-                AND taggings.taggable_type = 'Issue'
-            )
-          SQL
-          # limit to the tags matching given %name_like%
           if options[:name_like]
-            conditions[0] << case self.connection.adapter_name
+            matcher = "%#{options[:name_like].downcase}%"
+
+            case connection.adapter_name
             when 'PostgreSQL'
-              "AND tags.name ILIKE ?"
+              result_scope = result_scope.where('tags.name ILIKE ?', matcher)
             else
-              "AND tags.name LIKE ?"
+              result_scope = result_scope.where('tags.name LIKE ?', matcher)
             end
-            conditions << "%#{options[:name_like].downcase}%"
+
           end
 
-          # TODO: which one of these to keep?
-          self.specific_tag_counts(conditions: conditions, taggable_id_sql: sql_query)
-          # self.all_tag_counts(:conditions => conditions, :order => "tags.name ASC")
+          result_scope
         end
 
         def remove_unused_tags!
@@ -103,63 +95,6 @@ def remove_unused_tags!
           SQL
           unused.each(&:destroy)
         end
-
-        ##
-        # Calculate the tag counts for all tags.
-        #
-        # @param [Hash] options Options:
-        #   * :start_at   - Restrict the tags to those created after a certain time
-        #   * :end_at     - Restrict the tags to those created before a certain time
-        #   * :conditions - A piece of SQL conditions to add to the query
-        #   * :limit      - The maximum number of tags to return
-        #   * :order      - A piece of SQL to order by. Eg 'tags.count desc' or 'taggings.created_at desc'
-        #   * :at_least   - Exclude tags with a frequency less than the given value
-        #   * :at_most    - Exclude tags with a frequency greater than the given value
-        #   * :on         - Scope the find to only include a certain context
-        def specific_tag_counts(options = {})
-          options.assert_valid_keys :start_at, :end_at, :conditions, :at_least, :at_most, :order, :limit, :on, :id, :taggable_id_sql
-          scope = {}
-          ## Generate conditions:
-          options[:conditions] = sanitize_sql(options[:conditions]) if options[:conditions]
-          start_at_conditions = sanitize_sql(["taggings.created_at >= ?", options.delete(:start_at)]) if options[:start_at]
-          end_at_conditions = sanitize_sql(["taggings.created_at <= ?", options.delete(:end_at)])   if options[:end_at]
-          taggable_conditions = sanitize_sql(["taggings.taggable_type = ?", base_class.name])
-          taggable_conditions << sanitize_sql([" AND taggings.taggable_id = ?", options.delete(:id)])  if options[:id]
-          taggable_conditions << sanitize_sql([" AND taggings.context = ?", options.delete(:on).to_s]) if options[:on]
-          tagging_conditions = [taggable_conditions, scope[:conditions],
-            start_at_conditions, end_at_conditions].compact.reverse
-          tag_conditions = [options[:conditions]].compact.reverse
-          # Generate joins:
-          taggable_join = "INNER JOIN #{ table_name } ON #{ table_name }.#{ primary_key } = taggings.taggable_id"
-          # Current model is STI descendant, so add type checking to the join condition
-          taggable_join << " AND #{ table_name }.#{ inheritance_column } = '#{ name }'" unless descends_from_active_record?
-          tagging_joins = [taggable_join, scope[:joins]].compact
-          tag_joins = [].compact
-          # Generate scope:
-          tagging_scope = ActsAsTaggableOn::Tagging.select("taggings.tag_id, COUNT(taggings.tag_id) AS tags_count")
-          tag_scope = ActsAsTaggableOn::Tag.select("tags.*, taggings.tags_count AS count")
-            .order(options[:order]).limit(options[:limit])
-          # Joins and conditions
-          tagging_joins.each {|join| tagging_scope = tagging_scope.joins join }
-          tagging_conditions.each {|condition| tagging_scope = tagging_scope.where condition }
-          tag_joins.each {|join| tag_scope = tag_scope.joins join }
-          tag_conditions.each {|condition| tag_scope = tag_scope.where(condition) }
-          # GROUP BY and HAVING clauses:
-          at_least = sanitize_sql(["COUNT(taggings.tag_id) >= ?", options.delete(:at_least)]) if options[:at_least]
-          at_most = sanitize_sql(["COUNT(taggings.tag_id) <= ?", options.delete(:at_most)]) if options[:at_most]
-          having = ["COUNT(taggings.tag_id) > 0", at_least, at_most].compact.join(' AND ')
-          group_columns = "taggings.tag_id"
-          # Append the current scope to the scope, because we can't use scope(:find) in RoR 3.0 anymore:
-          scoped_select = "#{ table_name }.#{ primary_key }"
-          select_query = "#{ select(scoped_select).to_sql }"
-          select_query = options[:taggable_id_sql] if options[:taggable_id_sql]
-          res = ActiveRecord::Base.connection.select_all(select_query).map { |item| item.values }.flatten.compact.join(",")
-          res = "NULL" if res.blank?
-          tagging_scope = tagging_scope.where("taggings.taggable_id IN(#{ res })")
-          tagging_scope = tagging_scope.group(group_columns).having(having)
-          tag_scope = tag_scope.joins("JOIN (#{ tagging_scope.to_sql }) AS taggings ON taggings.tag_id = tags.id")
-          tag_scope
-        end
       end
 
       module InstanceMethods

spec/models/issue_patch_spec.rb

@@ -8,59 +8,173 @@
     expect(Issue.included_modules).to include(patch)
   end
 
-  context 'with default settings' do
-    let(:author)        { create :admin }
-    let(:priority)      { create :issue_priority }
-    let(:status_open)   { create :issue_status }
-    let(:status_closed) { create :issue_status, is_closed: true }
-    let(:tracker)       { create :tracker, default_status_id: status_open.id }
-    let(:project_1) do
-      project = create :project
-      project.trackers = [tracker]
-      project.save
-      project
-    end
-    let(:project_2) do
-      project = create :project
-      project.trackers = [tracker]
-      project.save
-      project
-    end
+  let(:author)        { create :user }
+  let(:role)          { create :role, :manager }
+  let(:priority)      { create :issue_priority }
+  let(:status_open)   { create :issue_status }
+  let(:status_closed) { create :issue_status, is_closed: true }
+  let(:tracker)       { create :tracker, default_status_id: status_open.id }
+  let(:project_1) do
+    project = create :project
+    project.trackers = [tracker]
+    project.save
+    member = create(
+      :member,
+      project_id: project.id,
+      role_ids:   [role.id],
+      user_id:    author.id
+    )
+    create :member_role, member_id: member.id, role_id: role.id
+    project
+  end
+  let(:project_2) do
+    project = create :project
+    project.trackers = [tracker]
+    project.save
+    member = create(
+      :member,
+      project_id: project.id,
+      role_ids:   [role.id],
+      user_id:    author.id
+    )
+    create :member_role, member_id: member.id, role_id: role.id
+    project
+  end
 
+  context '.available_tags' do
     before :example do
       allow(User).to receive(:current).and_return(author)
-      create_issue(project_1, %w{a1 a2}, author, tracker, status_open, priority)
-      create_issue(project_1, %w{a2 a3}, author, tracker, status_open, priority)
-      create_issue(project_1, %w{a4 a5}, author, tracker, status_closed, priority)
-      create_issue(project_2, %w{b6 b7}, author, tracker, status_closed, priority)
-      create_issue(project_2, %w{b8 b9}, author, tracker, status_open, priority)
     end
 
-    it 'returns a list of distinct tags' do
-      expect(Issue.available_tags.size).to eq(9)
+    it 'returns an empty relation when no issues exist' do
+      result = Issue.available_tags
+      expect(result).to be_an(ActiveRecord::Relation)
+      expect(result).to eq([])
+    end
+
+    it 'returns an empty relation when no issues have tags' do
+      create_issue(project_1, [], author, tracker, status_open, priority)
+      result = Issue.available_tags
+      expect(result).to be_an(ActiveRecord::Relation)
+      expect(result.to_a).to eq([])
     end
 
-    it 'allows listing tags of open issues only' do
-      expect(Issue.available_tags(open_only: true).size).to eq(5)
+    it 'returns one tag when an issue has one tag' do
+      create_issue(project_1, %w[a], author, tracker, status_open, priority)
+      result_scope = Issue.available_tags
+      expect(result_scope).to be_an(ActiveRecord::Relation)
+      result = result_scope.to_a
+      expect(result.size).to eq(1)
+      expect(result[0]).to be_an(ActsAsTaggableOn::Tag)
+      expect(result[0].name).to eq('a')
     end
 
-    it 'allows listing tags of specific project only' do
-      expect(Issue.available_tags(project: project_1).size).to eq(5)
-      expect(Issue.available_tags(project: project_2).size).to eq(4)
-      expect(Issue.available_tags(open_only: true, project: project_1).size).to eq(3)
-      expect(Issue.available_tags(open_only: true, project: project_2).size).to eq(2)
+    it 'returns one tag when manny issues have the same tag' do
+      create_issue(project_1, %w[a], author, tracker, status_open, priority)
+      create_issue(project_1, %w[a], author, tracker, status_open, priority)
+      result_scope = Issue.available_tags
+      expect(result_scope).to be_an(ActiveRecord::Relation)
+      result = result_scope.to_a
+      expect(result.size).to eq(1)
+      expect(result[0]).to be_an(ActsAsTaggableOn::Tag)
+      expect(result[0].name).to eq('a')
     end
 
-    it 'allows listing tags found by name' do
-      expect(Issue.available_tags(name_like: 'a').size).to eq(5)
-      expect(Issue.available_tags(name_like: 'b').size).to eq(4)
-      expect(Issue.available_tags(name_like: 'a1').size).to eq(1)
-      expect(Issue.available_tags(name_like: 'a2').size).to eq(1)
+    it 'returns all tags only once ' do
+      create_issue(project_1, %w[a b], author, tracker, status_open, priority)
+      create_issue(project_1, %w[b c], author, tracker, status_open, priority)
+      result_scope = Issue.available_tags
+      expect(result_scope).to be_an(ActiveRecord::Relation)
+      result = result_scope.to_a
+      expect(result.size).to eq(3)
+      result.each_with_index do |tag, index|
+        expect(tag).to be_an(ActsAsTaggableOn::Tag)
+        expect(%w[a b c]).to include(tag.name)
+      end
+    end
+
+    it 'returns tags for a specific project' do
+      create_issue(project_1, %w[a], author, tracker, status_open, priority)
+      create_issue(project_2, %w[c], author, tracker, status_open, priority)
+      result_scope = Issue.available_tags(project: project_1)
+      expect(result_scope).to be_an(ActiveRecord::Relation)
+      result = result_scope.to_a
+      expect(result.size).to eq(1)
+      expect(result[0]).to be_an(ActsAsTaggableOn::Tag)
+      expect(result[0].name).to eq('a')
+    end
 
-      expect(Issue.available_tags(name_like: 'a', project: project_1).size).to eq(5)
-      expect(Issue.available_tags(name_like: 'b', project: project_1).size).to eq(0)
-      expect(Issue.available_tags(name_like: 'a', open_only: true, project: project_1).size).to eq(3)
-      expect(Issue.available_tags(name_like: 'b', open_only: true, project: project_1).size).to eq(0)
+    it 'returns tags regardless of type of status' do
+      create_issue(project_1, %w[a], author, tracker, status_open, priority)
+      create_issue(project_1, %w[b], author, tracker, status_closed, priority)
+      result_scope = Issue.available_tags
+      expect(result_scope).to be_an(ActiveRecord::Relation)
+      result = result_scope.to_a.uniq
+      expect(result.size).to eq(2)
+      result.each_with_index do |tag, index|
+        expect(tag).to be_an(ActsAsTaggableOn::Tag)
+        expect(%w[a b]).to include(tag.name)
+      end
     end
+
+    it 'returns tags only for open issues when open_only: true' do
+      create_issue(project_1, %w[a], author, tracker, status_open, priority)
+      create_issue(project_1, %w[b], author, tracker, status_closed, priority)
+      result_scope = Issue.available_tags(open_only: true)
+      expect(result_scope).to be_an(ActiveRecord::Relation)
+      result = result_scope.to_a
+      expect(result.size).to eq(1)
+      expect(result[0]).to be_an(ActsAsTaggableOn::Tag)
+      expect(result[0].name).to eq('a')
+    end
+
+    it 'returns tags fitered by name' do
+      create_issue(project_1, %w[a1], author, tracker, status_open, priority)
+      create_issue(project_1, %w[a2 b1], author, tracker, status_open, priority)
+      result_scope = Issue.available_tags(name_like: 'a')
+      expect(result_scope).to be_an(ActiveRecord::Relation)
+      result = result_scope.to_a
+      expect(result.size).to eq(2)
+      result.each_with_index do |tag, index|
+        expect(tag).to be_an(ActsAsTaggableOn::Tag)
+        expect(%w[a1 a2]).to include(tag.name)
+      end
+    end
+
+    it 'returns tags fitered by name for open issues' do
+      create_issue(project_1, %w[a1], author, tracker, status_open, priority)
+      create_issue(project_1, %w[a2], author, tracker, status_closed, priority)
+      result_scope = Issue.available_tags(name_like: 'a', open_only: true)
+      expect(result_scope).to be_an(ActiveRecord::Relation)
+      result = result_scope.to_a
+      expect(result.size).to eq(1)
+      expect(result[0]).to be_an(ActsAsTaggableOn::Tag)
+      expect(result[0].name).to eq('a1')
+    end
+
+    it 'returns tags fitered by name for open issues and specific project' do
+      create_issue(project_1, %w[a1], author, tracker, status_open, priority)
+      create_issue(project_1, %w[a2], author, tracker, status_closed, priority)
+      create_issue(project_2, %w[a3], author, tracker, status_open, priority)
+      result_scope = Issue.available_tags(
+        name_like: 'a',
+        open_only: true,
+        project:   project_1
+      )
+      expect(result_scope).to be_an(ActiveRecord::Relation)
+      result = result_scope.to_a
+      expect(result.size).to eq(1)
+      expect(result[0]).to be_an(ActsAsTaggableOn::Tag)
+      expect(result[0].name).to eq('a1')
+    end
+  end
+
+  context '.remove_unused_tags!' do
+  end
+
+  context '.specific_tag_counts' do
+  end
+
+  context '#copy_from_with_redmine_tags' do
   end
 end

spec/support/setup_support.rb

@@ -10,8 +10,12 @@ def create_issue(project, tags, author, tracker, status, priority)
       status:  status,
       tracker: tracker
     )
-    issue.tag_list = tags
+    issue.tag_list = tags if tags.any?
     issue.save!
+    if status.is_closed?
+      issue.status = status
+      issue.save!
+    end
     issue
   end
 end