Report background job progress in bit Boilerplate (#11649) #11650
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe pull request refactors the push notification API to accept request objects instead of individual parameters, integrates background job progress tracking via SignalR, enhances job runners with Hangfire context support, updates social sign-in callback naming, and adds localization strings. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server as PushNotificationJobRunner
participant Hub as SignalR Hub
participant DB as Database
Client->>Server: Enqueue background job (RequestPush)
Server->>DB: Fetch connection ID from session
loop For each subscription
Server->>Server: Attempt send notification
alt Success
Server->>Server: succeededItems++
else Failure
Server->>Server: failedItems++<br/>record subscription ID
end
opt SignalR enabled & connection ID available
Server->>Hub: Send BACKGROUND_JOB_PROGRESS<br/>(jobId, title, counts)
Hub->>Client: Deliver progress update
end
end
Server->>Server: Aggregate exceptions<br/>with JobId metadata
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/Jobs/EmailServiceJobsRunner.cs (1)
16-42: Add null check for PerformContext before accessing its properties.The
contextparameter defaults tonull!, but is accessed on line 41 without a null check. If this method is invoked outside of Hangfire's execution context (e.g., in tests or direct calls), accessingcontext.BackgroundJob.Idwill throw aNullReferenceException.Apply this diff to add a null check:
catch (Exception exp) { - serverExceptionHandler.Handle(exp, new() - { - { "Subject", subject }, - { "ToEmailAddress", toEmailAddress }, - { "JobId", context.BackgroundJob.Id } - }); + var parameters = new Dictionary<string, object?> + { + { "Subject", subject }, + { "ToEmailAddress", toEmailAddress } + }; + + if (context?.BackgroundJob?.Id is not null) + { + parameters.Add("JobId", context.BackgroundJob.Id); + } + + serverExceptionHandler.Handle(exp, parameters); if (exp is not KnownException && cancellationToken.IsCancellationRequested is false) throw; // To retry the job }src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/Jobs/PhoneServiceJobsRunner.cs (1)
12-36: Avoid null‑dereferencingPerformContextwhen logging JobId
SendSmsnow acceptsPerformContext context = null!but the catch block usescontext.BackgroundJob.Idunconditionally. If this method is ever invoked without a Hangfire-provided context (e.g., direct calls or tests), the error path will throw aNullReferenceExceptionand hide the original failure.Consider guarding the access (minimal change):
- serverExceptionHandler.Handle(exp, new() - { - { "PhoneNumber", phoneNumber }, - { "JobId", context.BackgroundJob.Id } - }); + serverExceptionHandler.Handle(exp, new() + { + { "PhoneNumber", phoneNumber }, + { "JobId", context?.BackgroundJob?.Id } + });Optionally, you could also drop the
= null!and makePerformContextnullable (PerformContext? context = null) to better reflect the parameter’s semantics.Separately,
SendSmsis markedasyncbut contains noawait; if Twilio has an async API you may want to use it, otherwise consider removingasyncand returningTask.CompletedTaskfor clarity.
🧹 Nitpick comments (4)
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/SignalR/BackgroundJobProgressDto.cs (1)
3-10: LGTM! Clean DTO design.The DTO effectively captures background job progress information. Consider adding a computed percentage property for convenience:
public class BackgroundJobProgressDto { public required string JobId { get; set; } public required string JobTitle { get; set; } public int SucceededItems { get; set; } public int TotalItems { get; set; } public int FailedItems { get; set; } + public double ProgressPercentage => TotalItems > 0 ? (double)SucceededItems / TotalItems * 100 : 0; }src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Diagnostics/DiagnosticsController.cs (1)
55-62: Request object usage looks good; consider wiring requester session for progressThe switch to
RequestPush(new() { ... }, predicate, cancellationToken)is consistent with the new object-based API and preserves existing behavior.If this diagnostic endpoint is also meant to exercise background job progress reporting, you may want to attach the current user session:
await pushNotificationService.RequestPush(new() { Title = "Test Push", Message = $"Open terms page. {DateTimeOffset.Now:HH:mm:ss}", Action = "testAction", PageUrl = PageUrls.Terms, UserRelatedPush = false, RequesterUserSessionId = userSessionId }, s => s.DeviceId == pushNotificationSubscriptionDeviceId, cancellationToken);(that assumes
userSessionIdis set whenisAuthenticatedis true).src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/Jobs/PushNotificationJobRunner.cs (2)
19-21: AlignhubContextnullability with other injected fieldsOther
[AutoInject]fields are initialized with= default!;to satisfy nullable reference checks.hubContextis currently missing that, which can introduce new warnings even though DI guarantees a value.Consider:
- [AutoInject] private IHubContext<AppHub> hubContext; + [AutoInject] private IHubContext<AppHub> hubContext = default!;This keeps the pattern consistent and makes the intent clear that DI will provide a non-null instance at runtime.
47-58: Consider honoring the job cancellation token in the parallel send loopThe new progress reporting and per-subscription tracking look solid, and
RequestPushaccepts aCancellationToken cancellationToken. However:
Parallel.ForEachAsyncis configured withCancellationToken = default, so job cancellation does not stop the loop.adsPushSender.BasicSendAsyncis called withdefaultinstead of a real token.- SignalR
SendAsyncuses the loop’scancellationToken, which currently is also effectivelyCancellationToken.None.If you want the job to respect cancellation (e.g., when the Hangfire job is aborted), you could wire the method token through:
- await Parallel.ForEachAsync(subscriptions, parallelOptions: new() - { - MaxDegreeOfParallelism = 10, - CancellationToken = default - }, async (subscription, cancellationToken) => + await Parallel.ForEachAsync(subscriptions, parallelOptions: new() + { + MaxDegreeOfParallelism = 10, + CancellationToken = cancellationToken + }, async (subscription, parallelToken) => { try { ... - await adsPushSender.BasicSendAsync(target, subscription.PushChannel, payload, default); + await adsPushSender.BasicSendAsync(target, subscription.PushChannel, payload, parallelToken); ... } catch (Exception exp) { ... } finally { try { if (signalRConnectionId != null) { - _ = hubContext.Clients.Client(signalRConnectionId).SendAsync(SharedAppMessages.BACKGROUND_JOB_PROGRESS, new BackgroundJobProgressDto() + _ = hubContext.Clients.Client(signalRConnectionId).SendAsync(SharedAppMessages.BACKGROUND_JOB_PROGRESS, new BackgroundJobProgressDto() { ... - }, cancellationToken); + }, parallelToken); } } catch { } } });This keeps your new progress reporting logic but makes the job responsive to cancellation signals.
Also applies to: 63-68, 76-80, 84-89, 91-104
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (21)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppClientCoordinator.cs(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/WebInteropApp.ts(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/events.ts(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Diagnostics/DiagnosticsController.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/IdentityController.ResetPassword.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/IdentityController.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/RoleManagementController.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/UserController.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/EmailService.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/Jobs/EmailServiceJobsRunner.cs(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/Jobs/PhoneServiceJobsRunner.cs(3 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/Jobs/PushNotificationJobRunner.cs(4 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/PhoneService.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/PushNotificationService.cs(4 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/AppJsonContext.cs(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/Identity/SendNotificationToRoleDto.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/SignalR/BackgroundJobProgressDto.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fa.resx(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resx(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/SharedAppMessages.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: CodeQL analysis (csharp)
- GitHub Check: Agent
- GitHub Check: build and test
🔇 Additional comments (18)
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.fa.resx (1)
270-271: LGTM! Localization added for background job progress feature.The new
PushNotificationJobresource entry is properly structured and provides appropriate Persian localization for the background job progress reporting feature introduced in this PR.src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Resources/AppStrings.resx (1)
270-271: LGTM! English localization added for background job progress feature.The new
PushNotificationJobresource entry is properly structured and provides clear English localization for the background job progress reporting feature. The placement within the SignalR conditional block is appropriate and consistent with the Persian localization file.src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/Identity/SendNotificationToRoleDto.cs (1)
9-12: LGTM!The XML documentation clearly describes the property's purpose.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/EmailService.cs (1)
133-133: LGTM!The call correctly aligns with the updated
SendEmailJobsignature. Hangfire will automatically inject thePerformContextparameter when the job executes.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/Jobs/EmailServiceJobsRunner.cs (1)
3-3: LGTM!The
usingdirective is required for thePerformContexttype.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/PushNotificationService.cs (2)
49-73: Excellent refactoring to use structured request object.Consolidating the notification parameters into a
PushNotificationRequestobject improves the API surface and makes it easier to extend notification functionality in the future.
76-87: LGTM!The
PushNotificationRequestclass is well-designed with appropriate nullability. The conditional inclusion ofRequesterUserSessionIdfor SignalR scenarios aligns with the PR's background job progress reporting feature.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor.cs (1)
110-110: LGTM!The comment updates correctly reference the
SOCIAL_SIGN_IN_CALLBACKmessage constant, improving code documentation accuracy.Also applies to: 205-205
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/events.ts (1)
9-9: LGTM!The reordering of event listener registration doesn't affect runtime behavior, as both listeners are registered during the same execution phase.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/IdentityController.ResetPassword.cs (1)
69-73: LGTM!The call correctly adapts to the new
PushNotificationServiceAPI using object initialization while preserving the existing subscription filter and cancellation token logic.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Scripts/WebInteropApp.ts (1)
67-67: LGTM! Improved naming clarity.The rename from
SOCIAL_SIGN_INtoSOCIAL_SIGN_IN_CALLBACKbetter reflects the purpose of this IPC message.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/RoleManagementController.cs (1)
234-241: LGTM! Cleaner API with object parameter.The refactoring to use an object initializer for the push notification request improves readability and maintainability.
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/PhoneService.cs (1)
41-41: LGTM! Simplified job enqueue call.Removing the explicit
defaultcancellation token parameter is appropriate if the job runner method now has it as an optional parameter.src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/UserController.cs (1)
437-441: LGTM! Consistent push notification API refactoring.Both
SendElevatedAccessTokenandToggleNotificationproperly use the new object initializer pattern for push notification requests, maintaining consistency with other controllers.Also applies to: 465-469
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppClientCoordinator.cs (2)
5-5: LGTM! Required import for background job progress.The new using directive is necessary for the
BackgroundJobProgressDtoused in the SignalR message handler.
300-300: LGTM! Improved readability with explicit grouping.The parentheses around
HubConnectionState.Connected or HubConnectionState.Connectingimprove readability without changing the logic.src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Dtos/AppJsonContext.cs (1)
17-17: LGTM! Proper serialization context configuration.The
BackgroundJobProgressDtois correctly added to the source-generated JSON serialization context, enabling efficient SignalR message serialization.Also applies to: 68-68
src/Templates/Boilerplate/Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Controllers/Identity/IdentityController.cs (1)
351-355: LGTM! Consistent push notification API refactoring.Both
SendOtpandSendTwoFactorTokenproperly use the new object initializer pattern, maintaining consistency with the broader API refactoring across all identity controllers.Also applies to: 423-427
...rplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppClientCoordinator.cs
Outdated
Show resolved
Hide resolved
...Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/Jobs/PushNotificationJobRunner.cs
Show resolved
Hide resolved
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/SharedAppMessages.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
This PR implements background job progress reporting in bit Boilerplate using SignalR, enabling real-time progress updates for Hangfire jobs sent from server to clients. The primary use case is tracking push notification batch sending progress.
Key Changes:
- Added
BackgroundJobProgressDtoto communicate job progress information via SignalR - Refactored
PushNotificationServiceto use aPushNotificationRequestobject for better parameter organization - Updated all Hangfire job runners to accept
PerformContextandCancellationTokenparameters for job metadata access - Implemented real-time progress reporting in
PushNotificationJobRunnerfor batch notification sending - Fixed the message constant naming from
SOCIAL_SIGN_INtoSOCIAL_SIGN_IN_CALLBACKfor consistency
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| SharedAppMessages.cs | Added BACKGROUND_JOB_PROGRESS constant for SignalR job progress updates |
| AppStrings.resx & AppStrings.fa.resx | Added localized strings for "PushNotificationJob" |
| BackgroundJobProgressDto.cs | New DTO defining job progress structure with succeeded/failed/total item counts |
| SendNotificationToRoleDto.cs | Added documentation for PageUrl property |
| AppJsonContext.cs | Registered BackgroundJobProgressDto for JSON serialization |
| PushNotificationService.cs | Refactored to use PushNotificationRequest object and added RequesterUserSessionId for SignalR updates |
| PhoneServiceJobsRunner.cs | Added PerformContext parameter and JobId to error logging |
| PushNotificationJobRunner.cs | Implemented real-time progress reporting, added failure tracking, and enhanced error diagnostics |
| EmailServiceJobsRunner.cs | Added PerformContext parameter and JobId to error logging |
| PhoneService.cs, EmailService.cs | Removed explicit default parameters from Enqueue calls (Hangfire auto-injects them) |
| UserController.cs, RoleManagementController.cs, IdentityController.cs, DiagnosticsController.cs | Updated to use PushNotificationRequest object |
| events.ts | Reordered event listener registration for better organization |
| WebInteropApp.ts | Updated message constant from SOCIAL_SIGN_IN to SOCIAL_SIGN_IN_CALLBACK |
| SignInPanel.razor.cs | Updated comments to reference SOCIAL_SIGN_IN_CALLBACK |
| AppClientCoordinator.cs | Added SignalR handler for BACKGROUND_JOB_PROGRESS messages and fixed pattern matching syntax |
src/Templates/Boilerplate/Bit.Boilerplate/src/Shared/Services/SharedAppMessages.cs
Outdated
Show resolved
Hide resolved
...rplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppClientCoordinator.cs
Outdated
Show resolved
Hide resolved
...rplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppClientCoordinator.cs
Outdated
Show resolved
Hide resolved
...Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/Jobs/PushNotificationJobRunner.cs
Outdated
Show resolved
Hide resolved
...rplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/AppClientCoordinator.cs
Outdated
Show resolved
Hide resolved
...Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/Jobs/PushNotificationJobRunner.cs
Show resolved
Hide resolved
...Bit.Boilerplate/src/Server/Boilerplate.Server.Api/Services/Jobs/PushNotificationJobRunner.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Yaser Moradi <ysmoradi@outlook.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Yaser Moradi <ysmoradi@outlook.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Yaser Moradi <ysmoradi@outlook.com>
1 participant
closes #11649
Summary by CodeRabbit
New Features
Bug Fixes
Documentation