Improved tag management + added admin tests.

app/controllers/tags_controller.rb

@@ -35,7 +35,7 @@ class TagsController < ApplicationController
     success = Tag.tag_add(
       object: params[:object],
       o_id: params[:o_id],
-      item: params[:item].strip,
+      item: params[:item],
     )
     if success
       render json: success, status: :created
@@ -49,7 +49,7 @@ class TagsController < ApplicationController
     success = Tag.tag_remove(
       object: params[:object],
       o_id: params[:o_id],
-      item: params[:item].strip,
+      item: params[:item],
     )
     if success
       render json: success, status: :created
@@ -76,41 +76,24 @@ class TagsController < ApplicationController
   # POST /api/v1/tag_list
   def admin_create
     return if deny_if_not_role('Admin')
-    name = params[:name].strip
+    Tag::Item.lookup_by_name_and_create(params[:name])
-    if !Tag.tag_item_lookup(name)
-      Tag::Item.create(name: name)
-    end
     render json: {}
   end
 
   # PUT /api/v1/tag_list/:id
   def admin_rename
     return if deny_if_not_role('Admin')
-    name = params[:name].strip
+    Tag::Item.rename(
-    tag_item = Tag::Item.find(params[:id])
+      id: params[:id],
-    existing_tag_id = Tag.tag_item_lookup(name)
+      name: params[:name],
-    if existing_tag_id
+    )
-
-      # assign to already existing tag
-      Tag.where(tag_item_id: tag_item.id).update_all(tag_item_id: existing_tag_id)
-
-      # delete not longer used tag
-      tag_item.destroy
-
-    # update new tag name
-    else
-      tag_item.name = name
-      tag_item.save
-    end
     render json: {}
   end
 
   # DELETE /api/v1/tag_list/:id
   def admin_delete
     return if deny_if_not_role('Admin')
-    tag_item = Tag::Item.find(params[:id])
+    Tag::Item.remove(params[:id])
-    Tag.where(tag_item_id: tag_item.id).destroy_all
-    tag_item.destroy
     render json: {}
   end
 

app/models/observer/tag/ticket_history.rb

@@ -17,6 +17,7 @@ class Observer::Tag::TicketHistory < ActiveRecord::Observer
       history_object: 'Ticket',
       history_attribute: 'tag',
       value_to: record.tag_item.name,
+      created_by_id: record.created_by_id,
     )
   end
 
@@ -32,6 +33,7 @@ class Observer::Tag::TicketHistory < ActiveRecord::Observer
       history_object: 'Ticket',
       history_attribute: 'tag',
       value_to: record.tag_item.name,
+      created_by_id: record.created_by_id,
     )
   end
 end

app/models/tag.rb

@@ -4,11 +4,6 @@ class Tag < ApplicationModel
   belongs_to :tag_object,       class_name: 'Tag::Object'
   belongs_to :tag_item,         class_name: 'Tag::Item'
 
-  # rubocop:disable Style/ClassVars
-  @@cache_item = {}
-  @@cache_object = {}
-# rubocop:enable Style/ClassVars
-
 =begin
 
 add tags for certain object
@@ -23,18 +18,19 @@ add tags for certain object
 =end
 
   def self.tag_add(data)
+    data[:item].strip!
 
     # lookups
     if data[:object]
-      tag_object_id = tag_object_lookup(data[:object])
+      tag_object_id = Tag::Object.lookup_by_name_and_create(data[:object]).id
     end
     if data[:item]
-      tag_item_id = tag_item_lookup(data[:item].strip)
+      tag_item_id = Tag::Item.lookup_by_name_and_create(data[:item]).id
     end
 
-    # return in duplicate
+    # return if duplicate
     current_tags = tag_list(data)
