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

slug being unset if model has any errors (unset_slug_if_invalid) #902

Closed
Ancez opened this issue Apr 25, 2019 · 4 comments
Closed

slug being unset if model has any errors (unset_slug_if_invalid) #902

Ancez opened this issue Apr 25, 2019 · 4 comments
Labels

Comments

@Ancez
Copy link

Ancez commented Apr 25, 2019

When running a .save() method on a model which has errors on other fields (e.g. name is invalid), the slug gets set to nil but was valid. It should preserve the value after .save() on the model so the user can fix the remaining errors without having to rewrite the slug. This also leads to other errors on the slug field that may have caused the model to fail validation to be removed.

method unset_slug_if_invalid shouldn't be checking for errors.present?. Instead it should only check that the slug field has any errors (doing errors[friendly_id_config.query_field].present? solves the problem)

  def unset_slug_if_invalid
    if errors[friendly_id_config.query_field].present? && attribute_changed?(friendly_id_config.query_field.to_s)
      diff = changes[friendly_id_config.query_field]
      send "#{friendly_id_config.slug_column}=", diff.first
    end
   end
@parndt
Copy link
Collaborator

parndt commented Apr 27, 2019

Hi @Ancez thanks for this; could you please open a pull request with this change so we can run tests against it?

@Ancez
Copy link
Author

Ancez commented Apr 29, 2019

Hi @parndt, thank you for your reply, I've created a PR with this code change but some of the specs will need updating. This issue might not be visible when running the specs but it will become quite blunt once testing it on the website

@parndt
Copy link
Collaborator

parndt commented May 2, 2019

cool, keen to see what you come up with, hopefully avoiding a breaking change.

@stale
Copy link

stale bot commented Dec 11, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 11, 2019
@stale stale bot closed this as completed Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants