JSA: Add signing period, contact log, and email export#7
Conversation
raymondjstone
commented
Apr 8, 2026
- Add JSA settings tab for signing period & work coach details
- Support "Current"/"Previous Signing Period" quick filters
- Allow standalone contact/discussion logging in JSA report
- Show contact details in report and exports (PDF/Excel/Word)
- Enable emailing PDF report to work coach (with attachment)
- Add RecordStandaloneContactDiscussion to JobHistoryService
- Improve PDF export (links, table headers, font resolver)
- Use sendBeacon for reliable shutdown API call
- Add unit tests for JSA period logic, reporting, and contact log
- Add JSA settings tab for signing period & work coach details - Support "Current"/"Previous Signing Period" quick filters - Allow standalone contact/discussion logging in JSA report - Show contact details in report and exports (PDF/Excel/Word) - Enable emailing PDF report to work coach (with attachment) - Add RecordStandaloneContactDiscussion to JobHistoryService - Improve PDF export (links, table headers, font resolver) - Use sendBeacon for reliable shutdown API call - Add unit tests for JSA period logic, reporting, and contact log
There was a problem hiding this comment.
Pull request overview
This PR expands the JSA reporting feature set by adding configurable signing-period settings, standalone contact/discussion logging, richer export output (PDF/Excel/Word), and the ability to email the PDF report to a configured work coach.
Changes:
- Add JSA settings (signing period + work coach contact details) and use signing-period quick filters in the JSA report.
- Support standalone contact/discussion history entries and surface contact details in the report + exports.
- Improve operational/export behavior (PDF link rendering + font resolver configured at startup; sendBeacon-based shutdown; HttpClientFactory usage in update banner) and add tests.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| wwwroot/js/crawlPagesRunner.js | Harden window opening (opener null) and make shutdown API call more reliable via sendBeacon + fetch keepalive fallback. |
| Services/JsaReportService.cs | Include contact/discussion in defaults, support standalone groups, expand Excel/Word export columns, add clickable links in PDF, and add a cached font resolver. |
| Services/JobHistoryService.cs | Add API to record standalone contact/discussion history entries. |
| Services/EmailService.cs | Add SMTP send method supporting attachments (for emailing exported PDFs). |
| Services/AppSettingsService.cs | Ensure decrypted settings copies include the new Jsa settings section. |
| Program.cs | Configure PDFsharp font resolver once at startup. |
| Models/JobHistory.cs | Add contact/discussion fields to history entries and add new HistoryActionType.ContactDiscussion. |
| Models/AppSettings.cs | Add JsaSettings to app settings model (signing period + work coach details). |
| Components/Pages/Settings.razor | Add a “JSA” settings tab to edit signing period and work coach contact details. |
| Components/Pages/JsaReport.razor | Add “Previous Signing Period” filter, standalone contact/discussion modal, email-to-work-coach flow, and display contact fields in UI. |
| Components/Layout/UpdateBanner.razor | Switch update trigger call to IHttpClientFactory and use a button instead of a JS href. |
| Components/Layout/NavMenu.razor | Remove inline icon style (icon styling appears to be handled in the .razor.css). |
| Components/Layout/MainLayout.razor | Remove unused injections/empty code block. |
| JobTracker.Tests/JsaSigningPeriodTests.cs | Add signing-period calculation tests (currently duplicated logic). |
| JobTracker.Tests/JsaReportServiceTests.cs | Add report grouping/filtering/summary tests (currently via a re-implemented test service). |
| JobTracker.Tests/JobHistoryServiceTests.cs | Add tests for standalone contact/discussion history recording. |
| JobTracker.Tests/HistoryActionTypeTests.cs | Add tests asserting the new enum value exists and is distinct. |
| Portable/JobTracker-x86.zip.001 | Update Git LFS pointer for portable artifact segment. |
| Portable/JobTracker-x86.zip.002 | Update Git LFS pointer for portable artifact segment. |
| Portable/JobTracker-x64.zip.001 | Update Git LFS pointer for portable artifact segment. |
| Portable/JobTracker-x64.zip.002 | Update Git LFS pointer for portable artifact segment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public class WindowsFontResolver : IFontResolver | ||
| { | ||
| private static readonly string FontDir = Environment.GetFolderPath(Environment.SpecialFolder.Fonts); | ||
| private static readonly ConcurrentDictionary<string, byte[]?> FontCache = new(); | ||
|
|
There was a problem hiding this comment.
ConcurrentDictionary<TKey,TValue> does not allow storing null values. Here the cache is declared as ConcurrentDictionary<string, byte[]?> and GetFont uses GetOrAdd, but the factory can return null when the font family/file isn't found, which will throw at runtime. Consider caching only non-null font byte arrays (e.g., TryGetValue + TryAdd when non-null), or use a non-null sentinel/Optional wrapper to represent “missing” entries safely.
| // Testable version that accepts our test double | ||
| private class TestableJsaReportService | ||
| { | ||
| private readonly JobHistoryService _historyService; | ||
| private readonly TestJobListingService _jobService; | ||
|
|
||
| public TestableJsaReportService(JobHistoryService historyService, TestJobListingService jobService) | ||
| { | ||
| _historyService = historyService; | ||
| _jobService = jobService; | ||
| } | ||
|
|
||
| public List<JsaReportGroup> GenerateReport(JsaReportFilter filter) | ||
| { |
There was a problem hiding this comment.
These tests validate a local TestableJsaReportService that re-implements JsaReportService logic rather than exercising the real production service. This risks the tests diverging from actual behavior (tests can pass while production breaks). Prefer refactoring JsaReportService to depend on an interface/abstraction for job lookups (or mock/stub JobListingService) so the test suite can instantiate and assert against the real JsaReportService implementation.
| // Helper methods that mirror the logic in JsaReport.razor | ||
| private static (DateTime from, DateTime to) GetCurrentSigningPeriod(JsaSettings settings, DateTime today) | ||
| { | ||
| if (settings.SigningStartDate.HasValue && settings.PeriodLengthDays > 0) | ||
| { | ||
| var start = settings.SigningStartDate.Value.Date; | ||
| var periodDays = settings.PeriodLengthDays; | ||
| var daysSinceStart = (today - start).Days; | ||
| if (daysSinceStart >= 0) | ||
| { | ||
| var currentPeriodIndex = daysSinceStart / periodDays; | ||
| var periodStart = start.AddDays(currentPeriodIndex * periodDays); | ||
| var periodEnd = periodStart.AddDays(periodDays - 1); | ||
| return (periodStart, periodEnd); | ||
| } | ||
| } | ||
| // Fallback: last 14 days | ||
| return (today.AddDays(-14), today); | ||
| } |
There was a problem hiding this comment.
The signing-period tests currently exercise helper methods defined inside the test class (“mirror the logic in JsaReport.razor”) rather than calling production code. This makes the tests ineffective at catching regressions in the actual UI logic. Consider moving signing-period calculations into a shared service/utility method used by JsaReport.razor and then unit-test that shared method instead of duplicating the algorithm in the test file.
| <div class="modal fade show" style="display: block; background: rgba(0,0,0,0.5);" tabindex="-1"> | ||
| <div class="modal-dialog"> | ||
| <div class="modal-content"> | ||
| <div class="modal-header"> | ||
| <h5 class="modal-title"><i class="bi bi-person-plus"></i> Add Contact/Discussion</h5> | ||
| <button type="button" class="btn-close" @onclick="CloseAddContactModal"></button> |
There was a problem hiding this comment.
The modal close button is missing an accessible label. Add aria-label="Close" (and ideally ensure the modal container has appropriate dialog semantics like role="dialog"/aria-modal). This aligns with the accessibility improvements already made for the dismissible alerts on this page.
| <div class="modal fade show" style="display: block; background: rgba(0,0,0,0.5);" tabindex="-1"> | |
| <div class="modal-dialog"> | |
| <div class="modal-content"> | |
| <div class="modal-header"> | |
| <h5 class="modal-title"><i class="bi bi-person-plus"></i> Add Contact/Discussion</h5> | |
| <button type="button" class="btn-close" @onclick="CloseAddContactModal"></button> | |
| <div class="modal fade show" style="display: block; background: rgba(0,0,0,0.5);" tabindex="-1" role="dialog" aria-modal="true" aria-labelledby="addContactModalTitle"> | |
| <div class="modal-dialog"> | |
| <div class="modal-content"> | |
| <div class="modal-header"> | |
| <h5 class="modal-title" id="addContactModalTitle"><i class="bi bi-person-plus"></i> Add Contact/Discussion</h5> | |
| <button type="button" class="btn-close" aria-label="Close" @onclick="CloseAddContactModal"></button> |