Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove user_id from PromotionRules #1259

Merged

Conversation

jhawthorn
Copy link
Contributor

Both the belongs_to and the column have been around since promo was merged into spree_core, but I don't think they have ever been used.

@jasonfb
Copy link

jasonfb commented Jun 17, 2016

I don't think we are using this. I checked our DB and there are no values in the user_id column, and there existing (non-scalable) "By User" rule should be using the join table spree_promotion_rules_users, so I this seems 👍 to me

@gmacdougall
Copy link
Member

Should we consider deprecating this instead and removing the column in the next major version?

@jrochkind
Copy link
Contributor

jrochkind commented Jun 17, 2016

I think it's probably just okay to remove, I can't think of any use case for it (when I saw it there myself a month or so ago, I couldn't figure it out either, heh). In what cases would a promotion be associated with exactly one and only one user? A single-user-specific promotion seems unlikely.

If you want to be extra safe, you could have the migration that removes it written to make sure the column is nil in the database, and refrain from removing it if it has any values, emitting some scary warning that you might want to look into it and remove it yourself.

@mamhoff
Copy link
Contributor

mamhoff commented Jun 22, 2016

👍

jhawthorn and others added 3 commits June 29, 2016 11:42
Both the belongs_to and the column have been around since promo was
merged into spree_core, but I don't think they have ever been used.
@jhawthorn jhawthorn force-pushed the remove_user_id_from_promotion_rules branch from 66cea7c to 47dbdb1 Compare June 29, 2016 18:50
@jhawthorn
Copy link
Contributor Author

Rebased and added a note to CHANGELOG

@jhawthorn jhawthorn merged commit 6492e51 into solidusio:master Jun 29, 2016
@jhawthorn jhawthorn deleted the remove_user_id_from_promotion_rules branch September 30, 2016 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants