Add validation checks where we expect only one open task #15220
Comments
yoomlam
added
Product: caseflow-queue
Type: Bug
Team: Echo 🐬
Priority: Low
1 |
2 | ||||||
3 | |||
5 |
8 | Test flows of concern: reassignment, cancellation of JATask, cancellation of AttyTask. UI error probably a no-op for this task - but keep an eye out Why 1?Why 2?
Why 3?
|
|
Another reason to have this validation: https://github.com/department-of-veterans-affairs/dsva-vacols/issues/174 |
|
Communicated this verbally, but just to codify... Since this appears to be something that isn't isolated to |
|
Given PR #16072, which uses a thread-local variable, would a simpler way be something like https://stackoverflow.com/questions/8881712/skip-certain-validation-method-in-model, where we set a non-DB attribute like |
… time (#16072) Bumps #15220 ### Description Add a validation that ensures only one `JudgeAssignTask` can be created at a time. This functionality can be extended to other tasks where there should only be one, such as `ChangeHearingDispositionTask`. I am leaving that work for another PR. ### Acceptance Criteria - [x] Code compiles correctly - [x] No existing tests break - [x] There is test coverage for the new validation - [x] It is still possible to reassign a judge to a case through the UI ### Testing Plan 1. Ensure that automated tests pass. 2. While running the site locally, test the reassign judge scenario - Switch to user BVAAABSHIRE (VACO) - Find an appeal that has a JudgeAssignTask already associated with it - In the “currently active tasks” section, select “re-assign judge”  - Ensure that you can still re-assign a judge without getting any error messages 3. Through rails console, test that a second JudgeAssignTask cannot be made for the same appeal Find an appeal with a `JudgeAssignTask`. Next, find a user that will be `assigned_to`, and `assigned_by`. ``` appeal = JudgeAssignTask.last.appeal assigned_to = User.first assigned_by = User.last ``` Watch the validation fail after attempting to make a second `JudgeAssignTask` for this appeal. ``` FactoryBot.create(:ama_judge_assign_task, assigned_to: assigned_to, assigned_by: assigned_by, appeal: appeal) ``` ### Lessons Learned Along the Way I wanted to document some of the bumps I hit while working on this PR. The test initially failed on setup in the let block [where it creates the first JudgeAssignTask](https://github.com/department-of-veterans-affairs/caseflow/pull/16072/files#diff-3d062019a35dcde6fb082f68bce1d3ca970bfd5af401845e146b0616cdfb2000R18). While debugging with Pry, I found that the validation was called twice when creating the first `JudgeAssignTask`. While working on it with Yoom, he pointed out that I had written a validation which was called both when the record was inserted in the database and saved to the database. [Relevant documentation quote](https://guides.rubyonrails.org/active_record_validations.html#when-does-validation-happen-questionmark): >Creating and saving a new record will send an SQL INSERT operation to the database. Updating an existing record will send an SQL UPDATE operation instead. Validations are typically run before these commands are sent to the database. Since my custom validation ran on `update` and `create`, it also caused unrelated tests to fail on setup. I changed the validation to only be called when the `JudgeAssignTask` task is saved to the database. Here's [helpful documentation about custom validations](https://guides.rubyonrails.org/active_record_validations.html#custom-methods). Next, I locally tested reassigning a `JudgeAssignTask` to see if the functionality still works. Initially, it did not work because the reassign flow is 1. create a duplicate task and assign it to another user 2. cancel old task The on create validation made it impossible to do the above. I [edited the `reassign` method](https://github.com/department-of-veterans-affairs/caseflow/pull/16072/files#diff-4095b65754098dce7229824782d43e80a7cb3f0fe45aa245102593760b51a24bR559-R563) so that it will allow two `JudgeAssignTask`s to simultaneously exist. I found this [StackOverflow post](https://stackoverflow.com/questions/43105871/can-you-rescue-from-specific-errors-with-messages-in-ruby) helpful for my approach. ### Updated approach After pairing with Yoom and Bar, I revised my original approach in favor of using thread-local variables. You can read more about this approach in the associated hackmd document's [Section 5, Thread-local variables](https://hackmd.io/@mGFT9b9_R6WJvEipbrJIjA/rJfAr9iVd). I want to highlight that thread-local variables use the [`Thread.current.thread_variable_set("foo", "bar")`](https://api.rubyonrails.org/v4.2/classes/Thread.html) syntax, whereas fiber-local variables use the `Thread.current["foo"] = "bar"` syntax.
|
Confirmation for JudgeDecisionReviewTask and AttorneyTask: https://github.com/department-of-veterans-affairs/dsva-vacols/issues/212#issuecomment-889426499 |
|
Here is a list of tasks that the board has confirmed there should only be one open task at a time, and tasks that we still need the board to confirm there should only be one open task at a time. The board has confirmed that an appeal should have only one open task of this type at a time
We need the board to confirm if there should only be one open task at a time
|
|
Update: I spoke with JC today and he confirmed that an appeal should not have more than two open
|
I discovered this
Echo may want to add tests or other validations for those specific tasks. The caseflow/lib/helpers/check_task_tree.rb Lines 153 to 176 in 695ddfd |
Motivated by https://github.com/department-of-veterans-affairs/dsva-vacols/issues/145, https://github.com/department-of-veterans-affairs/dsva-vacols/issues/174, and #14307 (comment).
Description
Add validation checks where we expect only 1
opentask for certain task types, e.g.JudgeAssignTask.Acceptance criteria
JudgeAssignTaskshould exist on an appeal at any time -- PR Validate that an appeal can have only one JudgeAssignTask active at a time #16072Background/context/resources
Refer to https://github.com/department-of-veterans-affairs/appeals-team/blob/master/Project%20Folders/Tasks/tasktrees/descr/tasks-overview.md for identifying other task types.
Technical notes
Is there a
Concernor otherwise reusable code we can add to these task types?Reminder: ensure that task reassignment for the task type still works since the reassignment process is:
The text was updated successfully, but these errors were encountered: