Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Issue #79: Refactor UserController as demonstrated in PR 13 #80

Merged
merged 3 commits into from
Oct 19, 2017
Merged

Issue #79: Refactor UserController as demonstrated in PR 13 #80

merged 3 commits into from
Oct 19, 2017

Conversation

ddelponte
Copy link
Collaborator

  1. pulled the existing code used to build the roleMap out into its own, testable method
  2. implemented a test to verify existing functionality
  3. applied refactoring from PR Unnecessary logic double-checks whether user has roles, creates extra ma... #13 which simplified logic to only do the 'contains' check one time for each role

Fixes issue #79

@ddelponte
Issue #79: Refactor UserController as demonstrated in PR 13
341c1f7 
This is step one of the refactoring which:
1. pulls the existing code used to build the `roleMap` out into its own, testable method
2. implements a test to verify existing functionality
@ddelponte
Issue #79: Refactor UserController as demonstrated in PR 13
fccf262 
Applied refactoring from PR #13
@ddelponte ddelponte requested a review from sdelamo October 4, 2017 18:43
@ddelponte ddelponte mentioned this pull request Oct 4, 2017
Closed
@sdelamo sdelamo merged commit c9f2b4a into master Oct 19, 2017
@sdelamo sdelamo deleted the issue-79 branch October 19, 2017 15:25
@ddelponte ddelponte mentioned this pull request Oct 19, 2017
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@sdelamo sdelamo Awaiting requested review from sdelamo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ddelponte @sdelamo