Skip to content

Commit 7fa3345

Browse files
Chris0JekyChris0Jeky
committed
Fix SideEffectAnalyzer: target type checks, column awareness, webhook no-op guard
- Add TargetType check to hasCardMutation so column operations are not misclassified as card mutations - Introduce hasColumnMutation and hasBoardMutation flags; Cards row now shows context-appropriate descriptions for card-only, column-only, or combined board mutations - Webhooks tone is Active only when hasActiveWebhooks AND hasAnyOperation, preventing misleading "will fire" on no-op proposals - Fix test that used wrong action type ("create_column" vs real "create") which masked the target-type bug - Add 6 new tests covering column mutations, target type filtering, and webhook no-ops edge case
1 parent be9a111 commit 7fa3345

2 files changed

Lines changed: 109 additions & 11 deletions

File tree

‎backend/src/Taskdeck.Application/Services/SideEffectAnalyzer.cs‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,26 @@ internal static IReadOnlyList<SideEffectRow> BuildSideEffectRows(
6060
IReadOnlyList<AutomationProposalOperation> operations,
6161
bool hasActiveWebhooks)
6262
{
63-
bool hasCardMutation = operations.Any(op => CardMutatingActions.Contains(op.ActionType));
63+
bool hasCardMutation = operations.Any(op =>
64+
CardMutatingActions.Contains(op.ActionType) &&
65+
string.Equals(op.TargetType, "card", StringComparison.OrdinalIgnoreCase));
66+
bool hasColumnMutation = operations.Any(op =>
67+
string.Equals(op.TargetType, "column", StringComparison.OrdinalIgnoreCase));
68+
bool hasBoardMutation = hasCardMutation || hasColumnMutation;
6469
bool hasAnyOperation = operations.Count > 0;
6570

6671
return new List<SideEffectRow>
6772
{
6873
new(
6974
"Cards",
70-
hasCardMutation
71-
? "Creates, moves, or archives cards on the board"
72-
: "No card mutations",
73-
hasCardMutation ? SideEffectTone.Active : SideEffectTone.Passive),
75+
hasBoardMutation
76+
? hasCardMutation && hasColumnMutation
77+
? "Creates, moves, or archives cards and adds columns on the board"
78+
: hasCardMutation
79+
? "Creates, moves, or archives cards on the board"
80+
: "Adds columns to the board (no direct card mutations)"
81+
: "No board mutations",
82+
hasBoardMutation ? SideEffectTone.Active : SideEffectTone.Passive),
7483

7584
new(
7685
"Subtasks",
@@ -98,10 +107,12 @@ internal static IReadOnlyList<SideEffectRow> BuildSideEffectRows(
98107

99108
new(
100109
"Webhooks",
101-
hasActiveWebhooks
110+
hasActiveWebhooks && hasAnyOperation
102111
? "Outbound webhooks configured for this board will fire"
103-
: "No outbound webhooks configured",
104-
hasActiveWebhooks ? SideEffectTone.Active : SideEffectTone.Passive),
112+
: hasActiveWebhooks
113+
? "Outbound webhooks configured but no operations to trigger them"
114+
: "No outbound webhooks configured",
115+
hasActiveWebhooks && hasAnyOperation ? SideEffectTone.Active : SideEffectTone.Passive),
105116

106117
new(
107118
"Calendar",

‎backend/tests/Taskdeck.Application.Tests/Services/SideEffectAnalyzerTests.cs‎

Lines changed: 90 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,16 +148,18 @@ public async Task AnalyzeAsync_CardsMutation_ShouldBeActive_WhenBulkMoveOperatio
148148
}
149149

150150
[Fact]
151-
public async Task AnalyzeAsync_CardsMutation_ShouldBePassive_WhenCreateColumnOnly()
151+
public async Task AnalyzeAsync_CardsMutation_ShouldBeActive_WhenCreateColumnOnly()
152152
{
153-
var proposal = CreateProposal(RiskLevel.Low, null, ("create_column", "column"));
153+
// Real column creation uses actionType "create" with targetType "column"
154+
var proposal = CreateProposal(RiskLevel.Low, null, ("create", "column"));
154155
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
155156
.ReturnsAsync(proposal);
156157

157158
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
158159

159160
var cardsRow = result.Value.Rows.First(r => r.Key == "Cards");
160-
cardsRow.Tone.Should().Be("passive");
161+
cardsRow.Tone.Should().Be("active");
162+
cardsRow.Value.Should().Contain("columns");
161163
}
162164

163165
[Fact]
@@ -299,6 +301,56 @@ public async Task AnalyzeAsync_Calendar_ShouldAlwaysBePassive()
299301
calendarRow.Tone.Should().Be("passive");
300302
}
301303

304+
[Fact]
305+
public async Task AnalyzeAsync_CardsMutation_ShouldNotTreatColumnCreateAsCardMutation()
306+
{
307+
// A "create" operation targeting "column" should NOT be classified as a card mutation
308+
var proposal = CreateProposal(RiskLevel.Low, null, ("create", "column"));
309+
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
310+
.ReturnsAsync(proposal);
311+
312+
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
313+
314+
var cardsRow = result.Value.Rows.First(r => r.Key == "Cards");
315+
// Should be active (column mutation) but description should mention columns, not card mutations
316+
cardsRow.Value.Should().NotContain("Creates, moves, or archives cards on the board");
317+
cardsRow.Value.Should().Contain("column");
318+
}
319+
320+
[Fact]
321+
public async Task AnalyzeAsync_Cards_ShouldShowBothCardAndColumnMutations()
322+
{
323+
var proposal = CreateProposal(RiskLevel.Low, null, ("create", "card"), ("create", "column"));
324+
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
325+
.ReturnsAsync(proposal);
326+
327+
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
328+
329+
var cardsRow = result.Value.Rows.First(r => r.Key == "Cards");
330+
cardsRow.Tone.Should().Be("active");
331+
cardsRow.Value.Should().Contain("cards");
332+
cardsRow.Value.Should().Contain("columns");
333+
}
334+
335+
[Fact]
336+
public async Task AnalyzeAsync_Webhooks_ShouldBePassive_WhenActiveWebhooksButNoOperations()
337+
{
338+
var boardId = Guid.NewGuid();
339+
var proposal = CreateProposal(RiskLevel.Low, boardId);
340+
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
341+
.ReturnsAsync(proposal);
342+
343+
var subscription = new OutboundWebhookSubscription(boardId, Guid.NewGuid(), "https://example.com/webhook", "secret-key-123");
344+
_webhookRepoMock.Setup(r => r.GetActiveByBoardAsync(boardId, default))
345+
.ReturnsAsync(new List<OutboundWebhookSubscription> { subscription });
346+
347+
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
348+
349+
var webhookRow = result.Value.Rows.First(r => r.Key == "Webhooks");
350+
webhookRow.Tone.Should().Be("passive");
351+
webhookRow.Value.Should().Contain("no operations");
352+
}
353+
302354
#endregion
303355

304356
#region Reversibility Tests
@@ -422,6 +474,41 @@ public void BuildSideEffectRows_WithWebhooks_ShouldSetWebhooksActive()
422474
webhookRow.Tone.Should().Be(SideEffectTone.Active);
423475
}
424476

