Skip to content

Conversation

@qianxuege
Copy link

Refactor: src/user/settings.js

Overview

This refactor reduces cyclomatic complexity and improves maintainability of the user settings module without changing its external behavior or public API.

Goals

  • Decrease complexity inside the exported initializer function.
  • Separate concerns: validation, normalization, notification handling, persistence.
  • Preserve existing behavior, data shape, and hook semantics.
  • Prepare the module for easier future extension and unit testing.

Key Changes

Hoisted Constants

  • spiderDefaultSettings
  • remoteDefaultSettings

Extracted / Added Helper Functions

Helper Responsibility
parseJSONSetting Safe JSON parsing with fallback
getSetting Unified resolution (user → remote default → global config → fallback)
enhanceNotificationSettings Populate notification preference defaults
onSettingsLoaded Normalize all settings after load (moved/hoisted)
validatePagination Guard page size values
validateLanguages Validate and default language fields
buildSettingsObject Construct the persisted subset of settings
applyNotificationSettings Merge user-provided notification overrides

User.saveSettings Simplification

Replaced inline logic with:

  1. validatePagination (posts & topics)
  2. validateLanguages
  3. buildSettingsObject
  4. applyNotificationSettings
  5. Hook pipeline (action:user.saveSettingsfilter:user.saveSettings)
  6. Digest scheduling unchanged

Behavior Preserved

  • Public API methods: getSettings, getMultipleUserSettings, saveSettings, updateDigestSetting, setSetting
  • Storage key names and structure
  • Default value resolution order:
    1. Explicit user value
    2. Remote default (for ActivityPub-style users)
    3. Global config (meta.config)
    4. Hardcoded fallback
  • Notification type handling derived dynamically
  • Digest frequency handling logic

Benefits

  • Lower cognitive load when reading or modifying the module
  • Clear single-purpose helpers encourage targeted unit tests
  • Reduced risk of accidental side effects when adding new fields
  • Easier future introduction of schema validation (e.g. Zod/Joi)

Non-Functional Notes

  • No database migrations required
  • No plugin or theme integration changes
  • Hook contracts untouched

Suggested Follow-Ups (Optional)

  1. Add unit tests for:
    • validatePagination
    • buildSettingsObject
    • getSetting (especially remote default precedence)
  2. Cache notification type list if profiling shows repetition cost
  3. Introduce schema-based validation layer
  4. Further split onSettingsLoaded into:
    • Boolean coercions
    • Pagination normalization
    • Sorting & preference fields
  5. Add TypeScript typings or JSDoc typedef for the settings object

Safety & Compatibility

  • No breaking changes were introduced.
  • Fallbacks and defaults match pre-refactor logic.
  • Error messages for invalid pagination and language codes remain unchanged.

File Touched

  • src/user/settings.js (structural refactor only; no semantic changes to outcomes)

Quick Reference: Data Flow After Refactor

Load flow:

  1. Fetch raw object (or defaults for spider/remote)
  2. onSettingsLoaded normalizes
  3. Notification defaults via enhanceNotificationSettings
  4. JSON lists parsed (chatAllowList, chatDenyList)

Save flow:

  1. Validate pagination & languages
  2. Fire action:user.saveSettings
  3. Build canonical object
  4. Merge notification overrides
  5. Fire filter:user.saveSettings
  6. Persist & update digest scheduling
  7. Return fully normalized settings (getSettings re-run)

@qianxuege qianxuege changed the title refactor: copilot refactored User.export to decrease complexity refactor: refactor User.export to decrease complexity in src/user/settings.js Oct 2, 2025
@qianxuege qianxuege requested a review from Copilot October 2, 2025 13:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the User.export function in src/user/settings.js to reduce cyclomatic complexity by extracting helper functions and hoisting constants while preserving all existing behavior and public API contracts.

  • Extracts inline logic into focused helper functions for validation, normalization, and notification handling
  • Hoists constants and utility functions outside the main exported function to improve readability
  • Simplifies the main saveSettings method by replacing complex inline validation with extracted helper calls

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

function getSetting(settings, key, defaultValue) {
if (settings[key] || settings[key] === 0) {
return settings[key];
} else if (activitypub.helpers.isUri(settings.uid) && remoteDefaultSettings[key]) {
Copy link

Copilot AI Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The remoteDefaultSettings variable is referenced but not defined in the visible code. This will cause a ReferenceError when this condition is evaluated.

Copilot uses AI. Check for mistakes.
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.

2 participants