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

Make configuration and specs ORM independent #823

Merged
merged 2 commits into from
Apr 28, 2016
Merged

Make configuration and specs ORM independent #823

merged 2 commits into from
Apr 28, 2016

Conversation

nbulaj
Copy link
Member

@nbulaj nbulaj commented Apr 27, 2016

Hi. This PR fixes a few issues with dependence on the ORM:

  1. Models Uniquenes validations in specs - previously caught only ActiveRecord::RecordNotUnique exceptions, now error class(es) depends on configured ORM. Tests attached.
  2. Method refresh_token_revoked_on_use? in Doorkeeper configuration - was not actually a config option. Extracted to AccessToken model due to it's logic and can be override in any Doorkeeper ORM. Tests attached.
  3. expect { ... }.to change { relation.associations.count } expectations - fixed missed reload method to get actual information. Tests attached.

@tute
Copy link
Contributor

tute commented Apr 27, 2016

Beautiful changeset, thank you! Can you please improve the commit message, merging the contents of the current commit message and your pull request description? You can see a good example in http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html.

Also, can you update the NEWS.md file with a summary of these changes? Thanks again!

@nbulaj nbulaj changed the title Specs and config ORM independence Make configuration and specs ORM independent Apr 27, 2016
* Fix models uniquenes validation error in specs to depend on used ORM
* Extract 'refresh_token_revoked_on_use?' method to AccessToken model
* Fix missed 'reload' method in expect { }.to change { } expectations
@nbulaj
Copy link
Member Author

nbulaj commented Apr 28, 2016

@tute done.

@tute tute merged commit 2cbd70f into doorkeeper-gem:master Apr 28, 2016
@tute
Copy link
Contributor

tute commented Apr 28, 2016

Thank you! 🎉

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.

2 participants