477+
[Fact]
478+
public void BuildSideEffectRows_CreateColumnOperation_ShouldSetCardsActive()
479+
{
480+
var op = new AutomationProposalOperation(
481+
Guid.NewGuid(), 0, "create", "column", "{}", Guid.NewGuid().ToString());
482+
var rows = SideEffectAnalyzer.BuildSideEffectRows(new List<AutomationProposalOperation> { op }, false);
483+
484+
var cardsRow = rows.First(r => r.Key == "Cards");
485+
cardsRow.Tone.Should().Be(SideEffectTone.Active);
486+
cardsRow.Value.Should().Contain("column");
487+
}
488+
489+
[Fact]
490+
public void BuildSideEffectRows_CreateTargetingNonCard_ShouldNotSetCardMutation()
491+
{
492+
// "create" targeting "column" should not say "Creates, moves, or archives cards"
493+
var op = new AutomationProposalOperation(
494+
Guid.NewGuid(), 0, "create", "column", "{}", Guid.NewGuid().ToString());
495+
var rows = SideEffectAnalyzer.BuildSideEffectRows(new List<AutomationProposalOperation> { op }, false);
496+
497+
var cardsRow = rows.First(r => r.Key == "Cards");
498+
cardsRow.Value.Should().NotBe("Creates, moves, or archives cards on the board");
499+
}
500+
501+
[Fact]
502+
public void BuildSideEffectRows_WithWebhooksButNoOps_ShouldSetWebhooksPassive()
503+
{
504+
var operations = new List<AutomationProposalOperation>();
505+
var rows = SideEffectAnalyzer.BuildSideEffectRows(operations, hasActiveWebhooks: true);
506+
507+
var webhookRow = rows.First(r => r.Key == "Webhooks");
508+
webhookRow.Tone.Should().Be(SideEffectTone.Passive);
509+
webhookRow.Value.Should().Contain("no operations");
510+
}
511+
425512
#endregion
426513

427514
#region ComputeReversibility Static Tests

0 commit comments

Comments
 (0)