Fixes #13628: System configuration to visually distinguish different systems #13788
Conversation
…system types. - Change the menu's background-color.
Pa-Touche
changed the title
Pa-Touche
changed the title
📝 WalkthroughWalkthroughAdds system-configurable menu background color and optional subtitle, UI styling updates, two new validation keys, a small startup log, a broad SQL schema migration, a gitignore rule, and a new CSS constant. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
🤖 Fix all issues with AI agents
In `@sormas-ui/src/main/java/de/symeda/sormas/ui/Menu.java`:
- Around line 145-163: The early-return condition in
defineCustomColorBackgroundIfSystemConfigured() is inverted: replace the check
with if (StringUtils.isBlank(backgroundColor) ||
"default".equals(backgroundColor)) return; so customized colors are applied only
when a non-default, non-blank value exists; also make the added style name match
the CSS selector by changing addStyleName("color-background") to
addStyleName("valo-menu-color-background") (or alternatively update the CSS
selector to ".color-background") so the runtime style actually targets the menu
component; keep the use of determineActualColor(...) and
Page.getCurrent().getStyles() unchanged.
- Around line 158-162: The CSS gradient string built in Menu.java (the
styles.add(...) block that uses actualColorBackgroundColor) includes an
accidental "#EEFA5F" concatenated with actualColorBackgroundColor producing
invalid color tokens (e.g. "#EEFA5F#108548"); remove the stray "#EEFA5F" from
both the -webkit-linear-gradient and linear-gradient fragments so each gradient
stop uses actualColorBackgroundColor (i.e., build gradients like
"-webkit-linear-gradient(right, COLOR 0%, COLOR 8px)" and "linear-gradient(to
left, COLOR 0%, COLOR 8px)"), keeping the existing spacing/percent values and
!important flags.
🧹 Nitpick comments (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/Menu.java (1)
165-184: Consider using a Map for color mappings.The switch statement works but a
Map<String, String>constant would be more concise and easier to extend:♻️ Optional refactor suggestion
private static final Map<String, String> COLOR_MAP = Map.of( "green", "#108548", "red", "#dd2b0e", "indigo", "#7b58cf", "gray", "#737278" ); private static `@NotNull` String determineActualColor(String backgroundColor) { return COLOR_MAP.getOrDefault(backgroundColor, backgroundColor); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.gitignoresormas-ui/src/main/java/de/symeda/sormas/ui/Menu.java
🧰 Additional context used
🧬 Code graph analysis (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/Menu.java (1)
sormas-api/src/main/java/de/symeda/sormas/api/FacadeProvider.java (1)
FacadeProvider(128-599)
⏰ 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). (1)
- GitHub Check: SORMAS CI
🔇 Additional comments (3)
.gitignore (1)
49-50: LGTM!The addition of
/.run-configsto ignore IntelliJ run configurations is appropriate for keeping user-specific IDE settings out of version control.sormas-ui/src/main/java/de/symeda/sormas/ui/Menu.java (2)
67-73: Good refactor: fields marked asfinal.Making
navigator,viewButtons,menuItemsLayout, andmenuPartfinal enforces immutability and clarifies intent that these references won't change after construction.
186-196: LGTM!Using
window::closemethod reference is cleaner than an equivalent lambda.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13788 |
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13788 |
…ore. I will try to remove the parent container that seem to break it.
Before refactoring code
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@sormas-backend/src/main/resources/sql/sormas_schema.sql`:
- Around line 15107-15111: The MENU_SUBTITLE validation regex in the VALUES
tuple currently only allows word characters and is too restrictive for UI
subtitles; update the regex used there (replace the existing ^\w{0,16}$) with a
permissive pattern that permits word characters plus spaces, hyphens and periods
up to 16 characters (i.e., use a character class containing \w, space, hyphen
and dot and the same length limit) so values like "Test System", "DEV-01" or
"Stage 1" are accepted.
- Line 15066: Before adding the unique constraint
uk_systemconfigurationvalue_config_key on systemconfigurationvalue(config_key),
add a cleanup step that deletes any existing duplicate rows for the specific
keys 'MENU_BACKGROUND_COLOR' and 'MENU_SUBTITLE' (i.e., remove rows where
config_key is in that set) so the ALTER TABLE ADD CONSTRAINT will not fail;
place this DELETE immediately before the ALTER TABLE statement in the migration.
♻️ Duplicate comments (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/Menu.java (1)
173-183: Bug: CSS selector mismatch and invalid gradient syntax prevent color customization from working.Two issues remain unfixed from prior review:
Style name mismatch: Line 173 adds style
"color-background"but the CSS selector on line 180 targets.valo-menu-color-background. These don't match, so the CSS rule won't apply.Invalid CSS gradient: Line 181 concatenates
#EEFA5Fdirectly withactualColorBackgroundColor, producing invalid CSS like#EEFA5F#108548.🐛 Proposed fix
- private static final String COLOR_BACKGROUND_STYLE_NAME = "color-background"; + private static final String COLOR_BACKGROUND_STYLE_NAME = "valo-menu-color-background";And fix the gradient:
styles.add( ".valo-menu-color-background {\n" + " background-color: " + actualColorBackgroundColor + " !important;\n" - + " background-image: -webkit-linear-gradient(right, " + actualColorBackgroundColor + " 0%, `#EEFA5F`" + actualColorBackgroundColor + + " background-image: -webkit-linear-gradient(right, " + actualColorBackgroundColor + " 0%, " + actualColorBackgroundColor + " 8px) !important;\n" + " background-image: linear-gradient(to left, " + actualColorBackgroundColor + " 0%, " + actualColorBackgroundColor + " 8px) !important;" + " }");
🧹 Nitpick comments (2)
sormas-ui/src/main/webapp/VAADIN/themes/sormas/components/menu.scss (1)
123-135: Verify fixed max-height won’t clip subtitle/logo combinations.
If clipping appears at larger font sizes or longer localized text, consider usingmin-heightor removing the cap.♻️ Optional tweak to reduce clipping risk
-.top-menu-container { - max-height: 115px; -} +.top-menu-container { + min-height: 115px; +}sormas-ui/src/main/java/de/symeda/sormas/ui/Menu.java (1)
201-220: Consider validating color input to prevent CSS injection.The default case (lines 216-217) passes the raw configuration value directly into CSS. While this is admin-controlled, an arbitrary string like
red; } body { display: nonecould inject malicious CSS.Consider adding validation to ensure the value is either a recognized color name or a valid hex color pattern:
♻️ Proposed validation
+ private static final java.util.regex.Pattern HEX_COLOR_PATTERN = + java.util.regex.Pattern.compile("^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$"); + private static `@NotNull` String determineActualColor(String backgroundColor) { String actualColorBackgroundColor; switch (backgroundColor) { case "green": actualColorBackgroundColor = "#108548"; break; case "red": actualColorBackgroundColor = "#dd2b0e"; break; case "indigo": actualColorBackgroundColor = "#7b58cf"; break; case "gray": actualColorBackgroundColor = "#737278"; break; default: - actualColorBackgroundColor = backgroundColor; + if (HEX_COLOR_PATTERN.matcher(backgroundColor).matches()) { + actualColorBackgroundColor = backgroundColor; + } else { + // Fall back to default gray for invalid values + actualColorBackgroundColor = "#737278"; + } } return actualColorBackgroundColor; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
sormas-api/src/main/java/de/symeda/sormas/api/i18n/Validations.javasormas-api/src/main/resources/validations.propertiessormas-backend/src/main/java/de/symeda/sormas/backend/common/StartupShutdownService.javasormas-backend/src/main/resources/sql/sormas_schema.sqlsormas-ui/src/main/java/de/symeda/sormas/ui/Menu.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/CssStyles.javasormas-ui/src/main/webapp/VAADIN/themes/sormas/components/menu.scsssormas-ui/src/main/webapp/VAADIN/themes/sormas/global.scss
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-24T12:07:28.919Z
Learnt from: roldy
Repo: SORMAS-Foundation/SORMAS-Project PR: 13784
File: sormas-api/src/main/resources/enum_haw-US.properties:287-297
Timestamp: 2025-12-24T12:07:28.919Z
Learning: In SORMAS, enum_en.properties is intentionally empty (contains only license header) and uses enum.properties as the fallback base file for English locale. This is a standard Java ResourceBundle pattern and should not be flagged as missing translations.
Applied to files:
sormas-api/src/main/resources/validations.properties
🧬 Code graph analysis (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/Menu.java (1)
sormas-api/src/main/java/de/symeda/sormas/api/FacadeProvider.java (1)
FacadeProvider(128-599)
⏰ 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). (4)
- GitHub Check: SORMAS CI
- GitHub Check: android app test (27)
- GitHub Check: android app test (28)
- GitHub Check: android app test (26)
🔇 Additional comments (13)
sormas-backend/src/main/java/de/symeda/sormas/backend/common/StartupShutdownService.java (1)
700-700: Good observability add.Logging the computed
databaseVersionhelps with startup diagnostics and aligns with the DB update flow.sormas-backend/src/main/resources/sql/sormas_schema.sql (1)
15091-15100: LGTM!The
MENU_BACKGROUND_COLORconfiguration is well-structured with a clear validation pattern supporting named colors and hex values. TheON CONFLICT DO NOTHINGensures idempotent migrations.sormas-ui/src/main/java/de/symeda/sormas/ui/utils/CssStyles.java (1)
116-119: LGTM: new underline style token.sormas-ui/src/main/webapp/VAADIN/themes/sormas/components/menu.scss (2)
63-70: Style cleanup looks fine.
75-77: Margin tweak looks fine.sormas-ui/src/main/webapp/VAADIN/themes/sormas/global.scss (1)
534-536: LGTM: global underline utility.sormas-api/src/main/resources/validations.properties (1)
332-334: Verify validation logic matches the new messages.
Please confirm the actual validators enforce the same color list/hex formats and the 16‑character subtitle limit described here to avoid misleading error text.sormas-api/src/main/java/de/symeda/sormas/api/i18n/Validations.java (1)
274-278: LGTM: new validation key constants added.sormas-ui/src/main/java/de/symeda/sormas/ui/Menu.java (5)
25-45: LGTM: Imports are correctly added for new functionality.All new imports (
NotNull,Page,VerticalLayout) are utilized by the new color customization and subtitle features.
70-83: LGTM: Well-structured constants and improved field immutability.Making
navigator,viewButtons,menuItemsLayout, andmenuPartfinal is a good practice. Inline initialization ofviewButtonsis cleaner.
89-125: LGTM: Constructor restructured for new layout composition.The introduction of
topContaineras aVerticalLayoutwrapper allows proper vertical stacking of the header and optional subtitle. CallingdefineCustomColorBackgroundIfSystemConfigured()early ensures styles are set before layout composition.
186-199: LGTM: Subtitle configuration is correctly implemented.Proper blank-check guard clause, clean label creation, and appropriate styling application.
228-228: LGTM: Method reference is cleaner than lambda.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| INSERT INTO schema_version (version_number, comment) VALUES (602, 'External message additional fields'); | ||
| -- *** Insert new sql commands BEFORE this line. Remember to always consider _history tables. *** | ||
|
|
||
| ALTER TABLE systemconfigurationvalue ADD CONSTRAINT uk_systemconfigurationvalue_config_key UNIQUE (config_key); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the exact line and surrounding context around line 15066
sed -n '15050,15100p' sormas-backend/src/main/resources/sql/sormas_schema.sqlRepository: SORMAS-Foundation/SORMAS-Project
Length of output: 2625
🏁 Script executed:
# Search for schema version 603 and the complete migration block
rg -n "603|systemconfigurationvalue" sormas-backend/src/main/resources/sql/sormas_schema.sql -A 5 -B 5 | tail -150Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 8165
🏁 Script executed:
# Check if there are any precondition checks or duplicate handling before this constraint
rg -n "SELECT.*config_key|DELETE.*systemconfigurationvalue|TRUNCATE.*systemconfigurationvalue" sormas-backend/src/main/resources/sql/sormas_schema.sqlRepository: SORMAS-Foundation/SORMAS-Project
Length of output: 126
🏁 Script executed:
# Look at the complete migration block for schema version 603
rg -n "-- schema version 603|603" sormas-backend/src/main/resources/sql/sormas_schema.sql -A 50 | head -100Repository: SORMAS-Foundation/SORMAS-Project
Length of output: 503
Add cleanup before the unique constraint to prevent migration failures.
The ALTER TABLE ADD CONSTRAINT on line 15066 will fail if duplicate config_key values exist in production. Prior schema migrations (version 569) demonstrate the correct pattern: DELETE duplicate rows before adding the constraint. Add DELETE FROM systemconfigurationvalue WHERE config_key IN ('MENU_BACKGROUND_COLOR', 'MENU_SUBTITLE'); before the constraint addition, consistent with the established migration pattern for configuration updates.
🤖 Prompt for AI Agents
In `@sormas-backend/src/main/resources/sql/sormas_schema.sql` at line 15066,
Before adding the unique constraint uk_systemconfigurationvalue_config_key on
systemconfigurationvalue(config_key), add a cleanup step that deletes any
existing duplicate rows for the specific keys 'MENU_BACKGROUND_COLOR' and
'MENU_SUBTITLE' (i.e., remove rows where config_key is in that set) so the ALTER
TABLE ADD CONSTRAINT will not fail; place this DELETE immediately before the
ALTER TABLE statement in the migration.
| VALUES ('MENU_SUBTITLE', '', general_configuration_id, true, | ||
| '^\w{0,16}$', false, | ||
| null, | ||
| 'i18n/systemConfigurationValueValidationMenuSubtitle', now(), now(), nextval('entity_seq'), | ||
| generate_base32_uuid()) |
There was a problem hiding this comment.
MENU_SUBTITLE pattern may be too restrictive for display text.
The pattern ^\w{0,16}$ only allows alphanumeric characters and underscores (no spaces, hyphens, or other punctuation). For a subtitle displayed in the UI banner, this prevents values like "Test System", "DEV-01", or "Stage 1".
If this is intentional (e.g., for a short identifier), please disregard. Otherwise, consider a more permissive pattern:
Suggested pattern allowing spaces and common punctuation
- VALUES ('MENU_SUBTITLE', '', general_configuration_id, true,
- '^\w{0,16}$', false,
+ VALUES ('MENU_SUBTITLE', '', general_configuration_id, true,
+ '^[\w\s\-\.]{0,16}$', false,This allows word characters, spaces, hyphens, and periods up to 16 characters.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| VALUES ('MENU_SUBTITLE', '', general_configuration_id, true, | |
| '^\w{0,16}$', false, | |
| null, | |
| 'i18n/systemConfigurationValueValidationMenuSubtitle', now(), now(), nextval('entity_seq'), | |
| generate_base32_uuid()) | |
| VALUES ('MENU_SUBTITLE', '', general_configuration_id, true, | |
| '^[\w\s\-\.]{0,16}$', false, | |
| null, | |
| 'i18n/systemConfigurationValueValidationMenuSubtitle', now(), now(), nextval('entity_seq'), | |
| generate_base32_uuid()) |
🤖 Prompt for AI Agents
In `@sormas-backend/src/main/resources/sql/sormas_schema.sql` around lines 15107 -
15111, The MENU_SUBTITLE validation regex in the VALUES tuple currently only
allows word characters and is too restrictive for UI subtitles; update the regex
used there (replace the existing ^\w{0,16}$) with a permissive pattern that
permits word characters plus spaces, hyphens and periods up to 16 characters
(i.e., use a character class containing \w, space, hyphen and dot and the same
length limit) so values like "Test System", "DEV-01" or "Stage 1" are accepted.
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13788 |
3 similar comments
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13788 |
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13788 |
|
SonarCloud analysis: https://sonarcloud.io/dashboard?id=SORMAS-Project&pullRequest=13788 |
Fixes #13628
TODO:
Implementation:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.