Skip to content

Commit 73582d9

Browse files
Chris0JekyChris0Jeky
committed
Remove unused userId param from AnalyzeAsync and add SideEffectTone enum validation
Drop the unused userId parameter from ISideEffectAnalyzer.AnalyzeAsync, its implementation, the controller call site, and all tests. Add Enum.IsDefined guard to SideEffectRow constructor to reject undefined SideEffectTone values.
1 parent 7fa3345 commit 73582d9

5 files changed

Lines changed: 32 additions & 31 deletions

File tree

‎backend/src/Taskdeck.Api/Controllers/AutomationProposalsController.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ public async Task<IActionResult> GetProposalSideEffects(Guid id, CancellationTok
278278
if (auth.ErrorResult is not null)
279279
return auth.ErrorResult;
280280

281-
var result = await _sideEffectAnalyzer.AnalyzeAsync(id, callerUserId, cancellationToken);
281+
var result = await _sideEffectAnalyzer.AnalyzeAsync(id, cancellationToken);
282282
return result.IsSuccess ? Ok(result.Value) : result.ToErrorActionResult();
283283
}
284284

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace Taskdeck.Application.Services;
1010
public interface ISideEffectAnalyzer
1111
{
1212
/// <summary>
13-
/// Analyzes the side effects of the specified proposal for the given user.
13+
/// Analyzes the side effects of the specified proposal.
1414
/// </summary>
15-
Task<Result<ProposalSideEffectsDto>> AnalyzeAsync(Guid proposalId, Guid userId, CancellationToken cancellationToken = default);
15+
Task<Result<ProposalSideEffectsDto>> AnalyzeAsync(Guid proposalId, CancellationToken cancellationToken = default);
1616
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ public SideEffectAnalyzer(IUnitOfWork unitOfWork)
2828

2929
public async Task<Result<ProposalSideEffectsDto>> AnalyzeAsync(
3030
Guid proposalId,
31-
Guid userId,
3231
CancellationToken cancellationToken = default)
3332
{
3433
var proposal = await _unitOfWork.AutomationProposals.GetByIdAsync(proposalId, cancellationToken);

‎backend/src/Taskdeck.Domain/Entities/SideEffectRow.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ public SideEffectRow(string key, string value, SideEffectTone tone)
2121
throw new ArgumentException("Side-effect key cannot be empty.", nameof(key));
2222
if (string.IsNullOrWhiteSpace(value))
2323
throw new ArgumentException("Side-effect value cannot be empty.", nameof(value));
24+
if (!Enum.IsDefined(tone))
25+
throw new ArgumentOutOfRangeException(nameof(tone), tone, "Unrecognized SideEffectTone value.");
2426

2527
Key = key;
2628
Value = value;

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

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public async Task AnalyzeAsync_ShouldReturnNotFound_WhenProposalDoesNotExist()
6161
_proposalRepoMock.Setup(r => r.GetByIdAsync(It.IsAny<Guid>(), default))
6262
.ReturnsAsync((AutomationProposal?)null);
6363

64-
var result = await _analyzer.AnalyzeAsync(Guid.NewGuid(), Guid.NewGuid());
64+
var result = await _analyzer.AnalyzeAsync(Guid.NewGuid());
6565

6666
result.IsSuccess.Should().BeFalse();
6767
result.ErrorCode.Should().Be("NotFound");
@@ -74,7 +74,7 @@ public async Task AnalyzeAsync_ShouldReturnSevenRows()
7474
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
7575
.ReturnsAsync(proposal);
7676

77-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
77+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
7878

7979
result.IsSuccess.Should().BeTrue();
8080
result.Value.Rows.Should().HaveCount(7);
@@ -87,7 +87,7 @@ public async Task AnalyzeAsync_ShouldReturnCorrectRowKeys()
8787
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
8888
.ReturnsAsync(proposal);
8989

90-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
90+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
9191

9292
result.IsSuccess.Should().BeTrue();
9393
var keys = result.Value.Rows.Select(r => r.Key).ToList();
@@ -101,7 +101,7 @@ public async Task AnalyzeAsync_CardsMutation_ShouldBeActive_WhenCreateOperation(
101101
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
102102
.ReturnsAsync(proposal);
103103

104-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
104+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
105105

106106
result.IsSuccess.Should().BeTrue();
107107
var cardsRow = result.Value.Rows.First(r => r.Key == "Cards");
@@ -115,7 +115,7 @@ public async Task AnalyzeAsync_CardsMutation_ShouldBeActive_WhenMoveOperation()
115115
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
116116
.ReturnsAsync(proposal);
117117

118-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
118+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
119119

120120
var cardsRow = result.Value.Rows.First(r => r.Key == "Cards");
121121
cardsRow.Tone.Should().Be("active");
@@ -128,7 +128,7 @@ public async Task AnalyzeAsync_CardsMutation_ShouldBeActive_WhenArchiveOperation
128128
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
129129
.ReturnsAsync(proposal);
130130

131-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
131+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
132132

133133
var cardsRow = result.Value.Rows.First(r => r.Key == "Cards");
134134
cardsRow.Tone.Should().Be("active");
@@ -141,7 +141,7 @@ public async Task AnalyzeAsync_CardsMutation_ShouldBeActive_WhenBulkMoveOperatio
141141
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
142142
.ReturnsAsync(proposal);
143143

144-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
144+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
145145

146146
var cardsRow = result.Value.Rows.First(r => r.Key == "Cards");
147147
cardsRow.Tone.Should().Be("active");
@@ -155,7 +155,7 @@ public async Task AnalyzeAsync_CardsMutation_ShouldBeActive_WhenCreateColumnOnly
155155
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
156156
.ReturnsAsync(proposal);
157157

158-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
158+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
159159

160160
var cardsRow = result.Value.Rows.First(r => r.Key == "Cards");
161161
cardsRow.Tone.Should().Be("active");
@@ -169,7 +169,7 @@ public async Task AnalyzeAsync_Subtasks_ShouldAlwaysBePassive()
169169
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
170170
.ReturnsAsync(proposal);
171171

172-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
172+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
173173

174174
var subtasksRow = result.Value.Rows.First(r => r.Key == "Subtasks");
175175
subtasksRow.Tone.Should().Be("passive");
@@ -182,7 +182,7 @@ public async Task AnalyzeAsync_Comments_ShouldAlwaysBePassive()
182182
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
183183
.ReturnsAsync(proposal);
184184

185-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
185+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
186186

187187
var commentsRow = result.Value.Rows.First(r => r.Key == "Comments");
188188
commentsRow.Tone.Should().Be("passive");
@@ -195,7 +195,7 @@ public async Task AnalyzeAsync_ActivityLog_ShouldBeActive_WhenOperationsExist()
195195
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
196196
.ReturnsAsync(proposal);
197197

198-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
198+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
199199

200200
var activityRow = result.Value.Rows.First(r => r.Key == "Activity log");
201201
activityRow.Tone.Should().Be("active");
@@ -208,7 +208,7 @@ public async Task AnalyzeAsync_ActivityLog_ShouldBePassive_WhenNoOperations()
208208
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
209209
.ReturnsAsync(proposal);
210210

211-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
211+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
212212

213213
var activityRow = result.Value.Rows.First(r => r.Key == "Activity log");
214214
activityRow.Tone.Should().Be("passive");
@@ -221,7 +221,7 @@ public async Task AnalyzeAsync_Notifications_ShouldBeActive_WhenOperationsExist(
221221
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
222222
.ReturnsAsync(proposal);
223223

224-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
224+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
225225

226226
var notifRow = result.Value.Rows.First(r => r.Key == "Notifications");
227227
notifRow.Tone.Should().Be("active");
@@ -234,7 +234,7 @@ public async Task AnalyzeAsync_Notifications_ShouldBePassive_WhenNoOperations()
234234
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
235235
.ReturnsAsync(proposal);
236236

237-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
237+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
238238

239239
var notifRow = result.Value.Rows.First(r => r.Key == "Notifications");
240240
notifRow.Tone.Should().Be("passive");
@@ -252,7 +252,7 @@ public async Task AnalyzeAsync_Webhooks_ShouldBeActive_WhenBoardHasActiveSubscri
252252
_webhookRepoMock.Setup(r => r.GetActiveByBoardAsync(boardId, default))
253253
.ReturnsAsync(new List<OutboundWebhookSubscription> { subscription });
254254

255-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
255+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
256256

257257
var webhookRow = result.Value.Rows.First(r => r.Key == "Webhooks");
258258
webhookRow.Tone.Should().Be("active");
@@ -269,7 +269,7 @@ public async Task AnalyzeAsync_Webhooks_ShouldBePassive_WhenBoardHasNoSubscripti
269269
_webhookRepoMock.Setup(r => r.GetActiveByBoardAsync(boardId, default))
270270
.ReturnsAsync(new List<OutboundWebhookSubscription>());
271271

272-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
272+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
273273

274274
var webhookRow = result.Value.Rows.First(r => r.Key == "Webhooks");
275275
webhookRow.Tone.Should().Be("passive");
@@ -282,7 +282,7 @@ public async Task AnalyzeAsync_Webhooks_ShouldBePassive_WhenNoBoardId()
282282
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
283283
.ReturnsAsync(proposal);
284284

285-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
285+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
286286

287287
var webhookRow = result.Value.Rows.First(r => r.Key == "Webhooks");
288288
webhookRow.Tone.Should().Be("passive");
@@ -295,7 +295,7 @@ public async Task AnalyzeAsync_Calendar_ShouldAlwaysBePassive()
295295
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
296296
.ReturnsAsync(proposal);
297297

298-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
298+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
299299

300300
var calendarRow = result.Value.Rows.First(r => r.Key == "Calendar");
301301
calendarRow.Tone.Should().Be("passive");
@@ -309,7 +309,7 @@ public async Task AnalyzeAsync_CardsMutation_ShouldNotTreatColumnCreateAsCardMut
309309
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
310310
.ReturnsAsync(proposal);
311311

312-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
312+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
313313

314314
var cardsRow = result.Value.Rows.First(r => r.Key == "Cards");
315315
// Should be active (column mutation) but description should mention columns, not card mutations
@@ -324,7 +324,7 @@ public async Task AnalyzeAsync_Cards_ShouldShowBothCardAndColumnMutations()
324324
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
325325
.ReturnsAsync(proposal);
326326

327-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
327+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
328328

329329
var cardsRow = result.Value.Rows.First(r => r.Key == "Cards");
330330
cardsRow.Tone.Should().Be("active");
@@ -344,7 +344,7 @@ public async Task AnalyzeAsync_Webhooks_ShouldBePassive_WhenActiveWebhooksButNoO
344344
_webhookRepoMock.Setup(r => r.GetActiveByBoardAsync(boardId, default))
345345
.ReturnsAsync(new List<OutboundWebhookSubscription> { subscription });
346346

347-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
347+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
348348

349349
var webhookRow = result.Value.Rows.First(r => r.Key == "Webhooks");
350350
webhookRow.Tone.Should().Be("passive");
@@ -362,7 +362,7 @@ public async Task AnalyzeAsync_Reversibility_ShouldUseDefaultWindow_ForLowRisk()
362362
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
363363
.ReturnsAsync(proposal);
364364

365-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
365+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
366366

367367
result.Value.Reversibility.WindowMs.Should().Be(Reversibility.DefaultWindowMs);
368368
}
@@ -374,7 +374,7 @@ public async Task AnalyzeAsync_Reversibility_ShouldUseDefaultWindow_ForMediumRis
374374
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
375375
.ReturnsAsync(proposal);
376376

377-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
377+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
378378

379379
result.Value.Reversibility.WindowMs.Should().Be(Reversibility.DefaultWindowMs);
380380
}
@@ -386,7 +386,7 @@ public async Task AnalyzeAsync_Reversibility_ShouldUseDefaultWindow_ForHighRisk(
386386
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
387387
.ReturnsAsync(proposal);
388388

389-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
389+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
390390

391391
result.Value.Reversibility.WindowMs.Should().Be(Reversibility.DefaultWindowMs);
392392
}
@@ -398,7 +398,7 @@ public async Task AnalyzeAsync_Reversibility_ShouldUseHalfWindow_ForCriticalRisk
398398
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
399399
.ReturnsAsync(proposal);
400400

401-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
401+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
402402

403403
result.Value.Reversibility.WindowMs.Should().Be(Reversibility.DefaultWindowMs / 2);
404404
}
@@ -410,7 +410,7 @@ public async Task AnalyzeAsync_Reversibility_ShouldDescribeNoOps_WhenNoOperation
410410
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
411411
.ReturnsAsync(proposal);
412412

413-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
413+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
414414

415415
result.Value.Reversibility.Summary.Should().Contain("no operations");
416416
result.Value.Reversibility.WindowMs.Should().Be(Reversibility.DefaultWindowMs);
@@ -423,7 +423,7 @@ public async Task AnalyzeAsync_Reversibility_CriticalRisk_ShouldMentionManualInt
423423
_proposalRepoMock.Setup(r => r.GetByIdAsync(proposal.Id, default))
424424
.ReturnsAsync(proposal);
425425

426-
var result = await _analyzer.AnalyzeAsync(proposal.Id, Guid.NewGuid());
426+
var result = await _analyzer.AnalyzeAsync(proposal.Id);
427427

428428
result.Value.Reversibility.Summary.Should().Contain("manual intervention");
429429
result.Value.Reversibility.Description.Should().Contain("Critical-risk");

0 commit comments

Comments
 (0)