-    return true if current_tags.include?(data[:item].strip)
+    return true if current_tags.include?(data[:item])
 
     # create history
     Tag.create(
@@ -53,29 +49,41 @@ add tags for certain object
 
 remove tags of certain object
 
-  Tag.tag_add(
+  Tag.tag_remove(
     object: 'Ticket',
     o_id: ticket.id,
     item: 'some tag',
     created_by_id: current_user.id,
   )
 
+or by ids
+
+  Tag.tag_remove(
+    tag_object_id: 123,
+    o_id: ticket.id,
+    tag_item_id: 123,
+    created_by_id: current_user.id,
+  )
+
 =end
 
   def self.tag_remove(data)
 
     # lookups
     if data[:object]
-      tag_object_id = tag_object_lookup(data[:object])
+      data[:tag_object_id] = Tag::Object.lookup_by_name_and_create(data[:object]).id
+    else
+      data[:object] = Tag::Object.lookup(id: data[:tag_object_id]).name
     end
     if data[:item]
-      tag_item_id = tag_item_lookup(data[:item].strip)
+      data[:item].strip!
+      data[:tag_item_id] = Tag::Item.lookup_by_name_and_create(data[:item]).id
     end
 
     # create history
     result = Tag.where(
-      tag_object_id: tag_object_id,
+      tag_object_id: data[:tag_object_id],
-      tag_item_id: tag_item_id,
+      tag_item_id: data[:tag_item_id],
       o_id: data[:o_id],
     )
     result.each(&:destroy)
@@ -92,8 +100,6 @@ tag list for certain object
   tags = Tag.tag_list(
     object: 'Ticket',
     o_id: ticket.id,
-    item: 'some tag',
-    created_by_id: current_user.id,
   )
 
 returns
@@ -103,83 +109,155 @@ returns
 =end
 
   def self.tag_list(data)
-    tag_object_id_requested = tag_object_lookup(data[:object])
+    tag_object_id_requested = Tag::Object.lookup(name: data[:object])
+    return [] if !tag_object_id_requested
     tag_search = Tag.where(
       tag_object_id: tag_object_id_requested,
       o_id: data[:o_id],
     )
     tags = []
     tag_search.each {|tag|
-      tags.push tag_item_lookup_id(tag.tag_item_id)
+      tag_item = Tag::Item.lookup(id: tag.tag_item_id)
+      next if !tag_item
+      tags.push tag_item.name
     }
     tags
   end
 
-  def self.tag_item_lookup_id(id)
+  class Object < ApplicationModel
+    validates :name, presence: true
 
-    # use cache
+=begin
-    return @@cache_item[id] if @@cache_item[id]
 
-    # lookup
+lookup by name and create tag item
-    tag_item = Tag::Item.find(id)
+
-    @@cache_item[id] = tag_item.name
+  tag_object = Tag::Object.lookup_by_name_and_create('some tag')
-    tag_item.name
+
+=end
+
+    def self.lookup_by_name_and_create(name)
+      name.strip!
+
+      tag_object = Tag::Object.lookup(name: name)
+      return tag_object if tag_object
+
+      Tag::Object.create(name: name)
     end
 
-  def self.tag_item_lookup(name)
+  end
 
-    # use cache
+  class Item < ApplicationModel
-    return @@cache_item[name] if @@cache_item[name]
+    validates   :name, presence: true
+    before_save :fill_namedowncase
 
-    # lookup
+=begin
-    tag_items = Tag::Item.where(name: name)
+
-    tag_items.each {|tag_item|
+lookup by name and create tag item
-      next if tag_item.name != name
+
-      @@cache_item[name] = tag_item.id
+  tag_item = Tag::Item.lookup_by_name_and_create('some tag')
-      return tag_item.id
+
+=end
+
+    def self.lookup_by_name_and_create(name)
+      name.strip!
+
+      tag_item = Tag::Item.lookup(name: name)
+      return tag_item if tag_item
+
+      Tag::Item.create(name: name)
+    end
+
+=begin
+
+rename tag items
+
+  Tag::Item.rename(
+    id: existing_tag_item_to_rename,
+    name: 'new tag item name',
+    updated_by_id: current_user.id,
+  )
+
+=end
+
+    def self.rename(data)
+
+      new_tag_name = data[:name].strip
+      old_tag_item = Tag::Item.find(data[:id])
+      already_existing_tag = Tag::Item.lookup(name: new_tag_name)
+
+      # merge old with new tag if already existing
+      if already_existing_tag
+
+        # re-assign old tag to already existing tag
+        Tag.where(tag_item_id: old_tag_item.id).each {|tag|
+
+          # check if tag already exists on object
+          if Tag.find_by(tag_object_id: tag.tag_object_id, o_id: tag.o_id, tag_item_id: already_existing_tag.id)
+            Tag.tag_remove(
+              tag_object_id: tag.tag_object_id,
+              o_id: tag.o_id,
+              tag_item_id: old_tag_item.id,
+            )
+            next
+          end
+
+          # re-assign
+          tag_object = Tag::Object.lookup(id: tag.tag_object_id)
+          tag.tag_item_id = already_existing_tag.id
+          tag.save
+
+          # touch reference objects
+          Tag.touch_reference_by_params(
+            object: tag_object.name,
+            o_id: tag.o_id,
+          )
         }
 
-    # create
+        # delete not longer used tag
-    tag_item = Tag::Item.create(name: name)
+        old_tag_item.destroy
-    @@cache_item[name] = tag_item.id
+
-    tag_item.id
+      # update new tag name
+      else
+
+        old_tag_item.name = new_tag_name
+        old_tag_item.save
+
+        # touch reference objects
+        Tag.where(tag_item_id: old_tag_item.id).each {|tag|
+          tag_object = Tag::Object.lookup(id: tag.tag_object_id)
+          Tag.touch_reference_by_params(
+            object: tag_object.name,
+            o_id: tag.o_id,
+          )
+        }
       end
 
-  def self.tag_object_lookup_id(id)
+      true
-
-    # use cache
-    return @@cache_object[id] if @@cache_object[id]
-
-    # lookup
-    tag_object = Tag::Object.find(id)
-    @@cache_object[id] = tag_object.name
-    tag_object.name
     end
 
-  def self.tag_object_lookup(name)
+=begin
 
-    # use cache
+remove tag item (destroy with reverences)
-    return @@cache_object[name] if @@cache_object[name]
 
-    # lookup
+  Tag::Item.remove(id)
-    tag_object = Tag::Object.find_by(name: name)
+
-    if tag_object
+=end
-      @@cache_object[name] = tag_object.id
+
-      return tag_object.id
+    def self.remove(id)
+
+      # search for references, destroy and touch
+      Tag.where(tag_item_id: id).each {|tag|
+        tag_object = Tag::Object.lookup(id: tag.tag_object_id)
+        tag.destroy
+        Tag.touch_reference_by_params(
+          object: tag_object.name,
+          o_id: tag.o_id,
+        )
+      }
+      Tag::Item.find(id).destroy
+      true
     end
 
-    # create
-    tag_object = Tag::Object.create(name: name)
-    @@cache_object[name] = tag_object.id
-    tag_object.id
-  end
-
-  class Object < ActiveRecord::Base
-  end
-
-  class Item < ActiveRecord::Base
-    before_save :fill_namedowncase
-
     def fill_namedowncase
       self.name_downcase = name.downcase
     end

test/unit/tag_test.rb

@@ -163,4 +163,202 @@ class TagTest < ActiveSupport::TestCase
       assert(!list.include?(tags[:item]), 'Tag entry destroyed')
     }
   end
+
+  test 'tags - real live' do
+
+    ticket1 = Ticket.create(
+      title: 'some title tag1',
+      group: Group.lookup(name: 'Users'),
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'new'),
+      priority: Ticket::Priority.lookup(name: '2 normal'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    ticket2 = Ticket.create(
+      title: 'some title tag2',
+      group: Group.lookup(name: 'Users'),
+      customer_id: 2,
+      state: Ticket::State.lookup(name: 'new'),
+      priority: Ticket::Priority.lookup(name: '2 normal'),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+    sleep 2
+    Tag.tag_add(
+      object: 'Ticket',
+      o_id: ticket1.id,
+      item: 'some tag1',
+      created_by_id: 1,
+    )
+    Tag.tag_add(
+      object: 'Ticket',
+      o_id: ticket1.id,
+      item: 'some tag2 ',
+      created_by_id: 1,
+    )
+    Tag.tag_add(
+      object: 'Ticket',
+      o_id: ticket1.id,
+      item: ' some tag3',
+      created_by_id: 1,
+    )
+    Tag.tag_add(
+      object: 'Ticket',
+      o_id: ticket1.id,
+      item: 'some TAG4',
+      created_by_id: 1,
+    )
+    Tag.tag_add(
+      object: 'Ticket',
+      o_id: ticket1.id,
+      item: ' some tag4',
+      created_by_id: 1,
+    )
+    Tag.tag_add(
+      object: 'Ticket',
+      o_id: ticket2.id,
+      item: 'some tag3',
+      created_by_id: 1,
+    )
+    Tag.tag_add(
+      object: 'Ticket',
+      o_id: ticket2.id,
+      item: 'some TAG4',
+      created_by_id: 1,
+    )
+    Tag.tag_add(
+      object: 'Ticket',
+      o_id: ticket2.id,
+      item: 'some tag4 ',
+      created_by_id: 1,
+    )
+
+    Tag.tag_remove(
+      object: 'Ticket',
+      o_id: ticket1.id,
+      item: 'some tag1',
+      created_by_id: 1,
+    )
+
+    ticket1_lookup1 = Ticket.lookup(id: ticket1.id)
+    assert_not_equal(ticket1.updated_at.to_s, ticket1_lookup1.updated_at.to_s)
+    ticket2_lookup1 = Ticket.lookup(id: ticket2.id)
+    assert_not_equal(ticket2.updated_at.to_s, ticket2_lookup1.updated_at.to_s)
+
+    tags_ticket1 = Tag.tag_list(
+      object: 'Ticket',
+      o_id: ticket1.id,
+    )
+    assert_equal(4, tags_ticket1.count)
+    assert(tags_ticket1.include?('some tag2'))
+    assert(tags_ticket1.include?('some tag3'))
+    assert(tags_ticket1.include?('some TAG4'))
+    assert(tags_ticket1.include?('some tag4'))
+
+    tags_ticket2 = Tag.tag_list(
+      object: 'Ticket',
+      o_id: ticket2.id,
+    )
+    assert_equal(3, tags_ticket2.count)
+    assert(tags_ticket2.include?('some tag3'))
+    assert(tags_ticket2.include?('some TAG4'))
+    assert(tags_ticket2.include?('some tag4'))
+
+    # rename tag
+    sleep 2
+    tag_item3 = Tag::Item.find_by(name: 'some tag3')
+    Tag::Item.rename(
+      id: tag_item3.id,
+      name: ' some tag33',
+      created_by_id: 1,
+    )
+
+    ticket1_lookup2 = Ticket.lookup(id: ticket1.id)
+    assert_not_equal(ticket1_lookup2.updated_at.to_s, ticket1_lookup1.updated_at.to_s)
+    ticket2_lookup2 = Ticket.lookup(id: ticket2.id)
+    assert_not_equal(ticket2_lookup2.updated_at.to_s, ticket2_lookup1.updated_at.to_s)
+
+    tags_ticket1 = Tag.tag_list(
+      object: 'Ticket',
+      o_id: ticket1.id,
+    )
+
+    assert_equal(4, tags_ticket1.count)
+    assert(tags_ticket1.include?('some tag2'))
+    assert(tags_ticket1.include?('some tag33'))
+    assert(tags_ticket1.include?('some TAG4'))
+    assert(tags_ticket1.include?('some tag4'))
+
+    tags_ticket2 = Tag.tag_list(
+      object: 'Ticket',
+      o_id: ticket2.id,
+    )
+    assert_equal(3, tags_ticket2.count)
+    assert(tags_ticket2.include?('some tag33'))
+    assert(tags_ticket2.include?('some TAG4'))
+    assert(tags_ticket2.include?('some tag4'))
+
+    # merge tags
+    sleep 2
+    Tag::Item.rename(
+      id: tag_item3.id,
+      name: 'some tag2',
+      created_by_id: 1,
+    )
+
+    ticket1_lookup3 = Ticket.lookup(id: ticket1.id)
+    assert_not_equal(ticket1_lookup3.updated_at.to_s, ticket1_lookup2.updated_at.to_s)
+    ticket2_lookup3 = Ticket.lookup(id: ticket2.id)
+    assert_not_equal(ticket2_lookup3.updated_at.to_s, ticket2_lookup2.updated_at.to_s)
+
+    tags_ticket1 = Tag.tag_list(
+      object: 'Ticket',
+      o_id: ticket1.id,
+    )
+    assert_equal(3, tags_ticket1.count)
+    assert(tags_ticket1.include?('some tag2'))
+    assert(tags_ticket1.include?('some TAG4'))
+    assert(tags_ticket1.include?('some tag4'))
+
+    tags_ticket2 = Tag.tag_list(
+      object: 'Ticket',
+      o_id: ticket2.id,
+    )
+    assert_equal(3, tags_ticket2.count)
+    assert(tags_ticket2.include?('some tag2'))
+    assert(tags_ticket2.include?('some TAG4'))
+    assert(tags_ticket2.include?('some tag4'))
+
+    assert_not(Tag::Item.find_by(id: tag_item3.id))
+
+    # remove tag item
+    sleep 2
+    tag_item4 = Tag::Item.find_by(name: 'some TAG4')
+    Tag::Item.remove(tag_item4.id)
+
+    tags_ticket1 = Tag.tag_list(
+      object: 'Ticket',
+      o_id: ticket1.id,
+    )
+    assert_equal(2, tags_ticket1.count)
+    assert(tags_ticket1.include?('some tag2'))
+    assert(tags_ticket1.include?('some tag4'))
+
+    tags_ticket2 = Tag.tag_list(
+      object: 'Ticket',
+      o_id: ticket2.id,
+    )
+    assert_equal(2, tags_ticket2.count)
+    assert(tags_ticket2.include?('some tag2'))
+    assert(tags_ticket2.include?('some tag4'))
+
+    assert_not(Tag::Item.find_by(id: tag_item4.id))
+
+    ticket1_lookup4 = Ticket.lookup(id: ticket1.id)
+    assert_not_equal(ticket1_lookup4.updated_at.to_s, ticket1_lookup3.updated_at.to_s)
+    ticket2_lookup4 = Ticket.lookup(id: ticket2.id)
+    assert_not_equal(ticket2_lookup4.updated_at.to_s, ticket2_lookup3.updated_at.to_s)
+
+  end
 end