Skip to content
This repository

fix another hashtag 'inconvenience' #3298

Merged
merged 1 commit into from almost 2 years ago

4 participants

Florian Staudacher Don't Add Me To Your Organization a.k.a The Travis Bot Steven Hancock Maxwell Salzberg
Florian Staudacher
Collaborator
Raven24 commented May 22, 2012

The change in lib/stream/tag.rb is so that the tag pages show everything again (fixes #3294).

Also I added two rake tasks, that clean up the DB from the upper-cased tags that were created while the acts_as_taggable_on wasn't running the way we wanted...

Please apply your brain juices to that, I have only tested it on a copy of my pods db and I can't say if it works with PgSQL...

Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request passes (merged ef39b4e into 0030788).

Steven Hancock
Collaborator

:thumbsup: Looks good.. works for me on MySQL, but I can't test with PgSQL either.

Maxwell Salzberg
Owner
maxwell commented May 22, 2012

Can we verify what the :ANY option does the resulting SQL query? I think this might have been the reason for everything getting forked in the first place.

Also, does the master version of acts as taggable on include the downcase everything option? I think I saw it somewhere in the docs...

Florian Staudacher
Collaborator
Raven24 commented May 22, 2012

The problem with #3294 is that tags are fetched with where(["lower(name) = ?", name]) which applies to #diaspora as well as #Diaspora.
This means for every tag that might now have a version that is lowercase and mixed-case we get at least two results.

The way the gem handles this is that, when Model.tagged_with(...) is requested, all supplied tags have to match (AND). Only by adding the :any=>true option, the matching is changed to OR and we get the result we want to have. (At least that's what I get from the gem's code... the construction of that query is a little complicated.)
This fixes the problem described by that issue.

The rake tasks are just for all those, who don't want the falsely created mixed-case hashtags in the database (which were created in the time since we updated the gem to upstream and yesterday) and prefer to clean it up.

The 'force_lowercase' option was actually included in my PR yesterday and is the reason the gem has to come from git, because that option has not been packaged in a gem release. (https://github.com/diaspora/diaspora/blob/d815cf5d827119dbe73ec0ffc0448ba0b94eeb05/config/initializers/acts_as_taggable_on.rb)

Florian Staudacher
Collaborator
Raven24 commented May 22, 2012

This is what it does:

ActsAsTaggableOn::Tag.named("diaspora")
  ActsAsTaggableOn::Tag Load (0.7ms)  SELECT `tags`.* FROM `tags` WHERE (lower(name) = 'diaspora')
 => [#<ActsAsTaggableOn::Tag id: 3759, name: "Diaspora">, #<ActsAsTaggableOn::Tag id: 3, name: "diaspora">]

Notice two tags being returned above

StatusMessage.tagged_with("diaspora")
  ActsAsTaggableOn::Tag Load (26.7ms)  SELECT `tags`.* FROM `tags` WHERE (lower(name) = 'diaspora')
  StatusMessage Load (1.3ms)  SELECT `posts`.* FROM `posts` WHERE `posts`.`type` IN ('StatusMessage') AND (1 = 0)
StatusMessage.tagged_with("diaspora", :any => true)
  ActsAsTaggableOn::Tag Load (0.6ms)  SELECT `tags`.* FROM `tags` WHERE (lower(name) = 'diaspora')
  StatusMessage Load (9.4ms)  SELECT DISTINCT posts.* FROM `posts` JOIN taggings posts_taggings_873ac27 ON posts_taggings_873ac27.taggable_id = posts.id AND posts_taggings_873ac27.taggable_type = 'Post' WHERE `posts`.`type` IN ('StatusMessage') AND (posts_taggings_873ac27.tag_id = 3759 OR posts_taggings_873ac27.tag_id = 3)
Florian Staudacher
Collaborator
Raven24 commented May 24, 2012

ping

Maxwell Salzberg
Owner
maxwell commented May 24, 2012

is lower in pg?

Florian Staudacher
Collaborator
Raven24 commented May 24, 2012
Maxwell Salzberg
Owner
maxwell commented May 24, 2012

Cool, can we get a longer-ish commit message with instructions how to make this happen. Then, once i merge, do you want to mail some instructions to the list :) <3

Maxwell Salzberg maxwell commented on the diff May 24, 2012
lib/tasks/migrations.rake
((7 lines not shown))
  80
+  task :rewire_uppercase_hashtags => :environment do
  81
+    evil_tags = ActsAsTaggableOn::Tag.where("lower(name) != name")
  82
+    puts "found #{evil_tags.count} tags to convert..."
  83
+
  84
+    evil_tags.each_with_index do |tag, i|
  85
+      good_tag = ActsAsTaggableOn::Tag.find_or_create_by_name(tag.name.downcase)
  86
+      puts "++ '#{tag.name}' has #{tag.taggings.count} records attached"
  87
+      deleteme = []
  88
+
  89
+      tag.taggings.each do |tagging|
  90
+        deleteme << tagging
  91
+      end
  92
+
  93
+      deleteme.each do |tagging|
  94
+        #tag.taggings.delete(tagging)
  95
+        good_tag.taggings << tagging
1
Maxwell Salzberg Owner
maxwell added a note May 24, 2012

dumb q. does this save the record? I can never remember what does and what doesn't

EDIT : it does

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Florian Staudacher add rake tasks for cleaning up mixed-case hashtags,
fix querying tagged models, in case multiple tags are found
----
the first rake task will attach all posts tagged with mixed-
case hashtags to their lower-case variant

    $ bundle exec rake migrations:rewire_uppercase_hashtags

the other rake task will remove the - now unused - mixed-case
hashtags from the db

    $ bundle exec rake migrations:remove_uppercase_hashtags

as always, perform a backup first! ;)
472340e
Florian Staudacher
Collaborator
Raven24 commented May 24, 2012

is that commit message ok?
it also includes pretty much all the instructions to run the tasks :)

