Feature/ab#9075 Applicant Profile Add Attachments#2224
Conversation
… and S3 information
…lete attachments on all attachment windows
…functions should be moved to the highest possible scopejavascript:S7721"
There was a problem hiding this comment.
Pull request overview
Adds Applicant-level attachment management to the Applicant Profile Details page, reusing the existing attachments infrastructure (S3-backed storage + ABP DataTables widget) that already exists for Applications/Assessments.
Changes:
- Introduces a new ApplicantAttachments widget (Razor view + JS/CSS) and wires it into the Applicant Details “Attachments” tab with a live count.
- Adds server-side support for Applicant attachments: new
ApplicantAttachmententity + repository, EF migration, andAttachmentType.APPLICANT. - Extends attachment APIs/services/blob provider logic to upload/download/list Applicant attachments.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| applications/Unity.GrantManager/src/Unity.GrantManager.Web/Views/Shared/Components/ApplicantAttachments/Default.cshtml | New widget markup for Applicant attachments table + upload control |
| applications/Unity.GrantManager/src/Unity.GrantManager.Web/Views/Shared/Components/ApplicantAttachments/ApplicantAttachments.js | DataTable + PubSub refresh/count wiring for Applicant attachments |
| applications/Unity.GrantManager/src/Unity.GrantManager.Web/Views/Shared/Components/ApplicantAttachments/ApplicantAttachments.css | DataTables v2 width/scroll handling styles for Applicant attachments |
| applications/Unity.GrantManager/src/Unity.GrantManager.Web/Views/Shared/Components/ApplicantAttachments/ApplicantAttachments.cs | New ABP widget component + bundle contributors for Applicant attachments |
| applications/Unity.GrantManager/src/Unity.GrantManager.Web/Views/Shared/Components/_Shared/Attachments.js | Adds Applicant owner-id + refresh-topic support to shared attachment helpers |
| applications/Unity.GrantManager/src/Unity.GrantManager.Web/Views/Shared/Components/_Shared/Attachments.css | Adjusts dropdown positioning/z-index used by attachment action menus |
| applications/Unity.GrantManager/src/Unity.GrantManager.Web/Pages/Applicants/Details.js | Adds Applicant upload helpers + subscribes to attachment count updates |
| applications/Unity.GrantManager/src/Unity.GrantManager.Web/Pages/Applicants/Details.cshtml.cs | Exposes current user + S3 upload config values to the Applicant Details view |
| applications/Unity.GrantManager/src/Unity.GrantManager.Web/Pages/Applicants/Details.cshtml | Renders the ApplicantAttachments widget and adds attachments count UI |
| applications/Unity.GrantManager/src/Unity.GrantManager.HttpApi/Controllers/AttachmentController.cs | Adds Applicant attachment upload/download endpoints; dedupes “file required” message |
| applications/Unity.GrantManager/src/Unity.GrantManager.EntityFrameworkCore/Repositories/ApplicantAttachmentRepository.cs | Adds EF Core repository implementation for ApplicantAttachment |
| applications/Unity.GrantManager/src/Unity.GrantManager.EntityFrameworkCore/Migrations/TenantMigrations/GrantTenantDbContextModelSnapshot.cs | Updates tenant model snapshot to include ApplicantAttachments |
| applications/Unity.GrantManager/src/Unity.GrantManager.EntityFrameworkCore/Migrations/TenantMigrations/20260401222707_AddApplicantAttachments.Designer.cs | Migration designer for ApplicantAttachments table |
| applications/Unity.GrantManager/src/Unity.GrantManager.EntityFrameworkCore/Migrations/TenantMigrations/20260401222707_AddApplicantAttachments.cs | Migration creating ApplicantAttachments table + index |
| applications/Unity.GrantManager/src/Unity.GrantManager.EntityFrameworkCore/EntityFrameworkCore/GrantTenantDbContext.cs | Adds DbSet + model configuration for ApplicantAttachment |
| applications/Unity.GrantManager/src/Unity.GrantManager.Domain/Applications/IApplicantAttachmentRepository.cs | New repository contract for ApplicantAttachment |
| applications/Unity.GrantManager/src/Unity.GrantManager.Domain/Applications/ApplicantAttachment.cs | New domain entity for Applicant attachment rows |
| applications/Unity.GrantManager/src/Unity.GrantManager.Domain.Shared/Attachments/AttachmentType.cs | Adds APPLICANT attachment type |
| applications/Unity.GrantManager/src/Unity.GrantManager.Application/GrantManagerApplicationModule.cs | Wires ApplicantS3Folder into S3 blob provider configuration |
| applications/Unity.GrantManager/src/Unity.GrantManager.Application/Attachments/S3BlobProviderConfiguration.cs | Adds ApplicantS3Folder config property |
| applications/Unity.GrantManager/src/Unity.GrantManager.Application/Attachments/S3BlobProvider.cs | Adds Applicant save/delete support in S3 blob provider |
| applications/Unity.GrantManager/src/Unity.GrantManager.Application/Attachments/AttachmentAppService.cs | Adds Applicant attachment retrieval/metadata support |
| applications/Unity.GrantManager/src/Unity.GrantManager.Application.Contracts/Attachments/IAttachmentAppService.cs | Exposes GetApplicantAsync contract |
| currentUserId + | ||
| '&userName=' + | ||
| currentUserName; |
There was a problem hiding this comment.
The upload URL is built by string concatenation without URL-encoding query parameters. Since currentUserName can contain spaces, commas, or &, this can produce an invalid URL or allow query-string injection. Encode each query param (e.g., encodeURIComponent) before appending.
| currentUserId + | |
| '&userName=' + | |
| currentUserName; | |
| encodeURIComponent(currentUserId) + | |
| '&userName=' + | |
| encodeURIComponent(currentUserName); |
| contentType: false, | ||
| type: 'POST', | ||
| success: function (data) { | ||
| abp.notify.success(data.responseText, 'File Upload Is Successful'); |
There was a problem hiding this comment.
On successful upload the API returns an Ok(string) response, so data will be a string. Using data.responseText will be undefined, resulting in a blank/incorrect success notification. Use the returned data value (or the jqXHR object) for the toast message.
| abp.notify.success(data.responseText, 'File Upload Is Successful'); | |
| abp.notify.success(data, 'File Upload Is Successful'); |
| let currentUserId = decodeURIComponent($('#CurrentUserId').val()); | ||
| let currentUserName = decodeURIComponent($('#CurrentUserName').val()); | ||
| let url = | ||
| '/api/app/attachment/applicant/' + | ||
| applicantId + | ||
| '/upload?userId=' + | ||
| currentUserId + | ||
| '&userName=' + | ||
| currentUserName; |
There was a problem hiding this comment.
The client sends userId (and userName) from hidden fields in the request URL. These values are client-controlled and can be tampered with, allowing a user to attribute an upload to a different user. The server should derive the user identity from the authenticated principal (e.g., ICurrentUser) rather than trusting query-string values.
| let currentUserId = decodeURIComponent($('#CurrentUserId').val()); | |
| let currentUserName = decodeURIComponent($('#CurrentUserName').val()); | |
| let url = | |
| '/api/app/attachment/applicant/' + | |
| applicantId + | |
| '/upload?userId=' + | |
| currentUserId + | |
| '&userName=' + | |
| currentUserName; | |
| let url = '/api/app/attachment/applicant/' + applicantId + '/upload'; |
| <li class="nav-item"> | ||
| <button class="nav-link" id="attachments-tab" data-bs-toggle="tab" data-bs-target="#attachments" type="button" role="tab" aria-controls="attachments" aria-selected="true">Attachment</button> | ||
| <button class="nav-link" id="attachments-tab" data-bs-toggle="tab" data-bs-target="#attachments" type="button" role="tab" aria-controls="attachments" aria-selected="true"><i class="fl fl-paperclip" aria-hidden="true"></i> (<span id="applicant_attachment_count">0</span>)</button> | ||
| </li> |
There was a problem hiding this comment.
The attachments tab button now contains only an icon and a count, leaving it without an accessible name. Add visible text (e.g., “Attachments”) or an aria-label/aria-labelledby so screen readers can identify the tab.
| [HttpPost("applicant/{applicantId}/upload")] | ||
| #pragma warning disable IDE0060 // Remove unused parameter | ||
| public async Task<IActionResult> UploadApplicantAttachments(Guid applicantId, IList<IFormFile> files, string userId, string userName) | ||
| #pragma warning restore IDE0060 // Remove unused parameter | ||
| { | ||
| if (!ModelState.IsValid) | ||
| { | ||
| return BadRequest(ModelState); | ||
| } | ||
|
|
||
| if (files == null || files.Count == 0) | ||
| { | ||
| return BadRequest(fileProvidedError); | ||
| } | ||
|
|
||
| return await UploadFiles(files); | ||
| } |
There was a problem hiding this comment.
UploadApplicantAttachments accepts userId/userName parameters that are unused (suppressed via pragma) and are client-controlled. Prefer removing these parameters and deriving the user from the authenticated context server-side (or, if they must remain, mark them explicitly as [FromQuery] and validate/ignore them).
| public override async Task SaveAsync(BlobProviderSaveArgs args) | ||
| { | ||
| var httpContext = _httpContextAccessor.HttpContext ?? throw new InvalidOperationException("No active HttpContext."); | ||
| var queryParams = httpContext.Request?.Query ?? throw new InvalidOperationException("No query parameters in the current request."); | ||
| var routeData = _httpContextAccessor.HttpContext.GetRouteData(); | ||
| var assessmentId = routeData.Values["assessmentId"]; | ||
|
|
||
| var assessmentId = routeData.Values["assessmentId"]; | ||
| var applicationId = routeData.Values["applicationId"]; | ||
| var applicantId = routeData.Values["applicantId"]; | ||
| queryParams.TryGetValue("userId", out StringValues currentUserId); | ||
| if (assessmentId != null) | ||
| { | ||
| queryParams.TryGetValue("userId", out StringValues currentUserId); | ||
| #pragma warning disable CS8604 // Possible null reference argument. | ||
|
|
||
| #pragma warning disable CS8604 // Possible null reference argument. | ||
| await UploadAssessmentAttachment(args, assessmentId.ToString(), currentUserId.ToString()); | ||
| #pragma warning restore CS8604 // Possible null reference argument. | ||
| #pragma warning restore CS8604 // Possible null reference argument. | ||
| } | ||
| else if(applicationId != null) | ||
| { | ||
| #pragma warning disable CS8604 // Possible null reference argument. | ||
| await UploadApplicationAttachment(args, applicationId.ToString(), currentUserId.ToString()); | ||
| #pragma warning restore CS8604 // Possible null reference argument. | ||
| } | ||
| else if (applicantId != null) | ||
| { | ||
| #pragma warning disable CS8604 // Possible null reference argument. | ||
| await UploadApplicantAttachment(args, applicantId.ToString(), currentUserId.ToString()); | ||
| #pragma warning restore CS8604 // Possible null reference argument. | ||
| } |
There was a problem hiding this comment.
SaveAsync reads userId from the query string and passes it through to new Guid(currentUserId). If the query param is missing or not a valid GUID, this will throw and return a 500. Validate with Guid.TryParse and return a clear validation error (and ideally stop using a client-supplied userId at all).
| else if (attachmentType == "Applicant") | ||
| { | ||
| if (attachmentTypeId.IsNullOrEmpty()) | ||
| { | ||
| throw new AbpValidationException("Missing ApplicantId"); | ||
| } | ||
| IQueryable<ApplicantAttachment> queryableAttachment = _applicantAttachmentRepository.GetQueryableAsync().Result; | ||
| ApplicantAttachment? attachment = queryableAttachment.FirstOrDefault(a => a.S3ObjectKey.Equals(s3ObjectKey) && a.ApplicantId.Equals(new Guid(attachmentTypeId.ToString()))); | ||
| if (attachment != null) | ||
| { | ||
| await _applicantAttachmentRepository.DeleteAsync(attachment); | ||
| } |
There was a problem hiding this comment.
This new Applicant delete branch uses GetQueryableAsync().Result (sync-over-async) inside an async method. This can cause thread-pool starvation and makes failures harder to diagnose. Await GetQueryableAsync() and keep the query fully async.
| private async Task UploadApplicantAttachment(BlobProviderSaveArgs args, string applicantId, string currentUserId) | ||
| { | ||
| var config = args.Configuration.GetS3BlobProviderConfiguration(); | ||
| var bucket = config.Bucket; | ||
| var folder = args.Configuration.GetS3BlobProviderConfiguration().ApplicantS3Folder; | ||
| if (!folder.EndsWith('/')) | ||
| { | ||
| folder += "/"; | ||
| } | ||
| folder += applicantId; | ||
| var key = folder + "/" + args.BlobName; | ||
| var escapedKey = folder + "/" + Uri.EscapeDataString(args.BlobName); | ||
| var mimeType = GetMimeType(args.BlobName); | ||
| await UploadToS3(args, bucket, escapedKey, mimeType); | ||
| IQueryable<ApplicantAttachment> queryableAttachment = _applicantAttachmentRepository.GetQueryableAsync().Result; | ||
| ApplicantAttachment? attachment = queryableAttachment.FirstOrDefault(a => a.S3ObjectKey.Equals(key) && a.ApplicantId.Equals(new Guid(applicantId))); | ||
| if (attachment == null) | ||
| { | ||
| await _applicantAttachmentRepository.InsertAsync( | ||
| new ApplicantAttachment | ||
| { | ||
| ApplicantId = new Guid(applicantId), | ||
| S3ObjectKey = key, | ||
| UserId = new Guid(currentUserId), | ||
| FileName = args.BlobName, | ||
| Time = DateTime.UtcNow, | ||
| }); | ||
| } | ||
| else | ||
| { | ||
| attachment.UserId = new Guid(currentUserId); | ||
| attachment.FileName = args.BlobName; | ||
| attachment.Time = DateTime.UtcNow; | ||
| await _applicantAttachmentRepository.UpdateAsync(attachment); | ||
| } | ||
| } |
There was a problem hiding this comment.
UploadApplicantAttachment uses GetQueryableAsync().Result and synchronously queries for an existing row. This is sync-over-async and can impact scalability under load. Await GetQueryableAsync() and keep the DB query async (or use repository methods that avoid pulling an IQueryable synchronously).
| uploadFiles(inputId, url, 'refresh_applicant_attachment_list'); | ||
| } | ||
|
|
||
| function uploadFiles(inputId, urlStr, channel) { | ||
| let input = document.getElementById(inputId); | ||
| let files = input.files; | ||
| let formData = new FormData(); | ||
| const disallowedTypes = JSON.parse( | ||
| decodeURIComponent($('#Extensions').val()) | ||
| ); | ||
| const maxFileSize = decodeURIComponent($('#MaxFileSize').val()); | ||
|
|
||
| let isAllowedTypeError = false; | ||
| let isMaxFileSizeError = false; | ||
| if (files.length == 0) { | ||
| return; | ||
| } | ||
|
|
||
| for (let file of files) { | ||
| if ( | ||
| disallowedTypes.includes( | ||
| file.name | ||
| .slice(file.name.lastIndexOf('.') + 1, file.name.length) | ||
| .toLowerCase() | ||
| ) | ||
| ) { | ||
| isAllowedTypeError = true; | ||
| } | ||
| if (file.size * 0.000001 > maxFileSize) { | ||
| isMaxFileSizeError = true; | ||
| } | ||
|
|
||
| formData.append('files', file); | ||
| } | ||
|
|
||
| if (isAllowedTypeError) { | ||
| input.value = null; | ||
| return abp.notify.error('Error', 'File type not supported'); | ||
| } | ||
| if (isMaxFileSizeError) { | ||
| input.value = null; | ||
| return abp.notify.error( | ||
| 'Error', | ||
| 'File size exceeds ' + maxFileSize + 'MB' | ||
| ); | ||
| } | ||
|
|
||
| $.ajax({ | ||
| url: urlStr, | ||
| data: formData, | ||
| processData: false, | ||
| contentType: false, | ||
| type: 'POST', | ||
| success: function (data) { | ||
| abp.notify.success(data.responseText, 'File Upload Is Successful'); | ||
| PubSub.publish(channel); | ||
| input.value = null; | ||
| }, | ||
| error: function (data) { | ||
| abp.notify.error(data.responseText, 'File Upload Not Successful'); | ||
| PubSub.publish(channel); | ||
| input.value = null; |
There was a problem hiding this comment.
The uploadFiles helper (and validation/toast logic) duplicates the same implementation already present in Pages/GrantApplications/Details.js. To reduce drift (and avoid needing to fix bugs in multiple places), consider extracting this into a shared attachments utility (e.g., /Views/Shared/Components/_Shared/Attachments.js) and reusing it from both pages.
| uploadFiles(inputId, url, 'refresh_applicant_attachment_list'); | |
| } | |
| function uploadFiles(inputId, urlStr, channel) { | |
| let input = document.getElementById(inputId); | |
| let files = input.files; | |
| let formData = new FormData(); | |
| const disallowedTypes = JSON.parse( | |
| decodeURIComponent($('#Extensions').val()) | |
| ); | |
| const maxFileSize = decodeURIComponent($('#MaxFileSize').val()); | |
| let isAllowedTypeError = false; | |
| let isMaxFileSizeError = false; | |
| if (files.length == 0) { | |
| return; | |
| } | |
| for (let file of files) { | |
| if ( | |
| disallowedTypes.includes( | |
| file.name | |
| .slice(file.name.lastIndexOf('.') + 1, file.name.length) | |
| .toLowerCase() | |
| ) | |
| ) { | |
| isAllowedTypeError = true; | |
| } | |
| if (file.size * 0.000001 > maxFileSize) { | |
| isMaxFileSizeError = true; | |
| } | |
| formData.append('files', file); | |
| } | |
| if (isAllowedTypeError) { | |
| input.value = null; | |
| return abp.notify.error('Error', 'File type not supported'); | |
| } | |
| if (isMaxFileSizeError) { | |
| input.value = null; | |
| return abp.notify.error( | |
| 'Error', | |
| 'File size exceeds ' + maxFileSize + 'MB' | |
| ); | |
| } | |
| $.ajax({ | |
| url: urlStr, | |
| data: formData, | |
| processData: false, | |
| contentType: false, | |
| type: 'POST', | |
| success: function (data) { | |
| abp.notify.success(data.responseText, 'File Upload Is Successful'); | |
| PubSub.publish(channel); | |
| input.value = null; | |
| }, | |
| error: function (data) { | |
| abp.notify.error(data.responseText, 'File Upload Not Successful'); | |
| PubSub.publish(channel); | |
| input.value = null; | |
| uploadFiles(inputId, url, 'refresh_applicant_attachment_list'); | |
| } | |
| function getAttachmentUploadOptions() { | |
| return { | |
| disallowedTypes: JSON.parse(decodeURIComponent($('#Extensions').val())), | |
| maxFileSize: decodeURIComponent($('#MaxFileSize').val()), | |
| }; | |
| } | |
| function clearAttachmentInput(input) { | |
| input.value = null; | |
| } | |
| function getFileExtension(fileName) { | |
| return fileName | |
| .slice(fileName.lastIndexOf('.') + 1, fileName.length) | |
| .toLowerCase(); | |
| } | |
| function buildAttachmentFormData(files, options) { | |
| let formData = new FormData(); | |
| let isAllowedTypeError = false; | |
| let isMaxFileSizeError = false; | |
| for (let file of files) { | |
| if (options.disallowedTypes.includes(getFileExtension(file.name))) { | |
| isAllowedTypeError = true; | |
| } | |
| if (file.size * 0.000001 > options.maxFileSize) { | |
| isMaxFileSizeError = true; | |
| } | |
| formData.append('files', file); | |
| } | |
| return { | |
| formData: formData, | |
| isAllowedTypeError: isAllowedTypeError, | |
| isMaxFileSizeError: isMaxFileSizeError, | |
| }; | |
| } | |
| function notifyAttachmentValidationError(input, validationResult, maxFileSize) { | |
| if (validationResult.isAllowedTypeError) { | |
| clearAttachmentInput(input); | |
| abp.notify.error('Error', 'File type not supported'); | |
| return true; | |
| } | |
| if (validationResult.isMaxFileSizeError) { | |
| clearAttachmentInput(input); | |
| abp.notify.error('Error', 'File size exceeds ' + maxFileSize + 'MB'); | |
| return true; | |
| } | |
| return false; | |
| } | |
| function finalizeAttachmentUpload(input, channel) { | |
| PubSub.publish(channel); | |
| clearAttachmentInput(input); | |
| } | |
| function uploadFiles(inputId, urlStr, channel) { | |
| let input = document.getElementById(inputId); | |
| let files = input.files; | |
| let options = getAttachmentUploadOptions(); | |
| if (files.length == 0) { | |
| return; | |
| } | |
| let validationResult = buildAttachmentFormData(files, options); | |
| if (notifyAttachmentValidationError(input, validationResult, options.maxFileSize)) { | |
| return; | |
| } | |
| $.ajax({ | |
| url: urlStr, | |
| data: validationResult.formData, | |
| processData: false, | |
| contentType: false, | |
| type: 'POST', | |
| success: function (data) { | |
| abp.notify.success(data.responseText, 'File Upload Is Successful'); | |
| finalizeAttachmentUpload(input, channel); | |
| }, | |
| error: function (data) { | |
| abp.notify.error(data.responseText, 'File Upload Not Successful'); | |
| finalizeAttachmentUpload(input, channel); |
aurelio-aot
left a comment
There was a problem hiding this comment.
I tested the branch and everything is working well. Other than the Copilot reviews everything looks good to me.
Added the add attachment panel to the Applicant profile page, mimics what is currently implemented for Assessments and Applications