Skip to content

feat: add rsyslog server setting to network configuration#2410

Merged
Yeraze merged 1 commit intomainfrom
feat/rsyslog-setting
Mar 24, 2026
Merged

feat: add rsyslog server setting to network configuration#2410
Yeraze merged 1 commit intomainfrom
feat/rsyslog-setting

Conversation

@Yeraze
Copy link
Owner

@Yeraze Yeraze commented Mar 23, 2026

Summary

Adds rsyslog server field to Device Configuration > Network settings, allowing users to configure a remote syslog server address (e.g., 192.168.1.100:514).

Backend protobuf support already existed in protobufService.ts — this adds the UI and wiring.

Changes

File Change
src/components/ConfigurationTab.tsx State, load, save, and prop passing for rsyslogServer
src/components/configuration/NetworkConfigSection.tsx Props, change detection, reset, and input field
public/locales/en.json Translation keys for label and description

Test plan

  • npx vitest run — 3070 tests pass
  • npm run build — no TypeScript errors
  • Deploy on SQLite, verify field appears and persists

🤖 Generated with Claude Code

Add rsyslog server field to Device Configuration > Network settings,
allowing users to configure remote syslog server address. Backend
protobuf support already existed — this adds the UI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Mar 23, 2026

Claude finished @Yeraze's task —— View job


PR Review: Adding Rsyslog Server Setting

Reviewing this PR for code quality, best practices, potential bugs, performance considerations, security concerns, and test coverage.

Todo List:

  • Read and analyze changed files
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Assess security concerns
  • Review test coverage
  • Provide final feedback and recommendations

✅ Overall Assessment: APPROVED

This is a well-implemented and clean addition that follows existing patterns perfectly. The changes are minimal, focused, and properly integrated.

🎯 Strengths

Code Quality & Patterns

  • Excellent consistency: Follows exact same pattern as existing ntpServer field
  • Proper state management: All React hooks dependencies correctly updated
  • Clean integration: Seamlessly added to existing change detection, reset, and save logic
  • TypeScript compliance: Proper typing throughout (interfaces, props, state)

Implementation Details

  • Consistent validation: 33-character maxLength matches other network fields
  • Proper placeholder: Clear example format (192.168.1.100:514)
  • Good UX: Field positioned logically after NTP server in UI
  • Translation ready: Proper i18n keys with descriptive help text

Backend Integration

  • ✅ Protobuf support already exists in protobufService.ts:1249-1250
  • ✅ No additional backend changes needed
  • ✅ Follows device configuration pattern (not user settings)

🔍 Technical Review

Security: ✅ No Concerns

  • Input is properly constrained (33 char limit)
  • No injection risks (standard text input)
  • Follows same security model as existing network fields
  • Backend validation exists via protobuf service

Performance: ✅ Minimal Impact

  • No new API calls or database operations
  • Minimal state additions follow existing patterns
  • Change detection efficiently implemented
  • No rendering performance concerns

Testing: ✅ Adequately Covered

  • Author verified 3070 tests still pass
  • No new test logic needed (follows existing field patterns)
  • Integration testing covers the new field automatically
  • Backend protobuf handling already tested

🎯 Code Specific Notes

ConfigurationTab.tsx (src/components/ConfigurationTab.tsx:130)

const [rsyslogServer, setRsyslogServer] = useState('');
  • ✅ Perfect placement alongside other network state
  • ✅ State loading/saving properly integrated at lines 492, 1020, 1708

NetworkConfigSection.tsx

  • ✅ Props interface correctly extended (lines 20-21)
  • ✅ Change detection logic properly includes rsyslogServer (line 79)
  • ✅ Reset functionality includes new field (line 96)
  • ✅ Save callback dependencies updated (line 112)

Translations (public/locales/en.json)

"network_config.rsyslog_server": "Rsyslog Server",
"network_config.rsyslog_server_description": "Remote syslog server address (e.g., 192.168.1.100:514). Leave blank to disable."
  • ✅ Clear labeling with helpful example
  • ✅ Proper description explains purpose and optional nature

🛡️ Architecture Compliance

  • ✅ VALID_SETTINGS_KEYS: Not needed - this is device config, not user settings
  • ✅ Backend separation: Properly uses device configuration API
  • ✅ Database pattern: No database changes required (protobuf handles persistence)
  • ✅ React patterns: Follows established hook/state patterns

📋 Final Verdict

This PR is ready for merge. It demonstrates excellent understanding of the codebase patterns and implements the feature correctly with minimal, targeted changes. The implementation is clean, secure, and follows all established conventions.

No changes requested - the implementation is solid as-is.


@Yeraze Yeraze merged commit ec032cd into main Mar 24, 2026
17 checks passed
@Yeraze Yeraze deleted the feat/rsyslog-setting branch March 24, 2026 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant