Merge pull request #2255 from ZeiP/feature/#1955_user_tags

#1955: Migrate tags to belong to named users for enhanced privacy.

app/controllers/todos_controller.rb

@@ -712,9 +712,8 @@ class TodosController < ApplicationController
 
 
   def tags
-    # TODO: limit to current_user
+    tags_beginning = current_user.tags.where(Tag.arel_table[:name].matches("#{params[:term]}%"))
-    tags_beginning = Tag.where(Tag.arel_table[:name].matches("#{params[:term]}%"))
+    tags_all = current_user.tags.where(Tag.arel_table[:name].matches("%#{params[:term]}%"))
-    tags_all = Tag.where(Tag.arel_table[:name].matches("%#{params[:term]}%"))
     tags_all = tags_all - tags_beginning
 
     respond_to do |format|

app/models/tag.rb

@@ -3,13 +3,15 @@ class Tag < ApplicationRecord
   has_many :taggings
   has_many :taggable, :through => :taggings
 
+  belongs_to :user
+
   DELIMITER = ",".freeze # Controls how to split and join tagnames from strings. You may need to change the <tt>validates_format_of parameters</tt> if you change this.
   JOIN_DELIMITER = ", ".freeze
 
   # If database speed becomes an issue, you could remove these validations and
   # rescue the ActiveRecord database constraint errors instead.
   validates_presence_of :name
-  validates_uniqueness_of :name, :case_sensitive => false
+  validates_uniqueness_of :name, :scope => "user_id", :case_sensitive => false
 
   before_create :before_create
 

app/models/user.rb

@@ -94,6 +94,7 @@ class User < ApplicationRecord
               end
            end
 
+  has_many :tags, dependent: :delete_all
   has_many :notes, -> { order "created_at DESC" }, dependent: :delete_all
   has_one :preference, dependent: :destroy
   has_many :attachments, through: :todos

db/migrate/20190618202817_add_user_id_to_tag.rb

@@ -0,0 +1,88 @@
+class AddUserIdToTag < ActiveRecord::Migration[5.2]
+  def self.up
+    add_column :tags, :user_id, :integer
+
+    # Find uses of each tag for both Todos and RecurringTodos to
+    # figure out which users use which tags.
+    @tags = execute <<-EOQ
+      SELECT t.id AS tid, tds.user_id AS todo_uid, rt.user_id AS rtodo_uid
+      FROM tags t
+      JOIN taggings tgs ON tgs.tag_id = t.id
+      LEFT OUTER JOIN todos tds
+       ON tgs.taggable_type = "Todo" AND tds.id = tgs.taggable_id
+      LEFT OUTER JOIN recurring_todos rt
+       ON tgs.taggable_type = "RecurringTodo" AND rt.id = tgs.taggable_id
+      WHERE rt.id IS NOT NULL OR tds.id IS NOT NULL
+      GROUP BY t.id, tds.user_id, rt.user_id
+    EOQ
+
+    # Map each tag to the users using it.
+    @tag_users = {}
+    @tags.each do |row|
+      uid = (row[1] ? row[1] : row[2])
+      if not @tag_users[row[0]]
+        @tag_users[row[0]] = [uid]
+      elsif not @tag_users[row[0]].include? uid
+        @tag_users[row[0]] << uid
+      end
+    end
+
+    # Go through the tags assigning users and duplicating as necessary.
+    @tag_users.each do |tid, uids|
+      tag = Tag.find(tid)
+
+      # One of the users will get the original tag instance, but first
+      # duplicate their own copy to all the others.
+      extras = uids.length - 1
+      extras.times do |n|
+        uid = uids[n+1]
+
+        # Create a duplicate of the tag assigned to the user.
+        new_tag = tag.dup
+        new_tag.user_id = uid
+        new_tag.save!
+
+        # Move all the user's regular todos to the new tag.
+        execute <<-EOQ
+          UPDATE taggings ta
+          JOIN todos t
+           ON ta.taggable_type = "Todo" AND t.id = ta.taggable_id
+          SET ta.tag_id = #{new_tag.id}
+          WHERE t.user_id = #{uid} AND ta.tag_id = #{tid}
+        EOQ
+
+        # Move all the user's recurring todos to the new tag.
+        execute <<-EOQ
+          UPDATE taggings ta
+          JOIN recurring_todos t
+           ON ta.taggable_type = "RecurringTodo" AND t.id = ta.taggable_id
+          SET ta.tag_id = #{new_tag.id}
+          WHERE t.user_id = #{uid} AND ta.tag_id = #{tid}
+        EOQ
+      end
+
+      tag.user_id = uids[0]
+      tag.save!
+    end
+
+    # Set all unowned tags to the only user, if there's only one. Otherwise
+    # remove them since there's no way of knowing who they belong to.
+    if User.all.count == 1
+      uid = User.first.id
+      execute <<-EOQ
+        UPDATE tags
+        SET user_id = #{uid}
+        WHERE user_id IS NULL
+      EOQ
+    else
+      execute <<-EOQ
+        DELETE FROM tags
+        WHERE user_id IS NULL
+      EOQ
+    end
+  end
+  def self.down
+    remove_column :tags, :user_id
+  end
+end
+

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2016_01_31_233303) do
+ActiveRecord::Schema.define(version: 2019_06_18_202817) do
 
   create_table "attachments", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8", force: :cascade do |t|
     t.integer "todo_id"
