fix(email): gate draft attempt rotation
Rotate persisted delivery identity only after the matching owner attempt is definitively failed. Keep stale, foreign, pending, sent, and uncertain drafts non-retryable.
This commit is contained in:
@@ -105,6 +105,42 @@ public sealed class EmailDraftsControllerTests
|
||||
Assert.Equal("user-2", remaining.OwnerUserId);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task New_attempt_identity_requires_the_matching_definitive_failure()
|
||||
{
|
||||
await using var fixture = await Fixture.CreateAsync(seedDrafts: true);
|
||||
await using var db = fixture.Context("user-1");
|
||||
var controller = Controller(db, "user-1");
|
||||
var original = await db.EmailDrafts.AsNoTracking().SingleAsync();
|
||||
|
||||
var refused = await controller.NewAttempt(original.Id, new EmailDraftsController.NewAttemptRequest(1), default);
|
||||
Assert.IsType<ConflictObjectResult>(refused.Result);
|
||||
db.EmailSendAttempts.Add(new EmailSendAttempt
|
||||
{
|
||||
Id = Guid.NewGuid(),
|
||||
OwnerUserId = "user-1",
|
||||
JobApplicationId = original.JobApplicationId,
|
||||
Provider = original.Provider,
|
||||
ClientRequestId = original.ClientRequestId,
|
||||
PayloadHash = new string('a', 64),
|
||||
Status = EmailSendStatuses.Failed,
|
||||
FailureCategory = "provider_rejected",
|
||||
CreatedAtUtc = DateTime.UtcNow,
|
||||
CompletedAtUtc = DateTime.UtcNow,
|
||||
});
|
||||
await db.SaveChangesAsync();
|
||||
|
||||
var rotatedResult = await controller.NewAttempt(original.Id, new EmailDraftsController.NewAttemptRequest(1), default);
|
||||
var rotated = Assert.IsType<EmailDraftsController.DraftDto>(Assert.IsType<OkObjectResult>(rotatedResult.Result).Value);
|
||||
Assert.Equal(2, rotated.Revision);
|
||||
Assert.NotEqual(original.ClientRequestId, rotated.ClientRequestId);
|
||||
Assert.True(Guid.TryParse(rotated.ClientRequestId, out _));
|
||||
|
||||
Assert.IsType<ConflictObjectResult>((await controller.NewAttempt(original.Id, new EmailDraftsController.NewAttemptRequest(1), default)).Result);
|
||||
await using var otherDb = fixture.Context("user-2");
|
||||
Assert.IsType<NotFoundResult>((await Controller(otherDb, "user-2").NewAttempt(original.Id, new EmailDraftsController.NewAttemptRequest(2), default)).Result);
|
||||
}
|
||||
|
||||
private static EmailDraftsController Controller(JobTrackerContext db, string userId)
|
||||
{
|
||||
var controller = new EmailDraftsController(
|
||||
|
||||
@@ -39,6 +39,7 @@ public sealed class EmailDraftsController(
|
||||
string? ThreadId);
|
||||
|
||||
public sealed record UpdateDraftRequest(long Revision, string? To, string? Subject, string? BodyText);
|
||||
public sealed record NewAttemptRequest(long Revision);
|
||||
|
||||
[HttpGet]
|
||||
public async Task<ActionResult<IReadOnlyList<DraftDto>>> List(
|
||||
@@ -165,6 +166,40 @@ public sealed class EmailDraftsController(
|
||||
: NotFound();
|
||||
}
|
||||
|
||||
[HttpPost("{id:guid}/new-attempt")]
|
||||
public async Task<ActionResult<DraftDto>> NewAttempt(Guid id, NewAttemptRequest request, CancellationToken cancellationToken)
|
||||
{
|
||||
var ownerUserId = GetOwnerUserId();
|
||||
if (ownerUserId is null) return Unauthorized();
|
||||
if (request.Revision <= 0) return BadRequest("A positive revision is required.");
|
||||
|
||||
var draft = await db.EmailDrafts.AsNoTracking()
|
||||
.FirstOrDefaultAsync(item => item.Id == id && item.OwnerUserId == ownerUserId, cancellationToken);
|
||||
if (draft is null) return NotFound();
|
||||
if (draft.Revision != request.Revision)
|
||||
return Conflict(new ProblemDetails { Title = "Draft revision conflict", Detail = "Reload the latest draft before creating a new attempt." });
|
||||
|
||||
var failedAttempt = await db.EmailSendAttempts.AsNoTracking().AnyAsync(attempt =>
|
||||
attempt.OwnerUserId == ownerUserId &&
|
||||
attempt.ClientRequestId == draft.ClientRequestId &&
|
||||
attempt.Status == EmailSendStatuses.Failed,
|
||||
cancellationToken);
|
||||
if (!failedAttempt)
|
||||
return Conflict(new ProblemDetails { Title = "A new attempt is not allowed", Detail = "Only a definitively failed delivery can receive a new attempt identity." });
|
||||
|
||||
var newClientRequestId = Guid.NewGuid().ToString("D");
|
||||
var now = timeProvider.GetUtcNow().UtcDateTime;
|
||||
var affected = await db.EmailDrafts
|
||||
.Where(item => item.Id == id && item.OwnerUserId == ownerUserId && item.Revision == request.Revision && item.ClientRequestId == draft.ClientRequestId)
|
||||
.ExecuteUpdateAsync(setters => setters
|
||||
.SetProperty(item => item.ClientRequestId, newClientRequestId)
|
||||
.SetProperty(item => item.Revision, item => item.Revision + 1)
|
||||
.SetProperty(item => item.UpdatedAtUtc, now), cancellationToken);
|
||||
if (affected != 1)
|
||||
return Conflict(new ProblemDetails { Title = "Draft revision conflict", Detail = "Reload the latest draft before creating a new attempt." });
|
||||
return await Get(id, cancellationToken);
|
||||
}
|
||||
|
||||
private string? GetOwnerUserId() =>
|
||||
User.FindFirstValue(ClaimTypes.NameIdentifier) ?? User.FindFirstValue("sub");
|
||||
|
||||
|
||||
@@ -287,6 +287,40 @@ describe('CorrespondenceInboxPage', () => {
|
||||
expect(screen.getByLabelText(/message/i)).toHaveValue('Conflicting local edit.');
|
||||
});
|
||||
|
||||
test('persists a new delivery identity only after a definitive failed attempt', async () => {
|
||||
const original = mockedApi.get.getMockImplementation();
|
||||
const stored = {
|
||||
id: 'draft-1', jobApplicationId: 42, provider: 'gmail', to: 'maria@acme.test', subject: 'Saved subject', bodyText: 'Saved private draft.',
|
||||
threadId: 'thread-1', clientRequestId: 'failed-request-id', revision: 4, createdAtUtc: new Date().toISOString(), updatedAtUtc: new Date().toISOString(),
|
||||
};
|
||||
mockedApi.get.mockImplementation((url: string, config?: any) => {
|
||||
if (url === '/email/drafts') return Promise.resolve({ data: [stored] } as any);
|
||||
if (url === '/email/providers') return Promise.resolve({ data: [
|
||||
{ provider: 'gmail', displayName: 'Gmail', connected: true, address: 'owner@gmail.test', canRead: true, canSend: true },
|
||||
] } as any);
|
||||
return original!(url, config);
|
||||
});
|
||||
mockedApi.post.mockImplementation((url: string) => {
|
||||
if (url === '/email/send') return Promise.reject({ response: { status: 502, data: {
|
||||
attemptId: 'attempt-1', status: 'failed', duplicate: false, failureCategory: 'provider_rejected',
|
||||
} } });
|
||||
if (url === '/email/drafts/draft-1/new-attempt') return Promise.resolve({ data: {
|
||||
...stored, revision: 5, clientRequestId: 'new-server-request-id', updatedAtUtc: new Date().toISOString(),
|
||||
} } as any);
|
||||
return Promise.reject(new Error(`Unexpected POST ${url}`));
|
||||
});
|
||||
|
||||
renderPage();
|
||||
fireEvent.click(await screen.findByRole('button', { name: /resume saved subject/i }));
|
||||
fireEvent.click(screen.getByRole('button', { name: /review and send/i }));
|
||||
fireEvent.click(await screen.findByRole('button', { name: /^send email$/i }));
|
||||
fireEvent.click(await screen.findByRole('button', { name: /prepare new attempt/i }));
|
||||
|
||||
await waitFor(() => expect(mockedApi.post).toHaveBeenCalledWith('/email/drafts/draft-1/new-attempt', { revision: 4 }));
|
||||
await waitFor(() => expect(screen.queryByText(/provider confirmed that this attempt did not complete/i)).not.toBeInTheDocument());
|
||||
expect(screen.getByRole('button', { name: /review and send/i })).toBeEnabled();
|
||||
});
|
||||
|
||||
test('does not offer retry when delivery is uncertain', async () => {
|
||||
const original = mockedApi.get.getMockImplementation();
|
||||
mockedApi.get.mockImplementation((url: string, config?: any) => {
|
||||
|
||||
@@ -388,8 +388,23 @@ export default function CorrespondenceInboxPage() {
|
||||
}
|
||||
};
|
||||
|
||||
const prepareNewAttempt = () => {
|
||||
setDraft((current) => current ? { ...current, clientRequestId: globalThis.crypto.randomUUID() } : null);
|
||||
const prepareNewAttempt = async () => {
|
||||
if (!draft || sendResult?.status !== "failed") return;
|
||||
setDraftError(null);
|
||||
if (draft.id && draft.revision) {
|
||||
try {
|
||||
const response = await api.post<StoredEmailDraft>(`/email/drafts/${draft.id}/new-attempt`, { revision: draft.revision });
|
||||
setDraft((current) => current ? { ...current, ...response.data } : null);
|
||||
await loadStoredDrafts();
|
||||
} catch (error: any) {
|
||||
setDraftError(error?.response?.status === 409
|
||||
? "A new attempt is allowed only after the latest saved draft has a definitive failed delivery. Reload the draft before continuing."
|
||||
: getApiErrorMessage(error, "Failed to prepare a new delivery attempt."));
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
setDraft((current) => current ? { ...current, clientRequestId: globalThis.crypto.randomUUID() } : null);
|
||||
}
|
||||
setSendResult(null);
|
||||
setSendError(null);
|
||||
};
|
||||
@@ -524,7 +539,7 @@ export default function CorrespondenceInboxPage() {
|
||||
<Alert severity="warning" sx={{ mt: 1.5 }}>Delivery status is uncertain. Do not retry this draft. Check the provider Sent folder before taking any further action.</Alert>
|
||||
) : null}
|
||||
{sendResult?.status === "failed" ? (
|
||||
<Alert severity="error" sx={{ mt: 1.5 }} action={<Button color="inherit" size="small" onClick={prepareNewAttempt}>Prepare new attempt</Button>}>
|
||||
<Alert severity="error" sx={{ mt: 1.5 }} action={<Button color="inherit" size="small" onClick={() => void prepareNewAttempt()}>Prepare new attempt</Button>}>
|
||||
The provider confirmed that this attempt did not complete. Review the connection and draft before creating a new attempt.
|
||||
</Alert>
|
||||
) : null}
|
||||
|
||||
Reference in New Issue
Block a user