Add configurable queue name support to AzureQueueHostedService

[brant-livefront]

📔 Objective

Now that we have transitioned to use Azure Service Bus in production for our event integrations, the AzureQueueHostedService was somewhat obsolete. We were still running it to drain the Azure Queue for events, but we were not adding any events to that queue anymore (they all go to ASB instead). This meant it was still sending its regular interval read requests, but they would always return zero, resulting in needless traffic.

This PR adds a configuration property to pass in the queue name. It follows the convention we had with the connection string. Since we don't want to adjust the connection string in production (it is tied to other features), the queue name provides us a way to short-circuit the queue service. If the queue name is not preset, it will simply return

• Added configurable queue name support to AzureQueueHostedService
• Queue name is now read from azureStorageQueueName IConfiguration key
• Maintains backward compatibility with early return pattern

Configuration required to maintain existing functionality

To maintain the same functionality as the hardcoded version, the following configuration must be set:

appsettings.json:

{
  "azureStorageQueueName": "event"
}

In our case, we will leave this empty in all environments (including production), which will accomplish the task of turning off the read service on the azure queue. If anyone was using this in self-hosted or otherwise, they'll need to add the setting to continue using it as it was.

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[claude[bot]]

Claude finished @brant-livefront's task —— View job

---

Code Review Complete

Task List

• [x] Read the changed file to understand modifications
• [x] Perform detailed code review using bitwarden-code-reviewer agent
• [x] Compile and post review findings

---

Overall Assessment: REQUEST CHANGES ⚠️

The core logic of making the queue name configurable is sound, but this change introduces several critical issues that need to be addressed before merging.

---

❌ Finding 1: Breaking change without configuration updates or migration path

Location: src/EventsProcessor/AzureQueueHostedService.cs:60-63

Issue: This change will cause the EventsProcessor service to silently fail in existing deployments. The new azureQueueServiceQueueName configuration key is required but:

1. Not present in any appsettings.json files
2. No default value is provided
3. Existing deployments will stop processing events without any error indication

Impact: Production deployments will silently stop processing events after this change is deployed unless operators manually add the new configuration key.

Required Actions:

1. Update all appsettings files to include "azureQueueServiceQueueName": "event" to maintain current behavior
2. Add deployment documentation explaining this new required configuration
3. Communicate to DevOps team about the configuration requirement before deployment
4. Consider adding a warning log message when the service exits early due to missing configuration

Example configuration update needed:

{
  "azureStorageConnectionString": "SECRET",
  "azureQueueServiceQueueName": "event"
}

---

⚠️ Finding 2: Architectural inconsistency between event producer and consumer

Location: src/EventsProcessor/AzureQueueHostedService.cs:69

Issue: The event consumer (this service) now uses a configurable queue name, but the event producer (AzureQueueEventWriteService) still has the queue name "event" hardcoded:

public AzureQueueEventWriteService(GlobalSettings globalSettings) : base(
    new QueueClient(globalSettings.Events.ConnectionString, "event"),
    JsonHelpers.IgnoreWritingNull)
{ }

Risk: This creates potential for misconfiguration where:

• Producer writes to "event" queue
• Consumer reads from a different queue (if configured incorrectly)
• Events are never processed, with no error indication

Questions:

1. Should the producer also be made configurable to match this change?
2. Should there be validation ensuring producer and consumer use the same queue name?
3. What's the intended use case for changing the queue name from "event"?

Recommendation: Either make both producer and consumer use the same configurable value from GlobalSettings.Events, or add documentation explaining why they might differ.

---

⚠️ Finding 3: Silent failure makes debugging difficult

Location: src/EventsProcessor/AzureQueueHostedService.cs:61-65

Issue: When either azureStorageConnectionString or queueName is missing, the service returns silently without any log message. While the PR description states this is intentional, it makes it difficult to distinguish between:

