From b4fd5e2f96ddb83435cb6f49a4cf58637fdd4ac8 Mon Sep 17 00:00:00 2001 From: cesnimda Date: Sat, 11 Jul 2026 21:10:39 +0200 Subject: [PATCH] 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 --- .../AttachmentFlagsRecomputeTests.cs | 111 ++++++++++++++++++ .../JobApplicationsEndpointBehaviorTests.cs | 10 +- .../Controllers/AttachmentsController.cs | 42 ++++++- .../Controllers/JobApplicationsController.cs | 23 +--- Models/JobApplication.cs | 4 +- job-tracker-ui/src/components/AddJobModal.tsx | 4 - .../src/components/EditJobDialog.tsx | 15 +-- 7 files changed, 165 insertions(+), 44 deletions(-) create mode 100644 JobTrackerApi.Tests/AttachmentFlagsRecomputeTests.cs diff --git a/JobTrackerApi.Tests/AttachmentFlagsRecomputeTests.cs b/JobTrackerApi.Tests/AttachmentFlagsRecomputeTests.cs new file mode 100644 index 0000000..6ad240f --- /dev/null +++ b/JobTrackerApi.Tests/AttachmentFlagsRecomputeTests.cs @@ -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(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(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 { ["Data:Root"] = tempRoot }) + .Build(); + + var env = new Mock(); + 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() } + }; + } +} diff --git a/JobTrackerApi.Tests/JobApplicationsEndpointBehaviorTests.cs b/JobTrackerApi.Tests/JobApplicationsEndpointBehaviorTests.cs index 95c0b8d..48aab5e 100644 --- a/JobTrackerApi.Tests/JobApplicationsEndpointBehaviorTests.cs +++ b/JobTrackerApi.Tests/JobApplicationsEndpointBehaviorTests.cs @@ -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, diff --git a/JobTrackerApi/Controllers/AttachmentsController.cs b/JobTrackerApi/Controllers/AttachmentsController.cs index b8942aa..fbf9146 100644 --- a/JobTrackerApi/Controllers/AttachmentsController.cs +++ b/JobTrackerApi/Controllers/AttachmentsController.cs @@ -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>> 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(); } diff --git a/JobTrackerApi/Controllers/JobApplicationsController.cs b/JobTrackerApi/Controllers/JobApplicationsController.cs index d25d5df..94f5dac 100644 --- a/JobTrackerApi/Controllers/JobApplicationsController.cs +++ b/JobTrackerApi/Controllers/JobApplicationsController.cs @@ -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; diff --git a/Models/JobApplication.cs b/Models/JobApplication.cs index 729c6ef..0849e62 100644 --- a/Models/JobApplication.cs +++ b/Models/JobApplication.cs @@ -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; diff --git a/job-tracker-ui/src/components/AddJobModal.tsx b/job-tracker-ui/src/components/AddJobModal.tsx index 68584af..fda2c1a 100644 --- a/job-tracker-ui/src/components/AddJobModal.tsx +++ b/job-tracker-ui/src/components/AddJobModal.tsx @@ -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) { diff --git a/job-tracker-ui/src/components/EditJobDialog.tsx b/job-tracker-ui/src/components/EditJobDialog.tsx index a4f7102..186550d 100644 --- a/job-tracker-ui/src/components/EditJobDialog.tsx +++ b/job-tracker-ui/src/components/EditJobDialog.tsx @@ -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) {t("editJobAttachmentsChecklist")} - + {/* Derived from actual uploaded attachments (see the Attachments panel) -- not + manually editable, so this can never drift from what's really attached. */} + - - - setHasResume(e.target.checked)} />} label={t("editJobResume")} /> - setHasCoverLetter(e.target.checked)} />} label={t("editJobCoverLetter")} /> - setHasPortfolio(e.target.checked)} />} label={t("editJobPortfolio")} /> - setHasOtherAttachment(e.target.checked)} />} label={t("editJobOtherAttachment")} /> + {hasOtherAttachment && }