WIP Add auto-distributor logging #14897
Conversation
Generated by 🚫 Danger |
|
Code Climate has analyzed commit e5d5df6 and detected 6 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
| @@ -250,7 +250,7 @@ | |||
| it "describes what changes will be made and makes them" do | |||
| automatic_org_message = "Reassigning #{task_count} tasks with ids #{ids_output} to " \ | |||
| "#{team_member_count} members of the parent tasks' organization" | |||
| expect(Rails.logger).to receive(:info).with(automatic_org_message) | |||
| # TO FIX: expect(Rails.logger).to receive(:info).with(automatic_org_message) | |||
There was a problem hiding this comment.
@hschallhorn Is there a better way to test these?
If we end up adding logging to the distributors, the new logging causes these expect statements to fail since the new logging will occur first.
There was a problem hiding this comment.
will adding an expect(Rails.logger).to receive(:info).with(round_robin_logging) before this line work?
There was a problem hiding this comment.
It does, but I don't want to pollute this test with out-of-scope code.
We won't need to do logging since I identified the problem.
| &.tasks | ||
| &.open | ||
| &.find_by(assigned_to: assignee_pool) | ||
| &.assigned_to | ||
|
|
||
| @state[:appeal_id] = options[:appeal]&.id |
There was a problem hiding this comment.
@lomky Here's how I'm doing the logging. I populate @state with bits of state (similar to how Raven/Sentry does it I think).
| elements = [:class, :invoker, :appeal_id, :task_class, :task_id, :latest_task_id, :last_assignee_id, :next_index, | ||
| :existing_assignee, :assignee_pool] | ||
| log_string = "RRDTracking;" + elements.map { |key| "#{key}: #{@state[key]}" }.join("; ") |
There was a problem hiding this comment.
@lomky Then I log the state here. We should also probably clear the state after logging, but Colocated org creates a new Distributor with each next_assignee call, so it probably doesn't matter.
|
No need to collect info; problem was identified: #14629 (comment) |
| @@ -19,7 +19,7 @@ | |||
|
|
|||
| before do | |||
| total_distribution_count.times do | |||
| create(:task, assigned_to: colocated_task_distributor.next_assignee) | |||
| create(:ama_colocated_task) | |||
There was a problem hiding this comment.
Creating a ColocatedTask enables task.create_and_auto_assign_child_task to be called.
Resolves #14629
Description
Please explain the changes you made here.
Acceptance Criteria
Testing Plan
Code Documentation Updates