• Service intentionally disabled (expected behavior)
• Service accidentally disabled due to missing/incorrect configuration (operational error)

Recommended improvement:

if (string.IsNullOrWhiteSpace(storageConnectionString) ||
    string.IsNullOrWhiteSpace(queueName))
{
    _logger.LogInformation("Azure Queue service is disabled. Connection string present: {HasConnectionString}, Queue name present: {HasQueueName}",
        !string.IsNullOrWhiteSpace(storageConnectionString),
        !string.IsNullOrWhiteSpace(queueName));
    return;
}

This provides operational visibility while still allowing intentional disabling of the service.

---

🎨 Finding 4: Configuration pattern inconsistent with other services

Location: src/EventsProcessor/AzureQueueHostedService.cs:60

Observation: This service uses IConfiguration directly to read configuration, while other similar queue services use GlobalSettings:

• AzureQueueMailHostedService: Uses _globalSettings.Mail.ConnectionString
• Notifications/AzureQueueHostedService: Uses _globalSettings.Notifications.ConnectionString
• AzureQueueEventWriteService: Uses globalSettings.Events.ConnectionString

Consideration: Should this use GlobalSettings.Events instead for consistency? This would also ensure producer and consumer configurations are co-located, reducing misconfiguration risk.

Potential approach:

1. Add a QueueName property to the ConnectionStringSettings class
2. Inject GlobalSettings instead of IConfiguration
3. Read as _globalSettings.Events.QueueName

This isn't required for functionality but would improve architectural consistency.

---

♻️ Finding 5: Missing test coverage

Location: src/EventsProcessor/AzureQueueHostedService.cs

Issue: No unit tests exist for AzureQueueHostedService. This change adds new conditional logic that should be tested:

1. Service starts normally when both connection string and queue name are provided
2. Service exits early when connection string is missing
3. Service exits early when queue name is missing
4. Service exits early when queue name is empty/whitespace

Recommendation: Add unit tests covering the new configuration validation logic. Per CONTRIBUTING.md: "ALWAYS add unit tests (with mocking) for any new feature development."

---

❓ Finding 6: Documentation needed for intentional disabling behavior

Context: The PR description states this change allows operators to disable the EventsProcessor service by not setting the queue name.

Questions:

1. Is this the recommended way to disable the EventsProcessor service in production?
2. Should this be documented in deployment/operations documentation?
3. Are there other services that expect EventsProcessor to be running?
4. Is there a feature flag or more explicit "enabled/disabled" configuration option that would be clearer?

Recommendation: Document the intended use case and operational implications of disabling this service.

---

Summary

Must address before merging:

1. ❌ Configuration file updates (CRITICAL) - Add azureQueueServiceQueueName to all appsettings files
2. ⚠️ Operational logging (IMPORTANT) - Log when service is disabled to aid debugging
3. ⚠️ Architectural alignment (IMPORTANT) - Address inconsistency between producer and consumer configurations

Recommended improvements: 4. ♻️ Test coverage - Add unit tests per contributing guidelines 5. 🎨 Configuration pattern - Consider using GlobalSettings for consistency 6. ❓ Documentation - Clarify intended use cases and deployment requirements

[github-actions[bot]]

Checkmarx One – Scan Summary & Details – ca01ebd1-dd0e-41e1-94c9-8401c89cdb76

Great job! No new security vulnerabilities introduced in this pull request

[codecov[bot]]

Codecov Report

:x: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 53.88%. Comparing base (6d5d7e5) to head (44decfd). :warning: Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/EventsProcessor/AzureQueueHostedService.cs	0.00%	4 Missing :warning:

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6718   +/-   ##
=======================================
  Coverage   53.87%   53.88%           
=======================================
  Files        1914     1914           
  Lines       84957    84959    +2     
  Branches     7599     7599           
=======================================
+ Hits        45773    45782    +9     
+ Misses      37422    37415    -7     
  Partials     1762     1762           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.