Refactor user validations #1090
Conversation
|
This definitely feels like a good candidate to pull out into its own class 👍 |
Thanks for the feedback, @andrew. I'll do some extractin' 😄 |
AndyStabler
changed the title
|
They're failing because those tests have an invalid github id and the validation returns early if this is the case (see here). This means the expected errors aren't being added to the error object. @andrew what would you like to do here? We could stop returning early in the validation and add all errors to the |
|
@AndyStabler ah pesky 🤔 Adding all the errors feels like the right way to go to me |
Extracts methods for each validation around personal access tokens Addresses the following Rubocop issue: app/models/user.rb:123:3: C: Metrics/CyclomaticComplexity: Cyclomatic complexity for personal_access_token_validator is too high. [8/6] def personal_access_token_validator ... ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The token validation logic was quite noisy in the User class, so this commit pulls it out into its own class. As part of this we have updated the validation to not return early if validation fails—now all validation errors will be added to the errors object.
AndyStabler
force-pushed
the
refactor-user-validations
branch
from
52cfd6f
to
7b95c17
Compare
|
|
||
| class PersonalAccessTokenValidatorTest < ActiveSupport::TestCase | ||
| setup do | ||
| @user = build(:user, personal_access_token: '1234') |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
Alrighty, @andrew this is ready for review again just whenever you get the time 🙇 |
AndyStabler
changed the title
|
Great work, a very nice refactoring 👌 |
* 'master' of github.com:coreyja/octobox: (56 commits) Add Guard gem and config (octobox#1111) Bump xpath from 3.1.0 to 3.2.0 (octobox#1107) Bump jwt from 1.5.6 to 2.1.0 (octobox#1104) Bump autoprefixer-rails from 9.1.4 to 9.2.0 (octobox#1103) Bump i18n from 1.1.0 to 1.1.1 (octobox#1102) Bump mail from 2.7.0 to 2.7.1 (octobox#1101) Refactor user validations (octobox#1090) Bump oauth2 from 1.4.0 to 1.4.1 (octobox#1100) Tiny typo fix in notification model test (octobox#1099) Show first few lines of issue/pr body as title on notification links (octobox#1097) Fix typo for Unlabelled filter (octobox#1098) Subject syncing is now live on Octobox.io Sync subject body for issues and pull requests (octobox#1094) Ensure locked field is updated when existing subjects are updated (octobox#1096) Status webhooks should update subects with combined status result (octobox#1093) Fix dropdown svg icons colors Bump octokit from 4.12.0 to 4.13.0 (octobox#1091) Fixed sidebar colors (octobox#1079) Rubocop fixes (octobox#1075) Prompt users to log into the GitHub App (octobox#1078) ...
Addresses the following Rubocop issue by extracting methods for each validation around personal access tokens:
Although this reduces the cyclomatic complexity of
personal_access_token_validator, it introduces 5 new methods 🤔If we want to refactorpersonal_access_token_validatoris it worth introducing a validator class for this logic to live in?(also, I get it if you're not bothered by the Rubocop issue and want to close this—I'm just having fun exploring the codebase 😄)