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.

Sign up for GitHub

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

Jump to bottom

Make application relationship optional in access token model #780

Closed
wants to merge 1 commit into from
Closed

Make application relationship optional in access token model #780

wants to merge 1 commit into from

Conversation

ashishtajane
Copy link

This PR (rails/rails#18937) makes all belongs_to relations in rails 5 as required by default. But application object need not be present for access token creation in Resource Owner Password Credentials grant flow. So this makes application relationship in access token model as optional.

This fixes #774

@ashishtajane
Make application relation optional
2021b14 
This PR (rails/rails#18937) makes all
`belongs_to` relations required by default. So make application
relation optional manually as it is not required.

[Fixes doorkeeper-gem#774]
@ashishtajane ashishtajane mentioned this pull request Jan 27, 2016
Closed
@ashishtajane
Copy link
Author

The tests for rails 4.2 are failing because optional keyword is added in rails 5.
Is doorkeeper#master supporting rails 4.2 also?

@tute
Copy link
Contributor

tute commented Jan 29, 2016

Is doorkeeper#master supporting rails 4.2 also?

It is. We should do a conditional there. Yikes.

@tute
Copy link
Contributor

tute commented Jan 29, 2016

Thanks for your work on this! :)

@relu
Copy link
Contributor

relu commented Feb 13, 2016

Since required is deprecated in rails 5, I think a better approach that would suite both version groups would be to remove optional: true and just go with a validation: validates :application_id, presence: false. I tested this with rails master locally and it seems to work just fine. I know it doesn't look pretty but at least it works for both versions without any extra conditional garbage. Will follow-up with a PR.

EDIT: Actually, ignore that, it doesn't seem to be working as expected.

relu pushed a commit to relu/doorkeeper that referenced this pull request Feb 13, 2016
compat with rails 5's default belongs_to presence validation
a7afbc1 
Rails 5 adds default presence validation on belongs_to associations:
rails/rails#18233

Application object need not be present for access token creation in Resource
Owner Password Credentials grant flow.

This is an alternative to doorkeeper-gem#780 for backwards compatibility with rails 4
@relu relu mentioned this pull request Feb 13, 2016
Closed
@tute tute closed this in 747a7b4 Feb 26, 2016
@tute
Copy link
Contributor

tute commented Feb 26, 2016

Thank you, @ashishtajane and @relu! Merged in a variation of your solutions. 😃 👏 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ActiveRecord::RecordInvalid - Validation failed: Application must exist with Rails 5 and EmberSimpleAuth
3 participants
@ashishtajane @tute @relu