Cannot save user_ids to an instance of Spree::Promotion::Rules::User #3885

Closed
ckmorris opened this Issue Oct 17, 2013 · 7 comments

Comments

Projects
None yet
3 participants
@ckmorris

I want to create a list of Users that a promotion applies to. So, I setup a Promotion that requires specific Users in the Admin Tool.

However, I cannot directly pass user_ids or call user_ids_string setter on Spree::Promotion::Rules::User because the association is not associated with the Spree::User.table_name.

This causes it to be impossible to add Users to a specific Spree::Promotion::Rules::User list instance unless the user_id also happens to equal the Spree::Promotion::Rules::User.id resulting in a "Promotion not found" error in the promotions admin tool.

Lets say I instantiate a rule list

rule_list = Spree::Promotion::Rules::User.new
=> #<Spree::Promotion::Rules::User id: nil, activator_id: nil, user_id: nil, product_group_id: nil, type: "Spree::Promotion::Rules::User", created_at: nil, updated_at: nil>

Then populate it out

=> #<Spree::Promotion::Rules::User id: 7, activator_id: 3, user_id: nil, product_group_id: nil, type: "Spree::Promotion::Rules::User", created_at: "2013-10-17 19:41:41", updated_at: "2013-10-17 19:41:41">

This rule list has some methods that can be found under spree_core/app/models/spree/promotion/rules/
so in user.rb of that path, there is a getter and a setter

spree_core/app/models/spree/promotion/rules/user.rb
def user_ids_string
  user_ids.join(', ')
end

def user_ids_string=(s)
  self.user_ids = s.to_s.split(', ').map(&:strip)
end

That's pretty straight forward...

So if I do:

rule_list.user_ids_string = "1, 3"

It should look up Spree::User.id: 1 and Spree::User.id: 3 and upon verifying their existence, add them to the string that the setter uses to populate Spree::Promotion::Rules::User.user_ids,

but when i do it:

Spree::Promotion::Rules::User Load (0.8ms)  SELECT "spree_promotion_rules".* FROM "spree_promotion_rules" WHERE "spree_promotion_rules"."type" IN ('Spree::Promotion::Rules::User') AND "spree_promotion_rules"."id" IN (1, 3)
ActiveRecord::RecordNotFound: Couldn't find all Spree::Promotion::Rules::Users with IDs (1, 3) [WHERE "spree_promotion_rules"."type" IN ('Spree::Promotion::Rules::User')] (found 0 results, but was looking for 2)

But, if I specify the id of the rule_list.id itself....

2.0.0p247 :045 > rule_list.user_ids_string = rule_list.id
  Spree::Promotion::Rules::User Load (0.6ms)  SELECT "spree_promotion_rules".* FROM "spree_promotion_rules" WHERE "spree_promotion_rules"."type" IN ('Spree::Promotion::Rules::User') AND "spree_promotion_rules"."id" = $1 LIMIT 1  [["id", 7]]
   (0.2ms)  BEGIN
   (0.2ms)  COMMIT
 => 7

So for some reason it thinks that the user_ids should equal only the Spree::Promotion::Rules::User.ids
when it should instead have an association to equal only the Spree::User.ids

So to prove to myself that this is in fact what is happening I instead try to directly set user_ids of the rule_list using 2 verified user_ids: 1, 3 and the Spree::Promotion::Rules::User.id itself, 7.

2.0.0p247 :052 > rule_list.user_ids = Spree::User.find(1).id, Spree::User.find(3).id, rule_list.id
  Spree::User Load (1.0ms)  SELECT "spree_users".* FROM "spree_users" WHERE "spree_users"."id" = $1 LIMIT 1  [["id", 1]]
  Spree::User Load (1.1ms)  SELECT "spree_users".* FROM "spree_users" WHERE "spree_users"."id" = $1 LIMIT 1  [["id", 3]]
  Spree::Promotion::Rules::User Load (0.6ms)  SELECT "spree_promotion_rules".* FROM "spree_promotion_rules" WHERE "spree_promotion_rules"."type" IN ('Spree::Promotion::Rules::User') AND "spree_promotion_rules"."id" IN (1, 3, 7)