Maxwell Salzberg
Owner
maxwell commented May 24, 2012

thanks, perfect!

Maxwell Salzberg maxwell merged commit 33efa45 into from May 24, 2012
Maxwell Salzberg maxwell closed this May 24, 2012
Don't Add Me To Your Organization a.k.a The Travis Bot

This pull request fails (merged 472340e into bc25ef2).

Florian Staudacher
Collaborator
Raven24 commented May 24, 2012

... didn't know rspec also fails when the elements are all there, but in the wrong order...
-.-

Maxwell Salzberg
Owner
maxwell commented May 24, 2012

try using =~ for array. its a cool matcher :)

Florian Staudacher
Collaborator
Raven24 commented May 24, 2012

just googled, and pushed exactly that ;)

Maxwell Salzberg
Owner
maxwell commented May 24, 2012

word. just ran the rake tasks. no muss, no fuss <3

Florian Staudacher
Collaborator
Raven24 commented May 24, 2012

awesome!
(well, it isn't that complex, but you never know...) ;)

Hans Fase hfase01 referenced this pull request from a commit September 15, 2012
Commit has since been removed from the repository and is no longer available.
Hans Fase hfase01 referenced this pull request from a commit September 25, 2012
Commit has since been removed from the repository and is no longer available.
Alexander Wenzowski wenzowski referenced this pull request from a commit February 28, 2013
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

May 24, 2012
Florian Staudacher add rake tasks for cleaning up mixed-case hashtags,
fix querying tagged models, in case multiple tags are found
----
the first rake task will attach all posts tagged with mixed-
case hashtags to their lower-case variant

    $ bundle exec rake migrations:rewire_uppercase_hashtags

the other rake task will remove the - now unused - mixed-case
hashtags from the db

    $ bundle exec rake migrations:remove_uppercase_hashtags

as always, perform a backup first! ;)
472340e
This page is out of date. Refresh to see the latest.
4  lib/stream/tag.rb
@@ -52,11 +52,11 @@ def publisher_opts
52 52
 
53 53
   def construct_post_query
54 54
     posts = StatusMessage
55  
-    if user.present? 
  55
+    if user.present?
56 56
       posts = posts.owned_or_visible_by_user(user)
57 57
     else
58 58
       posts = posts.all_public
59 59
     end
60  
-    posts.tagged_with(tag_name)
  60
+    posts.tagged_with(tag_name, :any => true)
61 61
   end
62 62
 end
35  lib/tasks/migrations.rake
@@ -74,4 +74,39 @@ namespace :migrations do
74 74
     }
75 75
 
76 76
   end
  77
+
  78
+  # removes hashtags with uppercase letters and re-attaches
  79
+  # the posts to the lowercase version
  80
+  task :rewire_uppercase_hashtags => :environment do
  81
+    evil_tags = ActsAsTaggableOn::Tag.where("lower(name) != name")
  82
+    puts "found #{evil_tags.count} tags to convert..."
  83
+
  84
+    evil_tags.each_with_index do |tag, i|
  85
+      good_tag = ActsAsTaggableOn::Tag.find_or_create_by_name(tag.name.downcase)
  86
+      puts "++ '#{tag.name}' has #{tag.taggings.count} records attached"
  87
+      deleteme = []
  88
+
  89
+      tag.taggings.each do |tagging|
  90
+        deleteme << tagging
  91
+      end
  92
+
  93
+      deleteme.each do |tagging|
  94
+        #tag.taggings.delete(tagging)
  95
+        good_tag.taggings << tagging
  96
+      end
  97
+
  98
+      puts "-- converted '#{tag.name}' to '#{good_tag.name}' with #{deleteme.count} records"
  99
+      puts "\n## #{i} tags processed\n\n" if (i % 50 == 0)
  100
+    end
  101
+  end
  102
+
  103
+  task :remove_uppercase_hashtags => :environment do
  104
+    evil_tags = ActsAsTaggableOn::Tag.where("lower(name) != name")
  105
+    evil_tags.each do |tag|
  106
+      next if tag.taggings.count > 0 # non-ascii tags
  107
+
  108
+      puts "removing '#{tag.name}'..."
  109
+      tag.destroy
  110
+    end
  111
+  end
77 112
 end
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.