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

Do not show inactive users as options for task assignee #13111

Merged
merged 9 commits into from
Jan 13, 2020

Conversation

kevmo
Copy link
Contributor

@kevmo kevmo commented Jan 9, 2020

Resolves #12949

Description

In "Assign to Person" task assignment modals, we were offering inactive users

Acceptance Criteria

  • Test suite passes.

Testing Plan

  1. Log in as translation team member Set up master as protected branch #1, e.g. ORG_QUEUE_USER_1
  2. Navigate to http://localhost:3000/organizations/translation, open a case (e.g. http://localhost:3000/queue/appeals/2e7d0f5b-f9eb-4767-9b41-40426d9b0562)
  3. In task actions, go to "Assign to Person". You should see Translation team member Add shell for starting an appeal #5.
  4. Now make Translation team member Add shell for starting an appeal #5 as inactive in the rails console:
    User.find_by_css_id("ORG_QUEUE_USER_5").inactive!
  5. Repeat steps 1 through 3. You should no longer see Team Member 5.

User Facing Changes

BEFORE:
Screen Shot 2020-01-13 at 9 54 00 AM

AFTER:
Screen Shot 2020-01-13 at 9 58 46 AM

client/COPY.json Outdated
@@ -230,7 +230,7 @@
"ASSIGN_WIDGET_NO_TASK_DETAIL": "Please select a task.",
"ASSIGN_WIDGET_SUCCESS": "%(verb)s %(numCases)s %(casePlural)s",
"ASSIGN_WIDGET_ASSIGNMENT_ERROR_TITLE": "Error assigning tasks",
"ASSIGN_WIDGET_ASSIGNMENT_ERROR_DETAIL": "One or more tasks couldn't be assigned. Reassign tasks to a judge on the case deatils screen",
"ASSIGN_WIDGET_ASSIGNMENT_ERROR_DETAIL": "One or more tasks couldn't be assigned. Reassign tasks to a judge on the case details screen",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a typo I found.

@kevmo kevmo changed the title 12949/inactive users task assignee Do not show inactive users as options for task assignee Jan 9, 2020
@codeclimate
Copy link

codeclimate bot commented Jan 9, 2020

Code Climate has analyzed commit cfa5a4d and detected 0 issues on this pull request.

View more on Code Climate.

@kevmo kevmo requested review from ajspotts and lomky January 13, 2020 14:59
Copy link
Contributor

@ajspotts ajspotts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No more task assignment to inactive users!

@kevmo kevmo self-assigned this Jan 13, 2020
@kevmo kevmo added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label Jan 13, 2020
@va-bot va-bot merged commit 2bdd7f3 into master Jan 13, 2020
@va-bot va-bot deleted the 12949/inactive-users-task-assignee branch January 13, 2020 16:30

def potential_task_assignees(task)
if task.assigned_to.is_a?(Organization)
task.assigned_to.users.reject(&:inactive?)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enums come with scopes for free as well. This can be simplified to task.assigned_to.users.active

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried to go that route, but it doesn't work because task.assigned_to.users is a Ruby array, not an Active Record object.

if task.assigned_to.is_a?(Organization)
task.assigned_to.users.reject(&:inactive?)
elsif task.parent&.assigned_to.is_a?(Organization)
task.parent.assigned_to.users.reject do |check_user|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing here, we can keep the original logic and just add .active after users

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not show inactive users as options for task assignee
4 participants