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.

Sign up for GitHub

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

Jump to bottom

Creates subclasses of JudgeTask for two actions #8072

Merged
merged 5 commits into from
Dec 3, 2018
Merged

Creates subclasses of JudgeTask for two actions #8072

merged 5 commits into from
Dec 3, 2018

Conversation

tomas-nava
Copy link
Contributor

@tomas-nava tomas-nava commented Nov 30, 2018 •
edited
Loading

Connects #8033

Description

Creates subclasses of JudgeTask for two actions.

Acceptance Criteria

  • Code compiles correctly

@lowellrex
Creates subclasses of JudgeTask for two actions
c5d0ace 
Co-authored-by: Lowell Wood <lowell@navahq.com>
@ghost ghost assigned tomas-nava Nov 30, 2018
@ghost ghost added the In-Progress label Nov 30, 2018
@lowellrex
Copy link
Contributor

lowellrex commented Dec 3, 2018 •
edited
Loading

Looks great! Though I think we missed a few places where we attempt to match the type field of a task with JudgeTask explicitly: Taskable.assigned_judge() and AttorneyTask.available_actions(). I think using the task.is_a?(JudgeTask) pattern in both places will make them work properly.

Also, let's use the new :ama_judge_review_task factory instead of explicitly setting JudgeTask as the type in an existing test.

Tomas Apodaca and others added 2 commits December 3, 2018 10:18
fix matching task type with JudgeTask
3618d2f 
Also add tests for Taskable methods and other small cleanups.
@lowellrex
Add tests to satisfy test coverage
7541bbc
@ghost ghost assigned lowellrex Dec 3, 2018
lowellrex
lowellrex approved these changes Dec 3, 2018
View reviewed changes
Copy link
Contributor

@lowellrex lowellrex left a comment

Choose a reason for hiding this comment

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

LGTM!

@tomas-nava tomas-nava 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 Dec 3, 2018
@ghost ghost assigned va-bot Dec 3, 2018
@va-bot va-bot merged commit 82f9d37 into master Dec 3, 2018
@va-bot va-bot deleted the tomas/8033-create-judgetask-subclasses branch December 3, 2018 20:20
@ghost ghost removed the In-Progress label Dec 3, 2018
@lowellrex lowellrex mentioned this pull request Dec 21, 2018
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lowellrex lowellrex lowellrex approved these changes

Assignees

@va-bot va-bot

@lowellrex lowellrex

@tomas-nava tomas-nava

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tomas-nava @lowellrex @va-bot