fix(jobs): derive attachment checklist flags from actual Attachments
Backlog item 4 (Wave 3, first sub-item). HasResume/HasCoverLetter/HasPortfolio/ HasOtherAttachment were manually-editable checkboxes in EditJobDialog, completely independent of whether a file was actually attached -- classic drift: mark 'resume ready' by hand, later delete the resume attachment, flag stays stuck true forever. User confirmed (asked directly, since removing the manual-override capability is a product decision, not purely technical): make them fully computed from Attachments, no manual override. - AttachmentsController.RecomputeAttachmentFlagsAsync: the single place these four fields get written now, called after every attachment mutation (upload, delete, Purpose change) that could affect them. Deliberately kept as persisted columns (not [NotMapped] computed properties reading the Attachments navigation collection) -- ~15 query sites build JobApplication DTOs without .Include(Attachments), so a live-computed property would silently return false everywhere instead of throwing, the worst kind of bug. Recomputing at the one write funnel avoids touching any read path. - Removed HasResume/etc from CreateJobApplicationRequest/ UpdateJobApplicationRequest -- no longer client-settable. - EditJobDialog: removed the manual checkboxes, kept the (now genuinely accurate) read-only status chips. - AddJobModal: stopped sending has*-flags at job-creation time; the follow-up attachment upload call now sets them correctly via the same recompute path. Caught a real bug while testing this: the Purpose-change path recomputed before saving the Purpose change, so a fresh query missed the pending edit and the flags never updated. Fixed by committing the mutation before recomputing. 3 new backend tests (purpose-change sets flag, delete clears flag, non-primary purpose counts as "other"). 172/172 backend, 25/25 frontend suites (57 tests) green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,111 @@
|
||||
using JobTrackerApi.Controllers;
|
||||
using JobTrackerApi.Data;
|
||||
using JobTrackerApi.Models;
|
||||
using JobTrackerApi.Services;
|
||||
using JobTrackerApi.Tests.TestSupport;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using Microsoft.Extensions.Configuration;
|
||||
using Microsoft.Extensions.Hosting;
|
||||
using Microsoft.AspNetCore.Http;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
using Moq;
|
||||
using Xunit;
|
||||
|
||||
namespace JobTrackerApi.Tests;
|
||||
|
||||
// JobApplication.HasResume/HasCoverLetter/HasPortfolio/HasOtherAttachment are derived from
|
||||
// Attachment rows (backlog Wave 3), not manually settable. These tests exercise the single
|
||||
// place they're written: AttachmentsController's Purpose-change and Delete paths.
|
||||
public sealed class AttachmentFlagsRecomputeTests
|
||||
{
|
||||
[Fact]
|
||||
public async Task Changing_purpose_to_resume_sets_HasResume()
|
||||
{
|
||||
await using var db = TestHostFactory.CreateInMemoryDb();
|
||||
var (job, attachment) = await SeedJobWithAttachmentAsync(db, purpose: "other");
|
||||
var controller = CreateController(db);
|
||||
|
||||
var result = await controller.Rename(attachment.Id, new AttachmentsController.UpdateAttachmentRequest(null, "resume", null), CancellationToken.None);
|
||||
|
||||
Assert.IsType<NoContentResult>(result);
|
||||
var updated = await db.JobApplications.SingleAsync(j => j.Id == job.Id);
|
||||
Assert.True(updated.HasResume);
|
||||
Assert.True(updated.HasOtherAttachment == false);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Deleting_the_only_resume_attachment_clears_HasResume()
|
||||
{
|
||||
await using var db = TestHostFactory.CreateInMemoryDb();
|
||||
var (job, attachment) = await SeedJobWithAttachmentAsync(db, purpose: "resume");
|
||||
var controller = CreateController(db);
|
||||
|
||||
var result = await controller.Delete(attachment.Id, CancellationToken.None);
|
||||
|
||||
Assert.IsType<NoContentResult>(result);
|
||||
var updated = await db.JobApplications.SingleAsync(j => j.Id == job.Id);
|
||||
Assert.False(updated.HasResume);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Attachment_with_case_study_purpose_counts_as_other()
|
||||
{
|
||||
await using var db = TestHostFactory.CreateInMemoryDb();
|
||||
var (job, attachment) = await SeedJobWithAttachmentAsync(db, purpose: "resume");
|
||||
var controller = CreateController(db);
|
||||
|
||||
await controller.Rename(attachment.Id, new AttachmentsController.UpdateAttachmentRequest(null, "case-study", null), CancellationToken.None);
|
||||
|
||||
var updated = await db.JobApplications.SingleAsync(j => j.Id == job.Id);
|
||||
Assert.False(updated.HasResume);
|
||||
Assert.True(updated.HasOtherAttachment);
|
||||
}
|
||||
|
||||
private static async Task<(JobApplication Job, Attachment Attachment)> SeedJobWithAttachmentAsync(JobTrackerContext db, string purpose)
|
||||
{
|
||||
var company = new Company { Name = "Acme", OwnerUserId = "user-1" };
|
||||
db.Companies.Add(company);
|
||||
await db.SaveChangesAsync();
|
||||
|
||||
var job = new JobApplication { JobTitle = "Backend Developer", CompanyId = company.Id, OwnerUserId = "user-1" };
|
||||
db.JobApplications.Add(job);
|
||||
await db.SaveChangesAsync();
|
||||
|
||||
var attachment = new Attachment
|
||||
{
|
||||
JobApplicationId = job.Id,
|
||||
FileName = "file.pdf",
|
||||
FilePath = Path.Combine(Path.GetTempPath(), $"jobtracker-attachment-test-{Guid.NewGuid():N}.pdf"),
|
||||
FileType = "application/pdf",
|
||||
FileSize = 100,
|
||||
Purpose = purpose,
|
||||
};
|
||||
db.Attachments.Add(attachment);
|
||||
await db.SaveChangesAsync();
|
||||
|
||||
job.HasResume = purpose == "resume";
|
||||
job.HasOtherAttachment = purpose is not ("resume" or "cover-letter" or "portfolio");
|
||||
await db.SaveChangesAsync();
|
||||
|
||||
return (job, attachment);
|
||||
}
|
||||
|
||||
private static AttachmentsController CreateController(JobTrackerContext db)
|
||||
{
|
||||
var tempRoot = Path.Combine(Path.GetTempPath(), $"jobtracker-attachments-tests-{Guid.NewGuid():N}");
|
||||
Directory.CreateDirectory(tempRoot);
|
||||
|
||||
var config = new ConfigurationBuilder()
|
||||
.AddInMemoryCollection(new Dictionary<string, string?> { ["Data:Root"] = tempRoot })
|
||||
.Build();
|
||||
|
||||
var env = new Mock<IHostEnvironment>();
|
||||
env.SetupGet(x => x.ContentRootPath).Returns(tempRoot);
|
||||
var paths = new AppPaths(config, env.Object);
|
||||
|
||||
return new AttachmentsController(paths, db)
|
||||
{
|
||||
ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() }
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -202,11 +202,7 @@ public sealed class JobApplicationsEndpointBehaviorTests
|
||||
CoverLetterText: null,
|
||||
JobUrl: null,
|
||||
DateApplied: null,
|
||||
FeedbackRequestedAt: null,
|
||||
HasResume: null,
|
||||
HasCoverLetter: null,
|
||||
HasPortfolio: null,
|
||||
HasOtherAttachment: null);
|
||||
FeedbackRequestedAt: null);
|
||||
|
||||
var result = await controller.Create(request, CancellationToken.None);
|
||||
|
||||
@@ -255,10 +251,6 @@ public sealed class JobApplicationsEndpointBehaviorTests
|
||||
SalaryPeriod: "fortnight",
|
||||
NextAction: null,
|
||||
FollowUpAt: null,
|
||||
HasResume: null,
|
||||
HasCoverLetter: null,
|
||||
HasPortfolio: null,
|
||||
HasOtherAttachment: null,
|
||||
Notes: null,
|
||||
Description: null,
|
||||
TranslatedDescription: null,
|
||||
|
||||
@@ -59,6 +59,26 @@ namespace JobTrackerApi.Controllers
|
||||
return "other";
|
||||
}
|
||||
|
||||
// JobApplication.HasResume/HasCoverLetter/HasPortfolio/HasOtherAttachment are derived
|
||||
// from actual Attachment rows, not manually settable -- this is the single place they're
|
||||
// written, called after every attachment mutation (upload/delete/purpose change) so they
|
||||
// can never drift from what's actually attached.
|
||||
private async Task RecomputeAttachmentFlagsAsync(int jobId, CancellationToken cancellationToken)
|
||||
{
|
||||
var job = await _db.JobApplications.FirstOrDefaultAsync(j => j.Id == jobId, cancellationToken);
|
||||
if (job is null) return;
|
||||
|
||||
var purposes = await _db.Attachments
|
||||
.Where(a => a.JobApplicationId == jobId)
|
||||
.Select(a => a.Purpose)
|
||||
.ToListAsync(cancellationToken);
|
||||
|
||||
job.HasResume = purposes.Any(p => p == "resume");
|
||||
job.HasCoverLetter = purposes.Any(p => p == "cover-letter");
|
||||
job.HasPortfolio = purposes.Any(p => p == "portfolio");
|
||||
job.HasOtherAttachment = purposes.Any(p => p is not ("resume" or "cover-letter" or "portfolio"));
|
||||
}
|
||||
|
||||
[HttpGet("{jobId:int}")]
|
||||
public async Task<ActionResult<List<AttachmentDto>>> ListForJob([FromRoute] int jobId, CancellationToken cancellationToken)
|
||||
{
|
||||
@@ -102,15 +122,23 @@ namespace JobTrackerApi.Controllers
|
||||
att.UseForAi = request.UseForAi.Value;
|
||||
}
|
||||
|
||||
if (!string.IsNullOrWhiteSpace(request.Purpose))
|
||||
var purposeChanged = !string.IsNullOrWhiteSpace(request.Purpose);
|
||||
if (purposeChanged)
|
||||
{
|
||||
att.Purpose = request.Purpose.Trim().ToLowerInvariant();
|
||||
att.Purpose = request.Purpose!.Trim().ToLowerInvariant();
|
||||
}
|
||||
|
||||
var rawName = (request.FileName ?? string.Empty).Trim();
|
||||
if (rawName.Length == 0)
|
||||
{
|
||||
await _db.SaveChangesAsync(cancellationToken);
|
||||
if (purposeChanged)
|
||||
{
|
||||
// Recompute needs the Purpose change committed first -- a fresh query
|
||||
// wouldn't see the pending change yet.
|
||||
await RecomputeAttachmentFlagsAsync(att.JobApplicationId, cancellationToken);
|
||||
await _db.SaveChangesAsync(cancellationToken);
|
||||
}
|
||||
return NoContent();
|
||||
}
|
||||
|
||||
@@ -130,6 +158,11 @@ namespace JobTrackerApi.Controllers
|
||||
att.FileName = name;
|
||||
att.FilePath = newPath;
|
||||
await _db.SaveChangesAsync(cancellationToken);
|
||||
if (purposeChanged)
|
||||
{
|
||||
await RecomputeAttachmentFlagsAsync(att.JobApplicationId, cancellationToken);
|
||||
await _db.SaveChangesAsync(cancellationToken);
|
||||
}
|
||||
|
||||
return NoContent();
|
||||
}
|
||||
@@ -141,8 +174,11 @@ namespace JobTrackerApi.Controllers
|
||||
if (att is null) return NotFound();
|
||||
|
||||
var path = att.FilePath;
|
||||
var jobId = att.JobApplicationId;
|
||||
_db.Attachments.Remove(att);
|
||||
await _db.SaveChangesAsync(cancellationToken);
|
||||
await RecomputeAttachmentFlagsAsync(jobId, cancellationToken);
|
||||
await _db.SaveChangesAsync(cancellationToken);
|
||||
|
||||
try
|
||||
{
|
||||
@@ -200,6 +236,8 @@ namespace JobTrackerApi.Controllers
|
||||
});
|
||||
}
|
||||
|
||||
await _db.SaveChangesAsync(cancellationToken);
|
||||
await RecomputeAttachmentFlagsAsync(jobId, cancellationToken);
|
||||
await _db.SaveChangesAsync(cancellationToken);
|
||||
return Ok();
|
||||
}
|
||||
|
||||
@@ -1376,11 +1376,7 @@ Canonical profile:
|
||||
string? CoverLetterText,
|
||||
string? JobUrl,
|
||||
DateTime? DateApplied,
|
||||
DateTime? FeedbackRequestedAt,
|
||||
bool? HasResume,
|
||||
bool? HasCoverLetter,
|
||||
bool? HasPortfolio,
|
||||
bool? HasOtherAttachment
|
||||
DateTime? FeedbackRequestedAt
|
||||
);
|
||||
|
||||
private static (decimal? Min, decimal? Max, string? Currency, string? Period) NormalizeSalary(
|
||||
@@ -1422,10 +1418,9 @@ Canonical profile:
|
||||
NextAction = string.IsNullOrWhiteSpace(request.NextAction) ? null : request.NextAction.Trim(),
|
||||
FollowUpAt = request.FollowUpAt,
|
||||
FeedbackRequestedAt = request.FeedbackRequestedAt,
|
||||
HasResume = request.HasResume ?? false,
|
||||
HasCoverLetter = request.HasCoverLetter ?? false,
|
||||
HasPortfolio = request.HasPortfolio ?? false,
|
||||
HasOtherAttachment = request.HasOtherAttachment ?? false,
|
||||
// HasResume/HasCoverLetter/HasPortfolio/HasOtherAttachment are derived from
|
||||
// Attachment rows (see AttachmentsController.RecomputeAttachmentFlagsAsync), not
|
||||
// settable here -- they start false and get set correctly once files are uploaded.
|
||||
Notes = string.IsNullOrWhiteSpace(request.Notes) ? null : request.Notes,
|
||||
Description = string.IsNullOrWhiteSpace(request.Description) ? null : request.Description,
|
||||
TranslatedDescription = string.IsNullOrWhiteSpace(request.TranslatedDescription) ? null : request.TranslatedDescription,
|
||||
@@ -1486,10 +1481,6 @@ Canonical profile:
|
||||
string? SalaryPeriod,
|
||||
string? NextAction,
|
||||
DateTime? FollowUpAt,
|
||||
bool? HasResume,
|
||||
bool? HasCoverLetter,
|
||||
bool? HasPortfolio,
|
||||
bool? HasOtherAttachment,
|
||||
string? Notes,
|
||||
string? Description,
|
||||
string? TranslatedDescription,
|
||||
@@ -1529,10 +1520,8 @@ Canonical profile:
|
||||
job.NextAction = string.IsNullOrWhiteSpace(request.NextAction) ? null : request.NextAction.Trim();
|
||||
job.FollowUpAt = request.FollowUpAt;
|
||||
job.FeedbackRequestedAt = request.FeedbackRequestedAt;
|
||||
if (request.HasResume is not null) job.HasResume = request.HasResume.Value;
|
||||
if (request.HasCoverLetter is not null) job.HasCoverLetter = request.HasCoverLetter.Value;
|
||||
if (request.HasPortfolio is not null) job.HasPortfolio = request.HasPortfolio.Value;
|
||||
if (request.HasOtherAttachment is not null) job.HasOtherAttachment = request.HasOtherAttachment.Value;
|
||||
// HasResume/HasCoverLetter/HasPortfolio/HasOtherAttachment are derived from
|
||||
// Attachment rows, not settable here -- see AttachmentsController.RecomputeAttachmentFlagsAsync.
|
||||
job.Notes = request.Notes;
|
||||
job.Description = request.Description;
|
||||
job.TranslatedDescription = request.TranslatedDescription;
|
||||
|
||||
@@ -24,7 +24,9 @@ public class JobApplication
|
||||
public DateTime? FeedbackRequestedAt { get; set; }
|
||||
public string? RecruiterMessageDraft { get; set; }
|
||||
|
||||
// Attachment checklist
|
||||
// Attachment checklist. Derived from Attachment rows, not directly settable by API
|
||||
// consumers -- see AttachmentsController.RecomputeAttachmentFlagsAsync, the single place
|
||||
// these are written, so they can't drift from what's actually attached.
|
||||
public bool HasResume { get; set; } = false;
|
||||
public bool HasCoverLetter { get; set; } = false;
|
||||
public bool HasPortfolio { get; set; } = false;
|
||||
|
||||
@@ -327,10 +327,6 @@ export default function AddJobModal({ open, onClose, onCreated, initialUrl }: Pr
|
||||
notes,
|
||||
coverLetterText: null,
|
||||
dateApplied,
|
||||
hasResume: attachments.resume.length > 0,
|
||||
hasCoverLetter: attachments.coverLetter.length > 0,
|
||||
hasPortfolio: attachments.portfolio.length > 0,
|
||||
hasOtherAttachment: attachments.other.length > 0,
|
||||
});
|
||||
|
||||
if (response.data?.id && attachmentCount > 0) {
|
||||
|
||||
@@ -158,10 +158,6 @@ export default function EditJobDialog({ open, jobId, onClose, onSaved }: Props)
|
||||
salaryPeriod: salaryPeriod || null,
|
||||
nextAction: nextAction.trim() || null,
|
||||
followUpAt: followUpAt || null,
|
||||
hasResume,
|
||||
hasCoverLetter,
|
||||
hasPortfolio,
|
||||
hasOtherAttachment,
|
||||
notes: notes || null,
|
||||
description: description || null,
|
||||
translatedDescription: translatedDescription || null,
|
||||
@@ -243,16 +239,13 @@ export default function EditJobDialog({ open, jobId, onClose, onSaved }: Props)
|
||||
|
||||
<Paper variant="outlined" sx={{ p: 2 }}>
|
||||
<Typography variant="overline" sx={{ color: "text.secondary" }}>{t("editJobAttachmentsChecklist")}</Typography>
|
||||
<Box sx={{ display: "flex", gap: 1, flexWrap: "wrap", mt: 1, mb: 1.5 }}>
|
||||
{/* Derived from actual uploaded attachments (see the Attachments panel) -- not
|
||||
manually editable, so this can never drift from what's really attached. */}
|
||||
<Box sx={{ display: "flex", gap: 1, flexWrap: "wrap", mt: 1 }}>
|
||||
<Chip size="small" label={hasResume ? t("editJobResumeReady") : t("editJobResumeMissing")} color={hasResume ? "success" : "default"} variant={hasResume ? "filled" : "outlined"} />
|
||||
<Chip size="small" label={hasCoverLetter ? t("editJobCoverLetterReady") : t("editJobCoverLetterMissing")} color={hasCoverLetter ? "success" : "default"} variant={hasCoverLetter ? "filled" : "outlined"} />
|
||||
<Chip size="small" label={hasPortfolio ? t("editJobPortfolioReady") : t("editJobPortfolioOptional")} color={hasPortfolio ? "success" : "default"} variant={hasPortfolio ? "filled" : "outlined"} />
|
||||
</Box>
|
||||
<Box sx={{ display: "flex", gap: 2, flexWrap: "wrap", mt: 1 }}>
|
||||
<FormControlLabel control={<Checkbox checked={hasResume} onChange={(e) => setHasResume(e.target.checked)} />} label={t("editJobResume")} />
|
||||
<FormControlLabel control={<Checkbox checked={hasCoverLetter} onChange={(e) => setHasCoverLetter(e.target.checked)} />} label={t("editJobCoverLetter")} />
|
||||
<FormControlLabel control={<Checkbox checked={hasPortfolio} onChange={(e) => setHasPortfolio(e.target.checked)} />} label={t("editJobPortfolio")} />
|
||||
<FormControlLabel control={<Checkbox checked={hasOtherAttachment} onChange={(e) => setHasOtherAttachment(e.target.checked)} />} label={t("editJobOtherAttachment")} />
|
||||
{hasOtherAttachment && <Chip size="small" label={t("editJobOtherAttachment")} color="success" variant="filled" />}
|
||||
</Box>
|
||||
</Paper>
|
||||
</Box>
|
||||
|
||||
Reference in New Issue
Block a user