Commit e2d59e403c162938e1543f8b87a226cf16f4ca95
Exists in
master
and in
1 other branch
Merge branch 'ruby-1.9.3-upgrade'
Showing
6 changed files
with
113 additions
and
9 deletions
Show diff stats
app/controllers/choices_controller.rb
@@ -45,7 +45,7 @@ class ChoicesController < InheritedResources::Base | @@ -45,7 +45,7 @@ class ChoicesController < InheritedResources::Base | ||
45 | 45 | ||
46 | visitor = current_user.default_visitor | 46 | visitor = current_user.default_visitor |
47 | if visitor_identifier | 47 | if visitor_identifier |
48 | - visitor = Visitor.find_or_create_by_identifier_and_site_id(visitor_identifier, current_user.id) | 48 | + visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) |
49 | end | 49 | end |
50 | params[:choice].merge!(:creator => visitor) | 50 | params[:choice].merge!(:creator => visitor) |
51 | 51 | ||
@@ -68,7 +68,7 @@ class ChoicesController < InheritedResources::Base | @@ -68,7 +68,7 @@ class ChoicesController < InheritedResources::Base | ||
68 | 68 | ||
69 | end | 69 | end |
70 | if visitor_identifier = params[:visitor_identifier] | 70 | if visitor_identifier = params[:visitor_identifier] |
71 | - visitor = Visitor.find_or_create_by_identifier_and_site_id(visitor_identifier, current_user.id) | 71 | + visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) |
72 | flag_params.merge!({:visitor_id => visitor.id}) | 72 | flag_params.merge!({:visitor_id => visitor.id}) |
73 | end | 73 | end |
74 | respond_to do |format| | 74 | respond_to do |format| |
app/models/question.rb
@@ -183,7 +183,7 @@ class Question < ActiveRecord::Base | @@ -183,7 +183,7 @@ class Question < ActiveRecord::Base | ||
183 | if params[:with_prompt] | 183 | if params[:with_prompt] |
184 | 184 | ||
185 | if params[:with_appearance] && visitor_identifier.present? | 185 | if params[:with_appearance] && visitor_identifier.present? |
186 | - visitor = Visitor.find_or_create_by_identifier_and_site_id(visitor_identifier, current_user.id) | 186 | + visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) |
187 | 187 | ||
188 | last_appearance = get_first_unanswered_appearance(visitor) | 188 | last_appearance = get_first_unanswered_appearance(visitor) |
189 | 189 | ||
@@ -237,7 +237,7 @@ class Question < ActiveRecord::Base | @@ -237,7 +237,7 @@ class Question < ActiveRecord::Base | ||
237 | end | 237 | end |
238 | 238 | ||
239 | if params[:with_visitor_stats] | 239 | if params[:with_visitor_stats] |
240 | - visitor = Visitor.find_or_create_by_identifier_and_site_id(visitor_identifier, current_user.id) | 240 | + visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) |
241 | result.merge!(:visitor_votes => Vote.find_without_default_scope(:all, :conditions => {:voter_id => visitor, :question_id => self.id}).length) | 241 | result.merge!(:visitor_votes => Vote.find_without_default_scope(:all, :conditions => {:voter_id => visitor, :question_id => self.id}).length) |
242 | result.merge!(:visitor_ideas => visitor.choices.count) | 242 | result.merge!(:visitor_ideas => visitor.choices.count) |
243 | end | 243 | end |
app/models/user.rb
@@ -10,12 +10,12 @@ class User < ActiveRecord::Base | @@ -10,12 +10,12 @@ class User < ActiveRecord::Base | ||
10 | 10 | ||
11 | def create_question(visitor_identifier, question_params) | 11 | def create_question(visitor_identifier, question_params) |
12 | logger.info "the question_params are #{question_params.inspect}" | 12 | logger.info "the question_params are #{question_params.inspect}" |
13 | - visitor = Visitor.find_or_create_by_identifier_and_site_id(visitor_identifier, self.id) | 13 | + visitor = visitors.find_or_create_by_identifier(visitor_identifier) |
14 | question = visitor.questions.create(question_params.merge(:site => self)) | 14 | question = visitor.questions.create(question_params.merge(:site => self)) |
15 | end | 15 | end |
16 | 16 | ||
17 | def create_choice(visitor_identifier, question, choice_params = {}) | 17 | def create_choice(visitor_identifier, question, choice_params = {}) |
18 | - visitor = Visitor.find_or_create_by_identifier_and_site_id(visitor_identifier, self.id) | 18 | + visitor = visitors.find_or_create_by_identifier(visitor_identifier) |
19 | raise "Question not found" if question.nil? | 19 | raise "Question not found" if question.nil? |
20 | 20 | ||
21 | #TODO Does this serve a purpose? | 21 | #TODO Does this serve a purpose? |
@@ -35,7 +35,7 @@ class User < ActiveRecord::Base | @@ -35,7 +35,7 @@ class User < ActiveRecord::Base | ||
35 | if visitor_identifier.nil? | 35 | if visitor_identifier.nil? |
36 | visitor = default_visitor | 36 | visitor = default_visitor |
37 | else | 37 | else |
38 | - visitor = Visitor.find_or_create_by_identifier_and_site_id(visitor_identifier, self.id) | 38 | + visitor = visitors.find_or_create_by_identifier(visitor_identifier) |
39 | end | 39 | end |
40 | visitor.vote_for!(options) | 40 | visitor.vote_for!(options) |
41 | end | 41 | end |
@@ -50,7 +50,7 @@ class User < ActiveRecord::Base | @@ -50,7 +50,7 @@ class User < ActiveRecord::Base | ||
50 | if visitor_identifier.nil? | 50 | if visitor_identifier.nil? |
51 | visitor = default_visitor | 51 | visitor = default_visitor |
52 | else | 52 | else |
53 | - visitor = Visitor.find_or_create_by_identifier_and_site_id(visitor_identifier, self.id) | 53 | + visitor = visitors.find_or_create_by_identifier(visitor_identifier) |
54 | end | 54 | end |
55 | visitor.skip!(options) | 55 | visitor.skip!(options) |
56 | end | 56 | end |
@@ -0,0 +1,89 @@ | @@ -0,0 +1,89 @@ | ||
1 | +unless RUBY_VERSION == '1.8.7' | ||
2 | + # Make sure the logger supports encodings properly. | ||
3 | + module ActiveSupport | ||
4 | + class BufferedLogger | ||
5 | + def add(severity, message = nil, progname = nil, &block) | ||
6 | + return if @level > severity | ||
7 | + message = (message || (block && block.call) || progname).to_s | ||
8 | + | ||
9 | + # If a newline is necessary then create a new message ending with a newline. | ||
10 | + # Ensures that the original message is not mutated. | ||
11 | + message = "#{message}\n" unless message[-1] == ?\n | ||
12 | + buffer << message.force_encoding(Encoding.default_external) | ||
13 | + auto_flush | ||
14 | + message | ||
15 | + end | ||
16 | + end | ||
17 | + end | ||
18 | + | ||
19 | + # This makes it so all parameters get converted to UTF-8 before they hit your app. If someone sends invalid UTF-8 to your server, raise an exception. | ||
20 | + # At UserVoice, we rescue this exception and show a custom error page. | ||
21 | + class ActionController::InvalidByteSequenceErrorFromParams < Encoding::InvalidByteSequenceError; end | ||
22 | + class ActionController::Base | ||
23 | + | ||
24 | + def force_utf8_params | ||
25 | + traverse = lambda do |object, block| | ||
26 | + if object.kind_of?(Hash) | ||
27 | + object.each_value { |o| traverse.call(o, block) } | ||
28 | + elsif object.kind_of?(Array) | ||
29 | + object.each { |o| traverse.call(o, block) } | ||
30 | + else | ||
31 | + block.call(object) | ||
32 | + end | ||
33 | + object | ||
34 | + end | ||
35 | + force_encoding = lambda do |o| | ||
36 | + if o.respond_to?(:force_encoding) | ||
37 | + o.force_encoding(Encoding::UTF_8) | ||
38 | + raise ActionController::InvalidByteSequenceErrorFromParams unless o.valid_encoding? | ||
39 | + end | ||
40 | + if o.respond_to?(:original_filename) | ||
41 | + o.original_filename.force_encoding(Encoding::UTF_8) | ||
42 | + raise ActionController::InvalidByteSequenceErrorFromParams unless o.original_filename.valid_encoding? | ||
43 | + end | ||
44 | + end | ||
45 | + traverse.call(params, force_encoding) | ||
46 | + path_str = request.path.to_s | ||
47 | + if path_str.respond_to?(:force_encoding) | ||
48 | + path_str.force_encoding(Encoding::UTF_8) | ||
49 | + raise ActionController::InvalidByteSequenceErrorFromParams unless path_str.valid_encoding? | ||
50 | + end | ||
51 | + end | ||
52 | + before_filter :force_utf8_params | ||
53 | + end | ||
54 | + | ||
55 | + | ||
56 | + # Serialized columns in AR don't support UTF-8 well, so set the encoding on those as well. | ||
57 | + class ActiveRecord::Base | ||
58 | + def unserialize_attribute_with_utf8(attr_name) | ||
59 | + traverse = lambda do |object, block| | ||
60 | + if object.kind_of?(Hash) | ||
61 | + object.each_value { |o| traverse.call(o, block) } | ||
62 | + elsif object.kind_of?(Array) | ||
63 | + object.each { |o| traverse.call(o, block) } | ||
64 | + else | ||
65 | + block.call(object) | ||
66 | + end | ||
67 | + object | ||
68 | + end | ||
69 | + force_encoding = lambda do |o| | ||
70 | + o.force_encoding(Encoding::UTF_8) if o.respond_to?(:force_encoding) | ||
71 | + end | ||
72 | + value = unserialize_attribute_without_utf8(attr_name) | ||
73 | + traverse.call(value, force_encoding) | ||
74 | + end | ||
75 | + alias_method_chain :unserialize_attribute, :utf8 | ||
76 | + end | ||
77 | + | ||
78 | + # Make sure the flash sets the encoding to UTF-8 as well. | ||
79 | + module ActionController | ||
80 | + module Flash | ||
81 | + class FlashHash | ||
82 | + def [](k) | ||
83 | + v = super | ||
84 | + v.is_a?(String) ? v.force_encoding("UTF-8") : v | ||
85 | + end | ||
86 | + end | ||
87 | + end | ||
88 | + end | ||
89 | +end |
@@ -0,0 +1,13 @@ | @@ -0,0 +1,13 @@ | ||
1 | +# Finally, we have this innocuous looking patch. Without it, queries like this: current_account.tickets.recent.count | ||
2 | +# would instantiate AR objects all (!!) tickets in the account, not merely return a count of the recent ones. | ||
3 | +# See https://rails.lighthouseapp.com/projects/8994/tickets/5410-multiple-database-queries-when-chaining-named-scopes-with-rails-238-and-ruby-192 | ||
4 | +# (The patch in that lighthouse bug was not, in fact, merged in). | ||
5 | +module ActiveRecord | ||
6 | + module Associations | ||
7 | + class AssociationProxy | ||
8 | + def respond_to_missing?(meth, incl_priv) | ||
9 | + false | ||
10 | + end | ||
11 | + end | ||
12 | + end | ||
13 | +end |
spec/controllers/choices_controller_spec.rb
@@ -69,7 +69,9 @@ describe ChoicesController do | @@ -69,7 +69,9 @@ describe ChoicesController do | ||
69 | 69 | ||
70 | it "adds visitor_id params to flag if sent" do | 70 | it "adds visitor_id params to flag if sent" do |
71 | @visitor_identifier = "somelongunique32charstring" | 71 | @visitor_identifier = "somelongunique32charstring" |
72 | - Visitor.should_receive(:find_or_create_by_identifier_and_site_id).with(@visitor_identifier, @user.id).and_return(mock_visitor) | 72 | + visitor_list = [mock_visitor] |
73 | + @user.stub!(:visitors).and_return(visitor_list) | ||
74 | + visitor_list.should_receive(:find_or_create_by_identifier).with(@visitor_identifier).and_return(mock_visitor) | ||
73 | 75 | ||
74 | Flag.should_receive(:create!).with({:choice_id => 123, :question_id => 37, :site_id => @user.id, :explanation => "This is offensive", :visitor_id => mock_visitor.id}) | 76 | Flag.should_receive(:create!).with({:choice_id => 123, :question_id => 37, :site_id => @user.id, :explanation => "This is offensive", :visitor_id => mock_visitor.id}) |
75 | 77 |