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

Assign to configured slug column, not "slug", when validation fails #779

Merged
merged 1 commit into from
Jan 25, 2017

Conversation

cgunther
Copy link
Contributor

@cgunther cgunther commented Dec 2, 2016

1f8af74 mistakenly assigns to the literal slug method, rather than the configured slug_column. As a result, models using a custom slug_column get an error raised when validations fail that slug= is an undefined method.

Fixes #765.

@coveralls
Copy link

coveralls commented Dec 2, 2016

Coverage Status

Coverage remained the same at 96.734% when pulling 8cf0e7e on cgunther:fix-issue-765 into 920af01 on norman:master.

5 similar comments
@coveralls
Copy link

coveralls commented Dec 2, 2016

Coverage Status

Coverage remained the same at 96.734% when pulling 8cf0e7e on cgunther:fix-issue-765 into 920af01 on norman:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.734% when pulling 8cf0e7e on cgunther:fix-issue-765 into 920af01 on norman:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.734% when pulling 8cf0e7e on cgunther:fix-issue-765 into 920af01 on norman:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.734% when pulling 8cf0e7e on cgunther:fix-issue-765 into 920af01 on norman:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.734% when pulling 8cf0e7e on cgunther:fix-issue-765 into 920af01 on norman:master.

1f8af74 mistakenly assigns to the literal `slug` method, rather than
the configured `slug_column`. As a result, models using a custom
`slug_column` get an error raised when validations fail that `slug=` is
an undefined method.

Fixes norman#765.
@coveralls
Copy link

coveralls commented Dec 5, 2016

Coverage Status

Coverage remained the same at 96.734% when pulling cb4ddf1 on cgunther:fix-issue-765 into 920af01 on norman:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.734% when pulling cb4ddf1 on cgunther:fix-issue-765 into 920af01 on norman:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.734% when pulling cb4ddf1 on cgunther:fix-issue-765 into 920af01 on norman:master.

@marshall-lee
Copy link
Contributor

@parndt @kimrgrey @norman

Please please merge this one and release it in "5.2.1"! :)

We really need this unset_slug_if_invalid callback and other changes introduced in "5.2.0" but our tests fail on one of the models with non-standard slug field.

@ezekg
Copy link

ezekg commented Dec 22, 2016

Spree Commerce is also waiting on this patch: spree/spree#7758. Appreciate all the work that you put into this lib, everyone. 👍

@ezekg
Copy link

ezekg commented Jan 19, 2017

Is there anyway we could get this merged so that we can update friendly_id for Spree 3.3?

@parndt parndt merged commit d29ec3d into norman:master Jan 25, 2017
@parndt
Copy link
Collaborator

parndt commented Jan 25, 2017

Thanks @cgunther ❤️

@vfonic
Copy link
Contributor

vfonic commented Apr 7, 2017

+1 thanks for all the work! /cc @parndt
Can you please release 5.2.1?

@parndt
Copy link
Collaborator

parndt commented Apr 9, 2017

@vfonic done 😄 thanks for the reminder.

@vfonic
Copy link
Contributor

vfonic commented Apr 9, 2017

Thanks! 🍾 🎉 😄

@marshall-lee
Copy link
Contributor

Thanks!

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.

6 participants