@@ -159,6 +159,7 @@ ActiveRecord::Schema.define(version: 2016_01_31_233303) do
     t.string "name"
     t.datetime "created_at"
     t.datetime "updated_at"
+    t.integer "user_id"
     t.index ["name"], name: "index_tags_on_name"
   end
 

doc/CHANGELOG.md

@@ -1,13 +1,27 @@
 ## Version 2.4
+### New features
 * Removed support for deprecated password-hashing algorithm. This
   eliminates config.salt. Note the addition of a pre-upgrade step to
   check for obsolete passwords.
+* All tags now belong to a user. Existing tags are migrated to users based on
+  the taggings and duplicated as necessary. If there's only one user, all unused tags are
+  assigned to them, otherwise unused tags are removed.
+* All REST APIs now also accept user token as password.
+* The stats view now uses Charts.js instead of the Flash-based chart library.
+* A Docker environment is used unless the .skip-docker file exists.
 * Rails 4.2
 * Thin replaces WEBrick as the included web server
 * Tracks is tested on Ruby 1.9.3, 2.0.0, 2.1, and 2.2.
 * The MessageGateway will save the received email as an attachement to the todo
 * Add a configuration option for serving static assets from Rails
 
+### Removed features
+* Ruby versions below 2.4 are no longer supported.
+
+### Bug fixes
+* Multiple fixes to REST APIs.
+* Several UI bugs fixed.
+
 ## Version 2.3.0
 
 ### New and changed features

lib/is_taggable.rb

@@ -15,43 +15,43 @@ module IsTaggable
           self.to_a.reject{|tag| tag.name == Todo::STARRED_TAG_NAME}
         end
       end
-      
+
       def tag_list
         tags.reload
         tags.to_s
       end
-            
+
       def tag_list=(value)
         tag_with(value)
       end
-      
+
       # Replace the existing tags on <tt>self</tt>. Accepts a string of tagnames, an array of tagnames, or an array of Tags.
-      def tag_with list
+      def tag_with(list)
         list = tag_cast_to_string(list)
-  
+
         # Transactions may not be ideal for you here; be aware.
         Tag.transaction do
           current = tags.to_a.map(&:name)
           _add_tags(list - current)
           _remove_tags(current - list)
         end
-  
+
         self
       end
 
       def has_tag?(tag_name)
         return tags.any? {|tag| tag.name == tag_name}
       end
-      
+
       # Add tags to <tt>self</tt>. Accepts a string of tagnames, an array of tagnames, or an array of Tags.
       #
       # We need to avoid name conflicts with the built-in ActiveRecord association methods, thus the underscores.
-      def _add_tags incoming
+      def _add_tags(incoming)
         tag_cast_to_string(incoming).each do |tag_name|
           # added following check to prevent empty tags from being saved (which will fail)
           if tag_name.present?
             begin
-              tag = Tag.where(:name => tag_name).first_or_create
+              tag = self.user.tags.where(:name => tag_name).first_or_create
               raise Tag::Error, "tag could not be saved: #{tag_name}" if tag.new_record?
               tags << tag
             rescue ActiveRecord::StatementInvalid => e
@@ -62,11 +62,11 @@ module IsTaggable
       end
 
       # Removes tags from <tt>self</tt>. Accepts a string of tagnames, an array of tagnames, or an array of Tags.
-      def _remove_tags outgoing
+      def _remove_tags(outgoing)
         outgoing = tag_cast_to_string(outgoing)
-        tags.destroy(*(tags.select{|tag| outgoing.include? tag.name}))
+        tags.destroy(*(self.user.tags.select{|tag| outgoing.include? tag.name}))
       end
-      
+
       def get_tag_name_from_item(item)
         case item
         # removed next line as it prevents using numbers as tags
@@ -94,8 +94,6 @@ module IsTaggable
           raise "Invalid object of class #{obj.class} as tagging method parameter"
         end
       end
-            
     end
   end
-  
 end

test/fixtures/tags.yml

@@ -10,18 +10,21 @@ end
 foo:
   id: 1
   name: foo
+  user_id: 1
   created_at: <%= today %>
   updated_at: <%= today %>
 
 bar:
   id: 2
   name: bar
+  user_id: 1
   created_at: <%= today %>
   updated_at: <%= today %>
 
 baz:
   id: 3
   name: baz
+  user_id: 1
   created_at: <%= today %>
   updated_at: <%= today %>
-  
+

test/models/tag_test.rb

@@ -25,20 +25,20 @@ class TagTest < ActiveSupport::TestCase
     tag = Tag.where(:name => "8.1.2").first_or_create
     assert !tag.new_record?
   end
-  
+
   def test_tag_name_always_lowercase
     tag = Tag.where(:name => "UPPER").first_or_create
     assert !tag.new_record?
-    
+
     upper = Tag.where(:name => "upper").first
     assert_not_nil upper
     assert upper.name == "upper"
   end
-  
+
   def test_tag_name_stripped_of_spaces
     tag = Tag.where(:name => "  strip spaces  ").first_or_create
     assert !tag.new_record?
-    
+
     assert tag.name == "strip spaces"
   end