-
Notifications
You must be signed in to change notification settings - Fork 3
Test #1959
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test #1959
Conversation
- Enhanced DataTable state restoration logic to better handle column search and visibility, added robust error handling for state corruption - Improved global export of utility functions in table-utils.js - Added a check in GrantApplications/Index.js to ensure createNumberFormatter is loaded before use
- Add specific styling for the .cstm-save-view dropdown menu and its empty states, including border and padding adjustments to improve visual consistency
…8-Fix-Sonarqube-Errors
Bugfix/ab#31378 fix sonarqube errors
…om/bcgov/Unity into feature/AB#31366-SonarQubeTechDebt
AB#31628 remove obsolete and add replacement for breadcrumb
…-security-patch AB#31640 npm package update to lodash 4.17.23 (latest)
feature/AB#29486-RemoveSeriLog
…arnings feature/AB#31361 - Update NPM Packages
…e-InvalidCastException AB#31644: ChesHttpStatusCode InvalidCastException Fix
…on-datetime-format AB#31599: Use Pacific Timezone for all dates in FSB-AP email
…nmodule AB#31636 remove unused emailsender registration
…ectoral AB#31647 drop old Electoral District column
…rojectApplicantInfoAsync AB#31641 - remove dead code - UpdateProjectApplicantInfoAsync
| newApplicantAgent.IdentityName = intakeMap.ApplicantAgent?.name ?? ""; | ||
| newApplicantAgent.IdentityEmail = intakeMap.ApplicantAgent?.email ?? ""; | ||
|
|
||
| newApplicantAgent.OidcSubUser = intakeMap.ApplicantAgent?.oidc_sub_user; |
| newApplicantAgent.IdentityEmail = intakeMap.ApplicantAgent?.email ?? ""; | ||
|
|
||
| newApplicantAgent.OidcSubUser = intakeMap.ApplicantAgent?.oidc_sub_user; | ||
| newApplicantAgent.IdentityProvider = intakeMap.ApplicantAgent?.identity_provider ?? ""; |
| return indigenousOrgInd switch | ||
| { | ||
| true => "Yes", | ||
| false => "No", |
| string? emailBCC = null) | ||
| { | ||
| var toList = emailTo.ParseEmailList() ?? []; | ||
| var ccList = emailCC.ParseEmailList(); |
| { | ||
| var toList = emailTo.ParseEmailList() ?? []; | ||
| var ccList = emailCC.ParseEmailList(); | ||
| var bccList = emailBCC.ParseEmailList(); |
| public async Task<GetSummaryDto> GetSummaryAsync(Guid applicationId) | ||
| { | ||
| var query = from application in await applicationRepository.GetQueryableAsync() | ||
| join applicationForm in await applicationFormRepository.GetQueryableAsync() on application.ApplicationFormId equals applicationForm.Id |
|
|
||
| var applicationFormId = formVersion?.ApplicationFormId; | ||
| var applicationForm = applicationFormId.HasValue | ||
| ? await applicationFormRepository.FindAsync(x => x.Id == applicationFormId.Value) |
| foreach (var expenseApproval in paymentRequestDto.ExpenseApprovals) | ||
| { | ||
| if (expenseApproval.DecisionUserId.HasValue | ||
| && userDictionary.TryGetValue(expenseApproval.DecisionUserId.Value, out var expenseApprovalUserDto)) | ||
| { | ||
| expenseApproval.DecisionUser = expenseApprovalUserDto; | ||
| } | ||
| } |
| foreach (var timeZoneId in PacificTimeZoneIanaIds) | ||
| { | ||
| if (TimeZoneInfo.TryFindSystemTimeZoneById(timeZoneId, out var timeZone)) | ||
| { | ||
| return timeZone; | ||
| } | ||
| } |
| </div> | ||
| </div> | ||
| <div class="col-4"> | ||
| <div class="intake-mapping-content"> | ||
| <div id="intake-map-available-fields-column" class="col" style="overflow-y:scroll"></div> | ||
| <div class="tab-content unt-tab-content" id="nav-tabContent"> | ||
| <!--Tab Content: Mapping--> | ||
| <div class="tab-pane fade show active" id="nav-intake-fields" role="tabpanel" | ||
| aria-labelledby="nav-intake-fields-tab"> | ||
|
|
||
| <div class="intake-mapping-content"> | ||
| <div id="intake-map-available-fields-column" class="col" style="overflow-y:scroll"> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <!--Tab Content: Worksheet Fields--> | ||
| <div class="tab-pane fade" id="nav-worksheet-fields" role="tabpanel" | ||
| aria-labelledby="nav-worksheet-fields-tab"> | ||
| <div class="worksheet-mapping-content"> | ||
| <div id="worksheet-map-available-fields-column" class="col" | ||
| style="overflow-y:scroll"> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <div class="row"> | ||
| <div class="col-12"> | ||
| <div class="buttons"> | ||
| <div class="buttons-div"> | ||
| <abp-button id="btn-save" | ||
| text="Save" | ||
| icon-type="Other" | ||
| icon="fl fl-save" | ||
| class="btn unt-btn-primary btn-primary mx-1" | ||
| style="pointer-events: all;" | ||
| abp-tooltip="Save the Scoresheet and the Mapping of CHEFS fields to Unity Fields" | ||
| button-type="Primary" /> | ||
| <abp-button id="btn-edit" | ||
| text="Edit Mapping" | ||
| icon-type="Other" | ||
| data-target="#editMappingModal" | ||
| icon="fl fl-edit" | ||
| class="btn unt-btn-primary btn-primary mx-1" | ||
| data-toggle="modal" | ||
| style="pointer-events: all;" | ||
| abp-tooltip="Edit the Mapping JSON Manually" | ||
| button-type="Primary" /> | ||
| <abp-button id="btn-reset" | ||
| text="Reset Mapping" | ||
| icon-type="Other" | ||
| icon="fl fl-undo" | ||
| class="btn unt-btn-outline-primary btn-outline-primary mx-1" | ||
| abp-tooltip="This button resets the mapping to the last saved state" /> | ||
| <abp-button id="btn-back" | ||
| text="Back" | ||
| class="btn unt-btn-outline-primary btn-outline-primary mx-1" | ||
| abp-tooltip="Navigate back to the Forms" /> | ||
| </div> | ||
| </div> | ||
| <div class="row"> | ||
| <div class="col-12"> | ||
| <div class="buttons"> | ||
| <div class="buttons-div"> | ||
| <abp-button id="btn-save" text="Save" icon-type="Other" | ||
| class="btn unt-btn-primary btn-primary mx-1" style="pointer-events: all;" | ||
| abp-tooltip="Save the Scoresheet and the Mapping of CHEFS fields to Unity Fields" | ||
| button-type="Primary" /> | ||
| <abp-button id="btn-edit" text="Edit Mapping" icon-type="Other" data-target="#editMappingModal" | ||
| icon="fl fl-edit" class="btn unt-btn-primary btn-primary mx-1" data-toggle="modal" |
Feature/ab#9999 fix merge conflict
There was a problem hiding this 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 is a broad refactor/cleanup plus feature work across the GrantManager solution, including new UI components, permissions, integrations, and schema changes.
Changes:
- Introduces new CustomFields widget/controller and related styling for form/worksheet configuration.
- Adds FSB payment notification tracking (new fields, event flow from Notifications → Payments, UI column).
- Performs various refactors/cleanup: service/interface adjustments, routing tweaks, dependency/package/config tidy-ups, and migrations.
Reviewed changes
Copilot reviewed 219 out of 230 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| applications/Unity.GrantManager/src/Unity.GrantManager.Web/Views/Shared/Components/CustomFields/CustomFieldsViewComponent.cs | Adds CustomFields widget ViewComponent and bundles |
| applications/Unity.GrantManager/src/Unity.GrantManager.Web/Views/Shared/Components/CustomFields/CustomFieldsController.cs | Adds POST handler for updating worksheet link ordering |
| applications/Unity.GrantManager/modules/Unity.Notifications/src/Unity.Notifications.Application/Integrations/RabbitMQ/EmailConsumer.cs | Publishes FSB email-sent event + records CHES status |
| applications/Unity.GrantManager/src/Unity.GrantManager.HttpApi/Controllers/AttachmentController.cs | Refactors invalid file-type validation logic |
| applications/Unity.GrantManager/src/Unity.GrantManager.EntityFrameworkCore/Repositories/PersonRepository.cs | Refactors OIDC subject lookup logic |
Files not reviewed (1)
- applications/Unity.AutoUI/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var formVersion = await applicationFormVersionAppService.GetByChefsFormVersionId(model.ChefsFormVersionId); | ||
| _ = await worksheetLinkAppService | ||
| .UpdateWorksheetLinksAsync(new UpdateWorksheetLinksDto() | ||
| { | ||
| CorrelationId = formVersion?.Id ?? Guid.Empty, | ||
| CorrelationProvider = CorrelationConsts.FormVersion, | ||
| WorksheetAnchors = tabLinks | ||
| }); |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If GetByChefsFormVersionId returns null, the code falls back to CorrelationId = Guid.Empty and still calls UpdateWorksheetLinksAsync. That risks writing/updating worksheet links under an empty correlation. Return NotFound/BadRequest when the form version can't be resolved, rather than proceeding with Guid.Empty.
| foreach (var fileName in files.Where(file => | ||
| { | ||
| string FileType = System.IO.Path.GetExtension(file.FileName); | ||
| string FileType = Path.GetExtension(file.FileName); | ||
| if (FileType.StartsWith('.')) | ||
| { | ||
| FileType = FileType[1..]; | ||
| } | ||
| return DisallowedFileTypes.Contains(FileType.ToLower()); | ||
| })) | ||
| }).Select(source => source.FileName)) | ||
| { | ||
| ErrorList.Add(new ValidationResult("Invalid file type for " + source.FileName, [nameof(source.FileName)])); | ||
| ErrorList.Add(new ValidationResult("Invalid file type for " + fileName, [nameof(fileName)])); | ||
| } |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ValidationResult member names should refer to the model/property name (previously this was effectively "FileName"). Using [nameof(fileName)] produces "fileName", which is unlikely to match any bound field and may break client-side error association. Use nameof(IFormFile.FileName)/"FileName" (or another stable member name like nameof(files)) instead.
| public async Task<Person?> FindByOidcSub(string oidcSub) | ||
| { | ||
| var dbSet = await GetDbSetAsync(); | ||
| var compare = oidcSub.ToSubjectWithoutIdp().ToUpper(); | ||
| return await dbSet.AsQueryable() | ||
| .FirstOrDefaultAsync(s => s.OidcSub | ||
| .ToUpper() | ||
| .StartsWith(oidcSub.ToSubjectWithoutIdp())); | ||
| .FirstOrDefaultAsync(s => s.OidcSub.StartsWith(compare)); | ||
| } |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compare is uppercased, but the query now uses s.OidcSub.StartsWith(compare) without normalizing s.OidcSub. This becomes case-sensitive in PostgreSQL and will fail for existing/test data where OidcSub isn't already uppercase (e.g., GrantManagerTestDataSeedContributor seeds TestUser1). Either normalize OidcSub on write everywhere, or make the query case-insensitive again (e.g., ToUpper() on the column, or an EF-translatable case-insensitive comparison).
| [Widget( | ||
| RefreshUrl = "CustomFields/Refresh", | ||
| ScriptTypes = new[] { typeof(CustomFieldsScriptBundleContributor) }, | ||
| StyleTypes = new[] { typeof(CustomFieldsStyleBundleContributor) }, | ||
| AutoInitialize = true)] |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RefreshUrl = "CustomFields/Refresh" points to a refresh endpoint that doesn't exist (there is only GrantApplications/CustomFields/update in CustomFieldsController). As-is, widget auto-refresh will 404. Add a Refresh action with a matching route (or update RefreshUrl to the actual route) that returns the CustomFields view component.
| public async Task<IViewComponentResult> InvokeAsync(string? formVersionId, string formName) | ||
| { | ||
| var model = new CustomFieldsViewModel { }; | ||
| model.ChefsFormVersionId = Guid.Parse(formVersionId ?? Guid.Empty.ToString()); | ||
| model.FormName = formName; |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guid.Parse(formVersionId ?? Guid.Empty.ToString()) will throw if formVersionId is null/empty/invalid (e.g., missing query arg). Prefer Guid.TryParse and return an empty/validation view model (or BadRequest) instead of throwing during view rendering.
| newApplicantAgent.IdentityName = intakeMap.ApplicantAgent?.name ?? ""; | ||
| newApplicantAgent.IdentityEmail = intakeMap.ApplicantAgent?.email ?? ""; | ||
|
|
||
| newApplicantAgent.OidcSubUser = intakeMap.ApplicantAgent?.oidc_sub_user; |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Condition is always not null because of dynamic call to method IsJObject.
| newApplicantAgent.IdentityEmail = intakeMap.ApplicantAgent?.email ?? ""; | ||
|
|
||
| newApplicantAgent.OidcSubUser = intakeMap.ApplicantAgent?.oidc_sub_user; | ||
| newApplicantAgent.IdentityProvider = intakeMap.ApplicantAgent?.identity_provider ?? ""; |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Condition is always not null because of dynamic call to method IsJObject.
| return indigenousOrgInd switch | ||
| { | ||
| true => "Yes", | ||
| false => "No", |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Condition is always true because of ... => ....
| if (question.Type == QuestionType.SelectList) | ||
| { | ||
| question.Answer = ConvertNumericAnswerToSelectListValue(rawAnswer, question.Definition); | ||
| } | ||
| else | ||
| { | ||
| question.Answer = rawAnswer; | ||
| } |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
| if (question.Type == QuestionType.SelectList) | ||
| { | ||
| question.Answer = ConvertNumericAnswerToSelectListValue(rawAnswer, question.Definition); | ||
| } | ||
| else | ||
| { | ||
| question.Answer = rawAnswer; | ||
| } |
Copilot
AI
Jan 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.
No description provided.