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

Refactors SecureValidatable #222

Conversation

tmr08c
Copy link
Collaborator

@tmr08c tmr08c commented Jul 30, 2020

#63 ends up doing a fair amount of refactoring. I will revisit this PR to see if it's still useful when it gets merged.

Closes #203

My general approach was:

  • Make Rubocop happy
  • Follow practices used elsewhere (mainly, ActiveSupport::Concern)
  • Adds documentation

Adds test confirming functionality for when a class does not support
validations.
* Converts to use ActiveSupport::Concern
* Breaks up `included` block - specifically, pulled out some of the
  shared logic for adding validations
This felt more clear as a guard clause
@tmr08c tmr08c self-assigned this Jul 30, 2020
@coveralls
Copy link
Collaborator

Coverage Status

Coverage increased (+0.03%) to 96.842% when pulling 90975d1 on tmr08c:tmr08c-gh-203-refactor-secure-validatable into 5312662 on devise-security:master.

@dillonwelch dillonwelch added this to the 0.17.0 milestone Oct 11, 2021
@dillonwelch dillonwelch removed this from the 0.17.0 milestone Dec 21, 2021
@dillonwelch
Copy link
Contributor

All changes have been done via the linked PRs and issues in #63.

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 this pull request may close these issues.

Refactor secure_validatable
3 participants