Skip to content

Refactor: centralize Add Contact modal, update versioning#12

Merged
raymondjstone merged 1 commit into
masterfrom
develop
Apr 9, 2026
Merged

Refactor: centralize Add Contact modal, update versioning#12
raymondjstone merged 1 commit into
masterfrom
develop

Conversation

@raymondjstone
Copy link
Copy Markdown
Owner

  • Introduced reusable AddContactDiscussionButton component for adding contacts/discussions, replacing duplicated modal logic in JsaReport and adding to Jobs, Contacts, and AddJob pages.
  • Supports prefilled fields and parent refresh callbacks for better UX.
  • Simplified JsaReport.razor by removing old modal/state.
  • Updated versioning scheme: build number is now the third field (v1.YY.Build.MMdd) for proper MSI upgrades.
  • Refreshed .zip artifacts with new builds.
  • Changed HTTP port in launchSettings.json from 8147 to 8148.

- Introduced reusable AddContactDiscussionButton component for adding contacts/discussions, replacing duplicated modal logic in JsaReport and adding to Jobs, Contacts, and AddJob pages.
- Supports prefilled fields and parent refresh callbacks for better UX.
- Simplified JsaReport.razor by removing old modal/state.
- Updated versioning scheme: build number is now the third field (v1.YY.Build.MMdd) for proper MSI upgrades.
- Refreshed .zip artifacts with new builds.
- Changed HTTP port in launchSettings.json from 8147 to 8148.
Copilot AI review requested due to automatic review settings April 9, 2026 14:04
@raymondjstone raymondjstone merged commit a1a3583 into master Apr 9, 2026
3 checks passed
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 9, 2026

Qodana for JVM

It seems all right 👌

No new problems were found according to the checks applied

☁️ View the detailed Qodana report

Contact Qodana team

Contact us at qodana-support@jetbrains.com

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR centralizes the “Add Contact/Discussion” modal into a reusable Blazor component and updates the build/versioning scheme to better support MSI upgrade behavior.

Changes:

  • Added a reusable AddContactDiscussionButton component and replaced duplicated modal logic (notably in JsaReport.razor), plus surfaced the button on Jobs/Contacts/Add Job pages.
  • Updated version/tag formatting to v1.YY.Build.MMdd so the MSI upgrade-relevant fields change per build.
  • Refreshed portable ZIP artifacts and adjusted development launch settings port configuration.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Properties/launchSettings.json Updates HTTP port in the https profile’s applicationUrl.
Portable/JobTracker-x86.zip.001 Updates Git LFS pointer for refreshed portable artifact.
Portable/JobTracker-x86.zip.002 Updates Git LFS pointer for refreshed portable artifact.
Portable/JobTracker-x64.zip.001 Updates Git LFS pointer for refreshed portable artifact.
Portable/JobTracker-x64.zip.002 Updates Git LFS pointer for refreshed portable artifact.
JobTracker.csproj Adjusts version field ordering to support MSI upgrade semantics.
Components/Shared/AddContactDiscussionButton.razor Introduces centralized Add Contact/Discussion modal + save behavior.
Components/Pages/JsaReport.razor Replaces in-page modal implementation with the shared component.
Components/Pages/Jobs.razor Adds shared component button and refresh callback after add.
Components/Pages/Contacts.razor Adds shared component button and reload callback after add.
Components/Pages/AddJob.razor Adds shared component button with prefilled title/company.
.github/workflows/dotnet.yml Updates release tag formatting to match the new version scheme.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +82 to +88
protected override void OnParametersSet()
{
if (!string.IsNullOrEmpty(PrefilledJobTitle))
jobTitle = PrefilledJobTitle;
if (!string.IsNullOrEmpty(PrefilledCompany))
company = PrefilledCompany;
}
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OnParametersSet assigns jobTitle/company whenever prefilled parameters are non-empty. Because Blazor re-applies parameters on parent re-renders, this can overwrite user edits while the modal is open (e.g., if the parent page re-renders for unrelated state changes). Consider only applying prefilled defaults when opening the modal (or only when showModal == false / the fields are still blank), so user-entered values aren’t unexpectedly reset.

Copilot uses AI. Check for mistakes.
@using JobTracker.Services
@inject JobHistoryService HistoryService

<button class="btn @ButtonClass" @onclick="() => showModal = true" title="@Title">
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trigger button doesn’t specify type="button". If this component is ever rendered inside a <form>/EditForm, the default type="submit" can cause unintended form submission. Setting type="button" on the trigger button avoids that class of bugs.

Suggested change
<button class="btn @ButtonClass" @onclick="() => showModal = true" title="@Title">
<button type="button" class="btn @ButtonClass" @onclick="() => showModal = true" title="@Title">

Copilot uses AI. Check for mistakes.

@if (showModal)
{
<div class="modal fade show" style="display: block; background: rgba(0,0,0,0.5);" tabindex="-1">
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most existing modals in the codebase use the pattern class="modal fade show d-block" (instead of relying on an inline display: block). For consistency with other pages (e.g., Jobs/Contacts modals), consider adding d-block here and using the same style conventions to reduce UI drift.

Suggested change
<div class="modal fade show" style="display: block; background: rgba(0,0,0,0.5);" tabindex="-1">
<div class="modal fade show d-block" style="background: rgba(0,0,0,0.5);" tabindex="-1">

Copilot uses AI. Check for mistakes.
Comment thread JobTracker.csproj
Comment on lines +12 to 14
CI uses github.run_number; local builds fall back to HHmm. -->
<BuildRevision Condition="'$(BUILD_NUMBER)' != ''">$(BUILD_NUMBER)</BuildRevision>
<BuildRevision Condition="'$(BUILD_NUMBER)' == ''">$([System.DateTime]::Now.ToString("HHmm"))</BuildRevision>
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MSI/Windows Installer version components are limited (notably the build field is max 65535). Using github.run_number directly in the 3rd field can eventually exceed this and break CI builds / MSI upgrades once the run number grows. Consider constraining BUILD_NUMBER to a safe range or switching to a monotonic value that’s guaranteed <= 65535 (e.g., derived from day-of-year + time bucket) while keeping the MSI upgrade semantics you want.

Suggested change
CI uses github.run_number; local builds fall back to HHmm. -->
<BuildRevision Condition="'$(BUILD_NUMBER)' != ''">$(BUILD_NUMBER)</BuildRevision>
<BuildRevision Condition="'$(BUILD_NUMBER)' == ''">$([System.DateTime]::Now.ToString("HHmm"))</BuildRevision>
CI uses github.run_number; local builds fall back to HHmm.
MSI limits each version component to 65535, so normalize the raw value
before placing it in the 3rd field. -->
<RawBuildRevision Condition="'$(BUILD_NUMBER)' != ''">$(BUILD_NUMBER)</RawBuildRevision>
<RawBuildRevision Condition="'$(BUILD_NUMBER)' == ''">$([System.DateTime]::Now.ToString("HHmm"))</RawBuildRevision>
<BuildRevision>$([System.Decimal]::ToInt32($([System.Decimal]::Remainder($([System.Decimal]::Parse('$(RawBuildRevision)')), 65535))))</BuildRevision>

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants