Drag to reorder. Position 1 = highest priority.
@@ -182,7 +171,7 @@ if (!form) return; const type = form.dataset.questionType; - // ── Approval / DocumentReview: comment required when destructive option selected ── + // ── Approval: comment required when destructive option selected ── (function wireApproval() { const radios = form.querySelectorAll('input[type="radio"][name="approvalDecision"]'); if (radios.length === 0) return; @@ -202,9 +191,9 @@ }); })(); - // ── DocumentReview: at least one attachment confirmation ── + // ── Approval with attachments: at least one attachment confirmation ── (function wireDocReview() { - if (type !== 'documentReview') return; + if (type !== 'approval') return; const checkboxes = form.querySelectorAll('input[type="checkbox"][name="reviewedAttachmentIds"]'); if (checkboxes.length === 0) return; form.addEventListener('submit', function (e) { diff --git a/server/src/Dotbot.Server/Services/Delivery/NotificationSummaryBuilder.cs b/server/src/Dotbot.Server/Services/Delivery/NotificationSummaryBuilder.cs index 8483d01b..ea117084 100644 --- a/server/src/Dotbot.Server/Services/Delivery/NotificationSummaryBuilder.cs +++ b/server/src/Dotbot.Server/Services/Delivery/NotificationSummaryBuilder.cs @@ -29,8 +29,8 @@ public NotificationSummary Build( ? template.Project.ProjectId : template.Project.Name, // Legacy singleChoice templates carry no DeliverableSummary; providers render - // Title + Context as before. Validator enforces non-empty for approval / - // documentReview, so this stays null only when intentional (PRD §8 line 651). + // Title + Context as before. Validator enforces non-empty for approval with + // attachments, so this stays null only when intentional. DeliverableSummary = template.DeliverableSummary, Context = template.Context, Attachments = template.Attachments? diff --git a/server/src/Dotbot.Server/Validation/QuestionTemplateValidator.cs b/server/src/Dotbot.Server/Validation/QuestionTemplateValidator.cs index f2bc1648..ca273bbd 100644 --- a/server/src/Dotbot.Server/Validation/QuestionTemplateValidator.cs +++ b/server/src/Dotbot.Server/Validation/QuestionTemplateValidator.cs @@ -19,7 +19,7 @@ public QuestionTemplateValidator(IOptions se
CheckQuestionId,
CheckProjectId,
CheckType,
- CheckDeliverableSummary,
+ // CheckDeliverableSummary,
CheckOptionUniqueness,
CheckAttachments,
CheckReferenceLinks,
@@ -48,11 +48,16 @@ private IEnumerable CheckType(QuestionTemplate t)
yield return $"Unknown type '{t.Type}'. Allowed types: {string.Join(", ", QuestionTypes.AllowedTypes)}";
}
+ // TODO: unskip when outpost supports preparing summary of attachments
private IEnumerable CheckDeliverableSummary(QuestionTemplate t)
{
- if ((t.Type == QuestionTypes.Approval || t.Type == QuestionTypes.DocumentReview)
+ // Required only when an approval question carries attachments — that is the
+ // doc-review case where a reviewer needs a 1-3 line summary of what they're
+ // approving. Plain approvals (no doc) don't need it.
+ if (t.Type == QuestionTypes.Approval
+ && t.Attachments is { Count: > 0 }
&& string.IsNullOrWhiteSpace(t.DeliverableSummary))
- yield return $"deliverableSummary is required when type is '{t.Type}'";
+ yield return "deliverableSummary is required when type is 'approval' and attachments are present";
}
private IEnumerable CheckOptionUniqueness(QuestionTemplate t)
diff --git a/server/src/Dotbot.Server/Validation/RespondFormHandler.cs b/server/src/Dotbot.Server/Validation/RespondFormHandler.cs
index 80c1550b..e86c6ccb 100644
--- a/server/src/Dotbot.Server/Validation/RespondFormHandler.cs
+++ b/server/src/Dotbot.Server/Validation/RespondFormHandler.cs
@@ -39,9 +39,8 @@ public static RespondFormResult Validate(QuestionTemplate template, RespondFormI
return template.Type switch
{
- QuestionTypes.Approval => ValidateApproval(input),
+ QuestionTypes.Approval => ValidateApproval(template, input),
QuestionTypes.FreeText => ValidateFreeText(input),
- QuestionTypes.DocumentReview => ValidateDocumentReview(template, input),
QuestionTypes.PriorityRanking => ValidatePriorityRanking(template, input),
QuestionTypes.SingleChoice or QuestionTypes.MultiChoice
=> ValidateSingleOrMultiChoice(template, input),
@@ -49,44 +48,15 @@ QuestionTypes.SingleChoice or QuestionTypes.MultiChoice
};
}
- private static RespondFormResult ValidateApproval(RespondFormInput input)
+ private static RespondFormResult ValidateApproval(QuestionTemplate template, RespondFormInput input)
{
var decision = input.ApprovalDecision?.Trim().ToLowerInvariant();
if (string.IsNullOrEmpty(decision) || !ApprovalDecisions.ApprovalAllowed.Contains(decision))
- return RespondFormResult.Fail("Please choose Approve, Reject, or Abstain.");
+ return RespondFormResult.Fail("Please choose Approve or Reject.");
- if (decision == ApprovalDecisions.Reject && string.IsNullOrWhiteSpace(input.Comment))
+ if (decision == ApprovalDecisions.Rejected && string.IsNullOrWhiteSpace(input.Comment))
return RespondFormResult.Fail("A comment is required when rejecting.");
- return new RespondFormResult
- {
- ApprovalDecision = decision,
- Comment = NullIfBlank(input.Comment),
- SelectionLabel = ApprovalDecisions.Label(decision),
- };
- }
-
- private static RespondFormResult ValidateFreeText(RespondFormInput input)
- {
- if (string.IsNullOrWhiteSpace(input.FreeText))
- return RespondFormResult.Fail("Please type a response.");
-
- return new RespondFormResult
- {
- FreeText = input.FreeText.Trim(),
- SelectionLabel = "Free-text response",
- };
- }
-
- private static RespondFormResult ValidateDocumentReview(QuestionTemplate template, RespondFormInput input)
- {
- var decision = input.ApprovalDecision?.Trim().ToLowerInvariant();
- if (string.IsNullOrEmpty(decision) || !ApprovalDecisions.DocumentReviewAllowed.Contains(decision))
- return RespondFormResult.Fail("Please choose Approve, Request Changes, or Comment Only.");
-
- if (decision == ApprovalDecisions.RequestChanges && string.IsNullOrWhiteSpace(input.Comment))
- return RespondFormResult.Fail("A comment is required when requesting changes.");
-
var templateAttachments = template.Attachments ?? new List();
if (templateAttachments.Count > 0)
{
@@ -120,6 +90,18 @@ private static RespondFormResult ValidateDocumentReview(QuestionTemplate templat
};
}
+ private static RespondFormResult ValidateFreeText(RespondFormInput input)
+ {
+ if (string.IsNullOrWhiteSpace(input.FreeText))
+ return RespondFormResult.Fail("Please type a response.");
+
+ return new RespondFormResult
+ {
+ FreeText = input.FreeText.Trim(),
+ SelectionLabel = "Free-text response",
+ };
+ }
+
private static RespondFormResult ValidatePriorityRanking(QuestionTemplate template, RespondFormInput input)
{
var ranked = input.RankedItems?.Where(r => r is not null).ToList() ?? new List();
diff --git a/server/tests/Dotbot.Server.Tests/Integration/MagicLinkMiddlewareTests.cs b/server/tests/Dotbot.Server.Tests/Integration/MagicLinkMiddlewareTests.cs
index f5ee32c7..89efd283 100644
--- a/server/tests/Dotbot.Server.Tests/Integration/MagicLinkMiddlewareTests.cs
+++ b/server/tests/Dotbot.Server.Tests/Integration/MagicLinkMiddlewareTests.cs
@@ -41,7 +41,7 @@ public sealed class MagicLinkMiddlewareTests(DotbotApiFactory factory)
QuestionId = questionId,
Version = 1,
Title = $"Review {suffix}",
- Type = QuestionTypes.DocumentReview,
+ Type = QuestionTypes.Approval,
DeliverableSummary = "review",
Options = new(),
Project = new ProjectRef { ProjectId = ProjectId, Name = "MW" },
diff --git a/server/tests/Dotbot.Server.Tests/Unit/QuestionTemplateValidatorTests.cs b/server/tests/Dotbot.Server.Tests/Unit/QuestionTemplateValidatorTests.cs
index e1de9a28..2d924ab8 100644
--- a/server/tests/Dotbot.Server.Tests/Unit/QuestionTemplateValidatorTests.cs
+++ b/server/tests/Dotbot.Server.Tests/Unit/QuestionTemplateValidatorTests.cs
@@ -73,30 +73,39 @@ public void UnknownType_OneErrorListingAllowedValues()
[InlineData(QuestionTypes.MultiChoice)]
[InlineData(QuestionTypes.FreeText)]
[InlineData(QuestionTypes.PriorityRanking)]
+ [InlineData(QuestionTypes.Approval)]
public void TypeWithoutDeliverableSummaryRequirement_NoErrorWhenSummaryMissing(string type)
=> Assert.Empty(Validate(Template(type: type)));
- [Theory]
- [InlineData(QuestionTypes.Approval, null)]
- [InlineData(QuestionTypes.Approval, "")]
- [InlineData(QuestionTypes.Approval, " ")]
- [InlineData(QuestionTypes.DocumentReview, null)]
- [InlineData(QuestionTypes.DocumentReview, "")]
- [InlineData(QuestionTypes.DocumentReview, " ")]
- public void ApprovalOrDocumentReviewWithoutDeliverableSummary_OneError(string type, string? summary)
- {
- var errors = Validate(Template(type: type, deliverableSummary: summary));
+ // CheckDeliverableSummary is currently parked out of the validator
+ // rules array pending outpost support for preparing the summary of
+ // attachments. The skipped tests below document the intended behaviour
+ // and should be re-enabled once the rule is restored.
+ [Theory(Skip = "CheckDeliverableSummary parked; restore when outpost emits the summary")]
+ [InlineData(null)]
+ [InlineData("")]
+ [InlineData(" ")]
+ public void ApprovalWithAttachmentsAndNoDeliverableSummary_OneError(string? summary)
+ {
+ var errors = Validate(Template(
+ type: QuestionTypes.Approval,
+ deliverableSummary: summary,
+ attachments: [new QuestionAttachment { AttachmentId = Guid.NewGuid(), Name = "n", Url = "https://x" }]));
Assert.Single(errors);
Assert.Contains("deliverableSummary", errors[0]);
- Assert.Contains(type, errors[0]);
+ Assert.Contains("approval", errors[0]);
}
- [Theory]
- [InlineData(QuestionTypes.Approval)]
- [InlineData(QuestionTypes.DocumentReview)]
- public void ApprovalOrDocumentReviewWithDeliverableSummary_NoErrors(string type)
- => Assert.Empty(Validate(
- Template(type: type, deliverableSummary: "ship plan v1")));
+ [Fact]
+ public void ApprovalWithAttachmentsAndDeliverableSummary_NoErrors()
+ => Assert.Empty(Validate(Template(
+ type: QuestionTypes.Approval,
+ deliverableSummary: "ship plan v1",
+ attachments: [new QuestionAttachment { AttachmentId = Guid.NewGuid(), Name = "n", Url = "https://x" }])));
+
+ [Fact]
+ public void ApprovalWithoutAttachments_NoDeliverableSummaryRequired()
+ => Assert.Empty(Validate(Template(type: QuestionTypes.Approval)));
[Fact]
public void NullAttachments_NoErrors()
@@ -172,12 +181,13 @@ public void MultipleRulesFail_AllErrorsReturned()
Assert.Contains("bogus", errors[1]);
}
- [Fact]
- public void ProjectIdEmptyAndApprovalWithoutSummary_TwoErrors()
+ [Fact(Skip = "CheckDeliverableSummary parked; restore when outpost emits the summary")]
+ public void ProjectIdEmptyAndApprovalWithAttachmentsWithoutSummary_TwoErrors()
{
var errors = Validate(Template(
projectId: "",
- type: QuestionTypes.Approval));
+ type: QuestionTypes.Approval,
+ attachments: [new QuestionAttachment { AttachmentId = Guid.NewGuid(), Name = "n", Url = "https://x" }]));
Assert.Equal(2, errors.Count);
Assert.Contains("project.projectId", errors[0]);
Assert.Contains("deliverableSummary", errors[1]);
diff --git a/server/tests/Dotbot.Server.Tests/Unit/RespondFormHandlerTests.cs b/server/tests/Dotbot.Server.Tests/Unit/RespondFormHandlerTests.cs
index 34b03293..a999ad22 100644
--- a/server/tests/Dotbot.Server.Tests/Unit/RespondFormHandlerTests.cs
+++ b/server/tests/Dotbot.Server.Tests/Unit/RespondFormHandlerTests.cs
@@ -13,20 +13,20 @@ public class RespondFormHandlerTests
Type = type,
Options = options.ToList(),
Project = new ProjectRef { ProjectId = "p1" },
- DeliverableSummary = type is QuestionTypes.Approval or QuestionTypes.DocumentReview ? "deliverable" : null,
+ DeliverableSummary = type == QuestionTypes.Approval ? "deliverable" : null,
};
private static TemplateOption Option(string key = "A", string title = "Alpha")
=> new() { OptionId = Guid.NewGuid(), Key = key, Title = title };
- // ── Approval ───────────────────────────────────────────────────────────
+ // ── Approval (no attachments) ──────────────────────────────────────────
[Fact]
public void Approval_NoDecision_Fails()
{
var result = RespondFormHandler.Validate(Template(QuestionTypes.Approval), new RespondFormInput());
Assert.False(result.IsValid);
- Assert.Contains("Approve, Reject, or Abstain", result.Error);
+ Assert.Contains("Approve or Reject", result.Error);
}
[Fact]
@@ -39,43 +39,33 @@ public void Approval_UnknownDecision_Fails()
}
[Fact]
- public void Approval_Reject_RequiresComment()
+ public void Approval_Rejected_RequiresComment()
{
var result = RespondFormHandler.Validate(
Template(QuestionTypes.Approval),
- new RespondFormInput(ApprovalDecision: ApprovalDecisions.Reject, Comment: " "));
+ new RespondFormInput(ApprovalDecision: ApprovalDecisions.Rejected, Comment: " "));
Assert.False(result.IsValid);
Assert.Contains("comment is required", result.Error, StringComparison.OrdinalIgnoreCase);
}
[Fact]
- public void Approval_Reject_WithComment_Succeeds()
+ public void Approval_Rejected_WithComment_Succeeds()
{
var result = RespondFormHandler.Validate(
Template(QuestionTypes.Approval),
- new RespondFormInput(ApprovalDecision: ApprovalDecisions.Reject, Comment: "needs work"));
+ new RespondFormInput(ApprovalDecision: ApprovalDecisions.Rejected, Comment: "needs work"));
Assert.True(result.IsValid);
- Assert.Equal(ApprovalDecisions.Reject, result.ApprovalDecision);
+ Assert.Equal(ApprovalDecisions.Rejected, result.ApprovalDecision);
Assert.Equal("needs work", result.Comment);
Assert.Equal("Reject", result.SelectionLabel);
}
[Fact]
- public void Approval_Approve_NoCommentNeeded()
+ public void Approval_Approved_NoCommentNeeded()
{
var result = RespondFormHandler.Validate(
Template(QuestionTypes.Approval),
- new RespondFormInput(ApprovalDecision: ApprovalDecisions.Approve));
- Assert.True(result.IsValid);
- Assert.Null(result.Comment);
- }
-
- [Fact]
- public void Approval_Abstain_TrimsCommentNullIfBlank()
- {
- var result = RespondFormHandler.Validate(
- Template(QuestionTypes.Approval),
- new RespondFormInput(ApprovalDecision: ApprovalDecisions.Abstain, Comment: " "));
+ new RespondFormInput(ApprovalDecision: ApprovalDecisions.Approved));
Assert.True(result.IsValid);
Assert.Null(result.Comment);
}
@@ -85,9 +75,9 @@ public void Approval_DecisionCaseInsensitive_Normalised()
{
var result = RespondFormHandler.Validate(
Template(QuestionTypes.Approval),
- new RespondFormInput(ApprovalDecision: "APPROVE"));
+ new RespondFormInput(ApprovalDecision: "APPROVED"));
Assert.True(result.IsValid);
- Assert.Equal(ApprovalDecisions.Approve, result.ApprovalDecision);
+ Assert.Equal(ApprovalDecisions.Approved, result.ApprovalDecision);
}
// ── FreeText ───────────────────────────────────────────────────────────
@@ -112,56 +102,48 @@ public void FreeText_Trimmed_Succeeds()
Assert.Equal("hello", result.FreeText);
}
- // ── DocumentReview ─────────────────────────────────────────────────────
+ // ── Approval with attachments (formerly DocumentReview) ────────────────
[Fact]
- public void DocumentReview_NoDecision_Fails()
+ public void ApprovalWithAttachments_NoDecision_Fails()
{
- var template = Template(QuestionTypes.DocumentReview);
+ var template = Template(QuestionTypes.Approval);
+ template.Attachments = [new QuestionAttachment { AttachmentId = Guid.NewGuid(), Name = "spec", BlobPath = "x" }];
var result = RespondFormHandler.Validate(template, new RespondFormInput());
Assert.False(result.IsValid);
}
[Fact]
- public void DocumentReview_RequestChanges_RequiresComment()
+ public void ApprovalWithAttachments_Rejected_RequiresComment()
{
- var template = Template(QuestionTypes.DocumentReview);
+ var template = Template(QuestionTypes.Approval);
+ template.Attachments = [new QuestionAttachment { AttachmentId = Guid.NewGuid(), Name = "spec", BlobPath = "x" }];
var result = RespondFormHandler.Validate(template, new RespondFormInput(
- ApprovalDecision: ApprovalDecisions.RequestChanges));
+ ApprovalDecision: ApprovalDecisions.Rejected));
Assert.False(result.IsValid);
Assert.Contains("comment is required", result.Error, StringComparison.OrdinalIgnoreCase);
}
[Fact]
- public void DocumentReview_NoTemplateAttachments_AcceptsApproveWithoutReviewedIds()
- {
- var template = Template(QuestionTypes.DocumentReview);
- var result = RespondFormHandler.Validate(template, new RespondFormInput(
- ApprovalDecision: ApprovalDecisions.Approve));
- Assert.True(result.IsValid);
- Assert.Null(result.ReviewedAttachmentIds);
- }
-
- [Fact]
- public void DocumentReview_WithAttachments_RequiresAtLeastOneReviewed()
+ public void ApprovalWithAttachments_RequiresAtLeastOneReviewed()
{
- var template = Template(QuestionTypes.DocumentReview);
+ var template = Template(QuestionTypes.Approval);
template.Attachments = [new QuestionAttachment { AttachmentId = Guid.NewGuid(), Name = "spec", BlobPath = "x" }];
var result = RespondFormHandler.Validate(template, new RespondFormInput(
- ApprovalDecision: ApprovalDecisions.Approve,
+ ApprovalDecision: ApprovalDecisions.Approved,
ReviewedAttachmentIds: Array.Empty()));
Assert.False(result.IsValid);
Assert.Contains("reviewed at least one", result.Error, StringComparison.OrdinalIgnoreCase);
}
[Fact]
- public void DocumentReview_WithAttachments_FiltersUnknownIds()
+ public void ApprovalWithAttachments_FiltersUnknownIds()
{
var aid = Guid.NewGuid();
- var template = Template(QuestionTypes.DocumentReview);
+ var template = Template(QuestionTypes.Approval);
template.Attachments = [new QuestionAttachment { AttachmentId = aid, Name = "spec", BlobPath = "x" }];
var result = RespondFormHandler.Validate(template, new RespondFormInput(
- ApprovalDecision: ApprovalDecisions.Approve,
+ ApprovalDecision: ApprovalDecisions.Approved,
ReviewedAttachmentIds: new[] { aid, Guid.NewGuid() }));
Assert.True(result.IsValid);
Assert.Single(result.ReviewedAttachmentIds!);
@@ -169,25 +151,24 @@ public void DocumentReview_WithAttachments_FiltersUnknownIds()
}
[Fact]
- public void DocumentReview_OnlyUnknownIds_Fails()
+ public void ApprovalWithAttachments_OnlyUnknownIds_Fails()
{
- var template = Template(QuestionTypes.DocumentReview);
+ var template = Template(QuestionTypes.Approval);
template.Attachments = [new QuestionAttachment { AttachmentId = Guid.NewGuid(), Name = "spec", BlobPath = "x" }];
var result = RespondFormHandler.Validate(template, new RespondFormInput(
- ApprovalDecision: ApprovalDecisions.CommentOnly,
- Comment: "ok",
+ ApprovalDecision: ApprovalDecisions.Approved,
ReviewedAttachmentIds: new[] { Guid.NewGuid() }));
Assert.False(result.IsValid);
}
[Fact]
- public void DocumentReview_DuplicatesDeduped()
+ public void ApprovalWithAttachments_DuplicatesDeduped()
{
var aid = Guid.NewGuid();
- var template = Template(QuestionTypes.DocumentReview);
+ var template = Template(QuestionTypes.Approval);
template.Attachments = [new QuestionAttachment { AttachmentId = aid, Name = "spec", BlobPath = "x" }];
var result = RespondFormHandler.Validate(template, new RespondFormInput(
- ApprovalDecision: ApprovalDecisions.Approve,
+ ApprovalDecision: ApprovalDecisions.Approved,
ReviewedAttachmentIds: new[] { aid, aid, aid }));
Assert.True(result.IsValid);
Assert.Single(result.ReviewedAttachmentIds!);
diff --git a/tests/Test-Components.ps1 b/tests/Test-Components.ps1
index b6878965..f35c6045 100644
--- a/tests/Test-Components.ps1
+++ b/tests/Test-Components.ps1
@@ -3312,16 +3312,16 @@ if (Test-Path $notifModule) {
-Condition ($typedApproval.answer_type -eq 'approval') `
-Message "Expected answer_type=approval"
- # ConvertTo-TypedResponse: documentReview with attachments (metadata only)
+ # ConvertTo-TypedResponse: approval with attachments (metadata only)
$docReviewResp = [PSCustomObject]@{
- approvalDecision = 'changes_requested'
+ approvalDecision = 'rejected'
comment = 'see notes'
attachments = @(
[PSCustomObject]@{ name = 'notes.pdf'; sizeBytes = 1024; storageRef = 'sref-1'; description = 'reviewer notes' }
)
}
- $typedDoc = ConvertTo-TypedResponse -Response $docReviewResp -Type 'documentReview'
- Assert-True -Name "ConvertTo-TypedResponse documentReview includes attachment_refs metadata" `
+ $typedDoc = ConvertTo-TypedResponse -Response $docReviewResp -Type 'approval'
+ Assert-True -Name "ConvertTo-TypedResponse approval with attachments includes attachment_refs metadata" `
-Condition ($typedDoc.attachment_refs -and @($typedDoc.attachment_refs).Count -eq 1 -and $typedDoc.attachment_refs[0].storage_ref -eq 'sref-1') `
-Message "Expected attachment_refs[0].storage_ref=sref-1"
Assert-True -Name "ConvertTo-TypedResponse does NOT eager-download (no path field)" `
@@ -4115,13 +4115,12 @@ if ((Test-Path $mniMeta) -and (Test-Path $aqMeta)) {
-Condition ($threw -and $msg -match "ranked_items.*required") `
-Message "Expected throw on missing ranked_items, got: $msg"
- # documentReview with allowed decision should NOT throw on validation —
- # it will fail later at the task-not-found check, but only AFTER passing validation.
+ # approval with attachments (formerly documentReview) — rejected as a type
$threw = $false; $msg = ""
try { Invoke-TaskAnswerQuestion -Arguments @{ task_id='x'; type='documentReview'; decision='approved' } } catch { $threw = $true; $msg = $_.Exception.Message }
- Assert-True -Name "task-answer-question accepts documentReview decision=approved (passes validation)" `
- -Condition ($threw -and $msg -match "not found in needs-input") `
- -Message "Expected validation to pass and fail on task lookup, got: $msg"
+ Assert-True -Name "task-answer-question rejects deprecated documentReview type" `
+ -Condition ($threw -and $msg -match "Invalid 'type'") `
+ -Message "Expected throw on deprecated documentReview type, got: $msg"
# Invalid type
$threw = $false; $msg = ""
@@ -4175,12 +4174,12 @@ if ((Test-Path $mniMeta) -and (Test-Path $aqMeta)) {
-Condition ($threw -and $msg -match "'answer' is only valid") `
-Message "Expected throw, got: $msg"
- # changes_requested requires a comment — same requirement as rejected
+ # changes_requested was a documentReview value — now invalid on the merged approval type
$threw = $false; $msg = ""
- try { Invoke-TaskAnswerQuestion -Arguments @{ task_id='x'; type='documentReview'; decision='changes_requested' } } catch { $threw = $true; $msg = $_.Exception.Message }
- Assert-True -Name "task-answer-question rejects changes_requested without comment" `
- -Condition ($threw -and $msg -match "comment.*required") `
- -Message "Expected throw on missing comment for changes_requested, got: $msg"
+ try { Invoke-TaskAnswerQuestion -Arguments @{ task_id='x'; type='approval'; decision='changes_requested'; comment='ok' } } catch { $threw = $true; $msg = $_.Exception.Message }
+ Assert-True -Name "task-answer-question rejects legacy changes_requested decision on approval" `
+ -Condition ($threw -and $msg -match "Invalid 'decision'") `
+ -Message "Expected throw on legacy changes_requested decision, got: $msg"
# priorityRanking synthesis must normalize PSCustomObject ranked_items to strings —
# (verify the synthesis-time -join stays safe even for PSCustomObject input)
diff --git a/tests/Test-E2E-Mothership-QA.ps1 b/tests/Test-E2E-Mothership-QA.ps1
index 3444eafa..4312ab01 100644
--- a/tests/Test-E2E-Mothership-QA.ps1
+++ b/tests/Test-E2E-Mothership-QA.ps1
@@ -3,9 +3,9 @@
.SYNOPSIS
Layer 5: Mothership web UI E2E — Playwright against live server + Azurite.
.DESCRIPTION
- Runs Playwright specs that navigate the magic-link respond flow for all six
- question types: singleChoice, multiChoice, freeText, approval,
- documentReview, priorityRanking.
+ Runs Playwright specs that navigate the magic-link respond flow for all
+ question types: singleChoice, multiChoice, freeText, approval (with and
+ without attachments), priorityRanking.
For each question type the script:
1. POST /api/templates — creates a template
@@ -173,25 +173,13 @@ $questionTypes = @(
Submit = @{ selectedKey = "opt_a" }
}
@{
- Type = "approval"
- Title = "Playwright E2E: approval"
- DeliverableSummary = "Playwright E2E test deliverable for approval"
+ Type = "approval"
+ Title = "Playwright E2E: approval"
Options = @(
@{ key = "approve"; label = "Approve" }
@{ key = "reject"; label = "Reject" }
- @{ key = "abstain"; label = "Abstain" }
)
- Submit = @{ approvalDecision = "approve" }
- }
- @{
- Type = "documentReview"
- Title = "Playwright E2E: documentReview"
- DeliverableSummary = "Playwright E2E test deliverable for documentReview"
- Options = @(
- @{ key = "approve"; label = "Approve" }
- @{ key = "reject"; label = "Reject" }
- )
- Submit = @{ approvalDecision = "approve" }
+ Submit = @{ approvalDecision = "approved" }
}
@{
Type = "freeText"
diff --git a/tests/e2e-server/specs/mothership-question-flow.spec.ts b/tests/e2e-server/specs/mothership-question-flow.spec.ts
index 22e1bf4c..d9e139c3 100644
--- a/tests/e2e-server/specs/mothership-question-flow.spec.ts
+++ b/tests/e2e-server/specs/mothership-question-flow.spec.ts
@@ -58,16 +58,10 @@ for (const scenario of scenarios) {
if (scenario.type === "approval") {
await expect(
- page.locator('[value="approve"], [data-key="approve"]').first(),
+ page.locator('[value="approved"], [data-key="approve"]').first(),
).toBeVisible();
await expect(
- page.locator('[value="reject"], [data-key="reject"]').first(),
- ).toBeVisible();
- }
-
- if (scenario.type === "documentReview") {
- await expect(
- page.locator('[value="approve"], [data-key="approve"]').first(),
+ page.locator('[value="rejected"], [data-key="reject"]').first(),
).toBeVisible();
}
@@ -98,8 +92,8 @@ for (const scenario of scenarios) {
}
}
- if (scenario.type === "approval" || scenario.type === "documentReview") {
- const decision = scenario.submit.approvalDecision ?? "approve";
+ if (scenario.type === "approval") {
+ const decision = scenario.submit.approvalDecision ?? "approved";
const radio = page
.locator(`input[type="radio"][value="${decision}"]`)
.first();
@@ -151,8 +145,8 @@ for (const scenario of scenarios) {
injectBody.freeText = scenario.submit.freeText ?? "test answer";
} else if (scenario.type === "priorityRanking") {
injectBody.rankedItems = scenario.submit.rankedItems;
- } else if (scenario.type === "approval" || scenario.type === "documentReview") {
- injectBody.approvalDecision = scenario.submit.approvalDecision ?? "approve";
+ } else if (scenario.type === "approval") {
+ injectBody.approvalDecision = scenario.submit.approvalDecision ?? "approved";
} else {
injectBody.selectedKey = scenario.submit.selectedKey;
}
@@ -173,8 +167,8 @@ for (const scenario of scenarios) {
} else if (scenario.type === "priorityRanking") {
expect(Array.isArray(last.rankedItems)).toBeTruthy();
expect(last.rankedItems.length).toBeGreaterThan(0);
- } else if (scenario.type === "approval" || scenario.type === "documentReview") {
- expect(last.approvalDecision).toBe(scenario.submit.approvalDecision ?? "approve");
+ } else if (scenario.type === "approval") {
+ expect(last.approvalDecision).toBe(scenario.submit.approvalDecision ?? "approved");
} else if (scenario.submit.selectedKey) {
expect(last.selectedKey).toBe(scenario.submit.selectedKey);
}