Validate that an appeal can have only one JudgeAssignTask active at a time

[eileen-nava]

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

• Code compiles correctly
• No existing tests break
• There is test coverage for the new validation
• 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. 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:

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.

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 so that it will allow two JudgeAssignTasks to simultaneously exist. I found this StackOverflow post 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. I want to highlight that thread-local variables use the Thread.current.thread_variable_set("foo", "bar") syntax, whereas fiber-local variables use the Thread.current["foo"] = "bar" syntax.

[codeclimate]

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

View more on Code Climate.

[yoomlam]

Woohoo!

[eileen-nava]

@yoomlam Here is the expected error message.

[va-bot]

	1 Warning
⚠️	This PR modifies React components — consider adding/updating corresponding Storybook file

Generated by 🚫 Danger

[eileen-nava]

@yoomlam I consulted design about the error message wording after getting the PR approved by Ann-Marie during a demo. Below is the current wording.

I will merge this after the build passes.

[yoomlam]

@eileen-nava I think that message may be too specific. If the validation fails for other scenarios, this error message (mentioning "closing the window" and "previous window") is confusing.

[eileen-nava]

@eileen-nava I think that message may be too specific. If the validation fails for other scenarios, this error message (mentioning "closing the window" and "previous window") is confusing.

Good point. I'll check in with design again. Thank you!

[eileen-nava]

Here is the current error message.