# Fix: Save button disabled when timezone changes (#2343) #2361
Conversation
- Added timezone field to frontend validation schema - Added timezone field to backend validation schema - Fixed state synchronization in Settings component
WalkthroughThe changes update both client and server validation schemas to include an optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPage
participant ReduxStore
participant ValidationLayer
User->>SettingsPage: Change timezone
SettingsPage->>ReduxStore: Dispatch setTimezone action
SettingsPage->>SettingsPage: Update local settingsData.timezone
SettingsPage->>ValidationLayer: Validate settingsData (with timezone)
ValidationLayer-->>SettingsPage: Validation result
Assessment against linked issues
Why did the Canadian developer refuse to use a timezone? Because he didn’t want to lose track of his moose-ments! 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🔇 Additional comments (3)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
Auto Pull Request Review from LlamaPReview
1. Overview
1.1 Core Changes
- Primary purpose and scope: Fix disabled Save button when changing timezone by adding validation schemas and fixing state synchronization
- Key components modified:
- Frontend validation schema (validation.js)
- Backend validation schema (joi.js)
- Settings component state management
- Cross-component impacts: Affects both frontend validation and backend API input handling
- Business value alignment: Improves user experience in critical settings configuration area
2. Critical Findings
2.1 Must Fix (P0🔴)
No critical must-fix issues found
2.2 Should Fix (P1🟡)
Issue: Timezone validation lacks required constraint
- Analysis Confidence: High
- Impact: Allows empty/invalid timezone submissions potentially leading to system inconsistencies
- Suggested Solution: Update validation schemas to require valid timezone:
// Frontend
timezone: joi.string().required().valid(...IANA_TIMEZONES)
// Backend
timezone: joi.string().required()2.3 Consider (P2🟢)
Area: Timezone value validation
- Analysis Confidence: Medium
- Improvement Opportunity: Add IANA timezone validation to prevent invalid values
Area: State management redundancy
- Analysis Confidence: Low
- Improvement Opportunity: Investigate if double state update is necessary in Settings component
2.4 Summary of Action Items
- Strengthen timezone validation (P1 - immediate)
- Verify timezone value format handling (P2 - next sprint)
- Review state update logic (P2 - next sprint)
3. Technical Analysis
3.1 Code Logic Analysis
📁 client/src/Validation/validation.js - settingsValidation
- Submitted PR Code:
timezone: joi.string().allow("").optional(),- Analysis:
- Allows empty string and optional submission
- Missing validation for valid timezone format
- Impacts settings persistence reliability
- Core user preference setting requires strict validation
- LlamaPReview Suggested Improvements:
timezone: joi.string().required().valid(...IANA_TIMEZONES)
.messages({
"any.required": "Timezone is required",
"any.only": "Invalid timezone selection"
}),- Improvement rationale:
- Ensures valid system configuration
- Prevents null/invalid timezone states
- Low risk with clear error messaging
📁 server/validation/joi.js - updateAppSettingsBodyValidation
- Submitted PR Code:
timezone: joi.string().allow(""),- Analysis:
- No format validation for timezone input
- Backend accepts any string value
- Potential security/injection risk
- System-wide impact for time-sensitive operations
- LlamaPReview Suggested Improvements:
timezone: joi.string().required().pattern(/^[A-Za-z_\/]+$/),- Improvement rationale:
- Basic format validation prevents malformed inputs
- Maintains data integrity
- Low implementation cost
4. Overall Evaluation
- Technical assessment: Effective fix for reported issue with minor validation gaps
- Business impact: Critical user workflow restored with medium validation improvement potential
- Risk evaluation: Low deployment risk with suggested validations
- Notable positive aspects:
- Clear root cause analysis
- Cross-layer validation fixes
- Good state management correction
- Implementation quality: Functional solution with maintainability improvements possible
- Final recommendation: Approve with suggested validation enhancements
💡 LlamaPReview Community
Have feedback on this AI Code review tool? Join our GitHub Discussions to share your thoughts and help shape the future of LlamaPReview.
ajhollid
left a comment
ajhollid
left a comment
There was a problem hiding this comment.
Any reason to call setSettignsData in the timezone block?
Describe your changes
Problem
When changing the timezone in the Settings page, the Save button remains disabled, preventing users from saving their timezone preference. After refreshing the page, the Save button works correctly.
Root Cause
timezonefield was missing from both frontend and backend validation schemasSolution
timezonefield to the frontend validation schema invalidation.jstimezonefield to the backend validation schema inserver/validation/joi.jsWrite your issue number after "Fixes "
Fixes #2343
Please ensure all items are checked off before requesting a review. "Checked off" means you need to add an "x" character between brackets so they turn into checkmarks.
<div>Add</div>, use):Save.button.disabled.when.timezone.changes.mov
Summary by CodeRabbit
New Features
Bug Fixes