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

This commit is contained in:
Jyri-Petteri Paloposki 2019-06-25 03:15:23 +03:00
parent 5ec2c77f78
commit 5394973346
9 changed files with 97 additions and 24 deletions

View file

@ -106,7 +106,7 @@ class TodosController < ApplicationController
if @todo.errors.empty? if @todo.errors.empty?
@todo.add_predecessor_list(p.predecessor_list) @todo.add_predecessor_list(p.predecessor_list)
@saved = @todo.save @saved = @todo.save
@todo.tag_with(tag_list) if @saved && tag_list.present? @todo.tag_with(tag_list, current_user) if @saved && tag_list.present?
@todo.block! if @todo.uncompleted_predecessors? @todo.block! if @todo.uncompleted_predecessors?
else else
@saved = false @saved = false
@ -194,7 +194,7 @@ class TodosController < ApplicationController
todo.block! todo.block!
end end
todo.tag_with(tag_list) if @saved && tag_list.present? todo.tag_with(tag_list, current_user) if @saved && tag_list.present?
@todos << todo @todos << todo
@not_done_todos << todo if p.new_context_created || p.new_project_created @not_done_todos << todo if p.new_context_created || p.new_project_created
@ -712,9 +712,8 @@ class TodosController < ApplicationController
def tags 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 tags_all = tags_all - tags_beginning
respond_to do |format| respond_to do |format|
@ -1219,7 +1218,7 @@ end
def update_tags def update_tags
if params[:tag_list] if params[:tag_list]
@todo.tag_with(params[:tag_list]) @todo.tag_with(params[:tag_list], current_user)
@todo.tags.reload #force a reload for proper rendering @todo.tags.reload #force a reload for proper rendering
end end
end end

View file

@ -129,7 +129,7 @@ module RecurringTodos
end end
def save_tags def save_tags
@recurring_todo.tag_with(@filterred_attributes[:tag_list]) if @filterred_attributes[:tag_list].present? @recurring_todo.tag_with(@filterred_attributes[:tag_list], @user) if @filterred_attributes[:tag_list].present?
@recurring_todo.reload @recurring_todo.reload
end end

View file

@ -3,13 +3,15 @@ class Tag < ApplicationRecord
has_many :taggings has_many :taggings
has_many :taggable, :through => :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. 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 JOIN_DELIMITER = ", ".freeze
# If database speed becomes an issue, you could remove these validations and # If database speed becomes an issue, you could remove these validations and
# rescue the ActiveRecord database constraint errors instead. # rescue the ActiveRecord database constraint errors instead.
validates_presence_of :name 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 before_create :before_create

View file

@ -349,7 +349,7 @@ class Todo < ApplicationRecord
def add_tags=(params) def add_tags=(params)
unless params[:tag].nil? unless params[:tag].nil?
tag_list = params[:tag].inject([]) { |list, value| list << value[:name] } tag_list = params[:tag].inject([]) { |list, value| list << value[:name] }
tag_with tag_list.join(", ") tag_with tag_list.join(", "), user
end end
end end

View file

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

View file

@ -50,7 +50,7 @@ class TodoFromRichMessage
todo.project_id = project_id unless project_id.nil? todo.project_id = project_id unless project_id.nil?
todo.show_from = show_from if show_from.is_a? Time todo.show_from = show_from if show_from.is_a? Time
todo.due = due if due.is_a? Time todo.due = due if due.is_a? Time
todo.tag_with tags unless tags.nil? || tags.empty? todo.tag_with tags, @user unless tags.nil? || tags.empty?
todo.starred = star todo.starred = star
todo todo
end end

View file

@ -0,0 +1,72 @@
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.
connection.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}
EOQ
# Move all the user's recurring todos to the new tag.
connection.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}
EOQ
end
tag.user_id = uids[0]
tag.save!
end
end
def self.down
remove_column :tags, :user_id
end
end

View file

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

View file

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