Open
Conversation
Will be enabled by setting an ASP.NET config option, e.g. with an environment variable. Config has two properties, the message which will be displayed in the UI (and set in a header returned with any API calls, so keep it short) and a ReadOnlyMode boolean which will determine whether non-read-only HTTP requests (POST, PUT, PATCH, and DELETE) will be rejected by the server. Setting a message *without* setting ReadOnlyMode is useful for giving advance notice of planned maintenance windows.
github-actions
bot
added
the
📦 Lexbox
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA maintenance mode feature is implemented across both backend services (FwHeadless and LexBox) and the frontend. The change introduces a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
Contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
backend/LexBoxApi/Config/MaintenanceModeConfig.cs (1)
3-7: Use an immutable record for consistency with DTO guidance.This options object is DTO-style and currently mutable at Line 6; making it an immutable record improves safety and consistency.
Refactor example
-public class MaintenanceModeConfig +public sealed record MaintenanceModeConfig { public bool ReadOnlyMode { get; init; } = false; - public string? MaintenanceMessage { get; set; } = null; + public string? MaintenanceMessage { get; init; } = null; }As per coding guidelines:
backend/**/*.cs: Use Records for DTOs and immutable data structures in C#.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/LexBoxApi/Config/MaintenanceModeConfig.cs` around lines 3 - 7, Replace the mutable class MaintenanceModeConfig with an immutable record named MaintenanceModeConfig; convert the mutable property MaintenanceMessage to an init-only property (or use positional record parameters) and ensure ReadOnlyMode remains init-only with its default false value so the DTO is immutable and consistent with the project's Record-for-DTO guideline.backend/FwHeadless/MaintenanceModeConfig.cs (1)
3-7: Prefer an immutable record for this options DTO.This config model is DTO-like and currently mutable (Line 6). Converting to an immutable record aligns better with the backend guideline.
Refactor example
-public class MaintenanceModeConfig +public sealed record MaintenanceModeConfig { public bool ReadOnlyMode { get; init; } = false; - public string? MaintenanceMessage { get; set; } = null; + public string? MaintenanceMessage { get; init; } = null; }As per coding guidelines:
backend/**/*.cs: Use Records for DTOs and immutable data structures in C#.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/FwHeadless/MaintenanceModeConfig.cs` around lines 3 - 7, Replace the mutable class MaintenanceModeConfig with an immutable record named MaintenanceModeConfig and make ReadOnlyMode and MaintenanceMessage immutable (init-only) with the same default values; update the declaration so the DTO is a record type (preserving the property names ReadOnlyMode and MaintenanceMessage and their defaults) to follow backend/**/*.cs guideline for records as DTOs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/FwHeadless/appsettings.json`:
- Around line 12-15: The JSON configuration section is named "MaintenanceMode"
but the code in FwHeadlessKernel.cs calls
.BindConfiguration("MaintenanceModeConfig"), causing the binding to fail; fix by
making names match: either rename the JSON key from "MaintenanceMode" to
"MaintenanceModeConfig" in appsettings.json, or change the C# call in
FwHeadlessKernel.cs from .BindConfiguration("MaintenanceModeConfig") to
.BindConfiguration("MaintenanceMode") so the binder picks up the section; update
whichever side you choose and ensure the POCO/usage still matches the chosen
section name.
In `@backend/FwHeadless/FwHeadlessKernel.cs`:
- Around line 24-27: The binding key string passed to
AddOptions<MaintenanceModeConfig>() is incorrect: replace the configuration key
"MaintenanceModeConfig" with "MaintenanceMode" in the
services.AddOptions<MaintenanceModeConfig>().BindConfiguration(...) call (this
same change should also be applied to the analogous call in LexBoxKernel where
AddOptions<MaintenanceModeConfig>() is used) so the MaintenanceModeConfig
options are bound to the actual "MaintenanceMode" configuration section.
In `@backend/LexBoxApi/appsettings.json`:
- Around line 49-52: The configuration section name in appsettings.json is
"MaintenanceMode" but LexBoxKernel.cs binds to "MaintenanceModeConfig"; open the
LexBoxKernel class and change the configuration binding key from
"MaintenanceModeConfig" to "MaintenanceMode" (e.g., update the call that
configures or binds the MaintenanceMode/maintenance config in LexBoxKernel.cs so
it matches the JSON section name).
In `@backend/LexBoxApi/LexBoxKernel.cs`:
- Around line 61-64: The options binding is using the wrong section name; update
the BindConfiguration call used with
services.AddOptions<MaintenanceModeConfig>() to reference the actual appsettings
section ("MaintenanceMode" instead of "MaintenanceModeConfig") so the
MaintenanceModeConfig options are populated (keep the existing
ValidateDataAnnotations() and ValidateOnStart() calls intact).
---
Nitpick comments:
In `@backend/FwHeadless/MaintenanceModeConfig.cs`:
- Around line 3-7: Replace the mutable class MaintenanceModeConfig with an
immutable record named MaintenanceModeConfig and make ReadOnlyMode and
MaintenanceMessage immutable (init-only) with the same default values; update
the declaration so the DTO is a record type (preserving the property names
ReadOnlyMode and MaintenanceMessage and their defaults) to follow
backend/**/*.cs guideline for records as DTOs.
In `@backend/LexBoxApi/Config/MaintenanceModeConfig.cs`:
- Around line 3-7: Replace the mutable class MaintenanceModeConfig with an
immutable record named MaintenanceModeConfig; convert the mutable property
MaintenanceMessage to an init-only property (or use positional record
parameters) and ensure ReadOnlyMode remains init-only with its default false
value so the DTO is immutable and consistent with the project's Record-for-DTO
guideline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 24eff584-e695-47e9-9c44-15f82bcd4e33
📒 Files selected for processing (15)
backend/FwHeadless/FwHeadlessKernel.csbackend/FwHeadless/MaintenanceModeConfig.csbackend/FwHeadless/MaintenanceModeMiddleware.csbackend/FwHeadless/Program.csbackend/FwHeadless/appsettings.Development.jsonbackend/FwHeadless/appsettings.jsonbackend/LexBoxApi/Config/MaintenanceModeConfig.csbackend/LexBoxApi/LexBoxKernel.csbackend/LexBoxApi/Middleware/MaintenanceModeMiddleware.csbackend/LexBoxApi/Program.csbackend/LexBoxApi/appsettings.Development.jsonbackend/LexBoxApi/appsettings.jsonfrontend/src/hooks.client.tsfrontend/src/hooks.server.tsfrontend/src/lib/layout/AppBar.svelte
Contributor
Author
|
Perhaps we should just use the |
Contributor
Author
Contributor
Author
|
Perhaps put this into WebServiceDefaults.Extensions... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Will be enabled by setting an ASP.NET config option, e.g. with an environment variable.
Config has two properties, the message which will be displayed in the UI (and set in a header returned with any API calls, so keep it short) and a ReadOnlyMode boolean which will determine whether non-read-only HTTP requests (POST, PUT, PATCH, and DELETE) will be rejected by the server.
Setting a message without setting ReadOnlyMode is useful for giving advance notice of planned maintenance windows.
This does not yet address #1956 because some FwLite client changes will be needed to address what happens when Lexbox is in maintenance mode, but it does fix #2238 with a message displayed in the Lexbox UI (a banner across the top, in the same place as the "This is a development environment" banner).