ActiveRecord::RecordNotFound: Couldn't find all Spree::Promotion::Rules::Users with IDs (1, 3, 7) [WHERE "spree_promotion_rules"."type" IN ('Spree::Promotion::Rules::User')] (found 1 results, but was looking for 3)

So it does find the 7, but not the 1 and 3. Something about the Spree::Promotion::Rules::User.user_ids attribute erroneously expects the table it wants to be "spree_promotion_rules" instead of "spree_promotion_rules_users".

Spree Version: 2.1.1
Rails Version: 4.0.0
Ruby Version: 2.0.0-p247

Gemfile:

source 'https://rubygems.org'
ruby '2.0.0'
gem 'rails', '4.0.0'
gem 'pg'
gem 'sass-rails', '~> 4.0.0'
gem 'uglifier', '>= 1.3.0'
gem 'coffee-rails', '~> 4.0.0'
gem 'jquery-rails'
gem 'turbolinks'
gem 'jbuilder', '~> 1.2'

group :doc do
  gem 'sdoc', require: false
end

gem 'spree', '2.1.1'
gem 'spree_gateway', :git => 'https://github.com/spree/spree_gateway.git', :branch => '2-1-stable'
gem 'spree_auth_devise', :git => 'https://github.com/spree/spree_auth_devise.git', :branch => '2-1-stable'
@radar

This comment has been minimized.

Show comment Hide comment
@radar

radar Oct 17, 2013

Member

@ckmorris please provide the information we ask for in the Contributing Guidelines.

Thanks!

Member

radar commented Oct 17, 2013

@ckmorris please provide the information we ask for in the Contributing Guidelines.

Thanks!

@ckmorris

This comment has been minimized.

Show comment Hide comment
@ckmorris

ckmorris Oct 17, 2013

I tried to provide a lot more information. Please let me know if I need to provide even more.

I tried to provide a lot more information. Please let me know if I need to provide even more.

@radar

This comment has been minimized.

Show comment Hide comment
@radar

radar Oct 17, 2013

Member

Thanks for providing the information, @ckmorris. I will probably have time to look further at this tomorrow.

Member

radar commented Oct 17, 2013

Thanks for providing the information, @ckmorris. I will probably have time to look further at this tomorrow.

@ckmorris

This comment has been minimized.

Show comment Hide comment
@ckmorris

ckmorris Oct 17, 2013

Thanks Ryan :)

Thanks Ryan :)

@radar

This comment has been minimized.

Show comment Hide comment
@radar

radar Oct 23, 2013

Member

I took a look at this today and it's actually quite difficult to solve. It's been about an hour now and I don't have a solid solution to the problem. However: I do know what's causing it, so maybe that can be of some help to a future investigator.

The Promotion::Rules::User class is being loaded before Spree.user_class is being set, therefore it's always falling to the else statement in model which does not define any class name for the two associations. One solution I have considered is moving the concatenation out of where it currently sits in core/engine.rb and moving that into spree_auth_devise... but then that would mean that anyone who was using that promotion rule would also need to have that code to add that rule in too...

Maybe it's a worthwhile tradeoff, or maybe not. Right now, I am conflicted about the decision on this one.

Member

radar commented Oct 23, 2013

I took a look at this today and it's actually quite difficult to solve. It's been about an hour now and I don't have a solid solution to the problem. However: I do know what's causing it, so maybe that can be of some help to a future investigator.

The Promotion::Rules::User class is being loaded before Spree.user_class is being set, therefore it's always falling to the else statement in model which does not define any class name for the two associations. One solution I have considered is moving the concatenation out of where it currently sits in core/engine.rb and moving that into spree_auth_devise... but then that would mean that anyone who was using that promotion rule would also need to have that code to add that rule in too...

Maybe it's a worthwhile tradeoff, or maybe not. Right now, I am conflicted about the decision on this one.

@radar

This comment has been minimized.

Show comment Hide comment
@radar

radar Jan 20, 2014

Member

I believe this was fixed by #4118.

Member

radar commented Jan 20, 2014

I believe this was fixed by #4118.

@radar radar closed this Jan 20, 2014

@maxstudener

This comment has been minimized.

Show comment Hide comment
@maxstudener

maxstudener Jan 21, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment