Skip to content

refactor(config): add config lexicon for app config keys#13342

Merged
kesselb merged 1 commit into
mainfrom
refactor/config-lexicon
Jul 23, 2026
Merged

refactor(config): add config lexicon for app config keys#13342
kesselb merged 1 commit into
mainfrom
refactor/config-lexicon

Conversation

@ChristophWurst

@ChristophWurst ChristophWurst commented Jul 22, 2026

Copy link
Copy Markdown
Member

Declare the Mail app's appconfig keys in a ConfigLexicon with their type, default and definition, and route call sites through key constants and the typed IAppConfig API instead of scattered string literals and mixed accessors.

Scope is appconfig values only. User preferences (stored via IUserPreferences with hyphenated keys) remain out of scope and unchanged.

Assisted-by: Claude:claude-opus-4-8

Ref https://docs.nextcloud.com/server/latest/developer_manual/digging_deeper/config/lexicon.html

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Copilot AI left a comment

Copy link
Copy Markdown

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 centralizes the Mail app’s instance-level appconfig keys in a dedicated ConfigLexicon, and updates call sites to use key constants plus typed IAppConfig accessors instead of scattered string literals and stringly-typed reads/writes.

Changes:

  • Introduce OCA\Mail\ConfigLexicon and register it via the app bootstrap.
  • Migrate multiple services/controllers/integrations from IConfig/string values (yes/no) to IAppConfig typed reads/writes with ConfigLexicon::* keys.
  • Add/update unit tests to cover the lexicon and the updated typed config access patterns.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/Unit/Service/ContextChat/ContextChatSettingsServiceTest.php Adds unit coverage for Context Chat indexing default + preference fallback using typed app config.
tests/Unit/Service/Classification/ClassificationSettingsServiceTest.php Updates tests to expect boolean appconfig access for classification defaults.
tests/Unit/Service/AiIntegrationsServiceTest.php Updates tests to use boolean appconfig for llm_processing.
tests/Unit/Controller/PageControllerTest.php Adjusts controller test wiring and expectations for mixed IConfig vs typed IAppConfig usage.
tests/Unit/Controller/AccountsControllerTest.php Updates controller tests to mock typed appconfig gating for account creation.
tests/Unit/ConfigLexiconTest.php Adds a focused test ensuring lexicon key set, value types, and defaults match expectations.
lib/Settings/AdminSettings.php Switches admin settings initial state to typed IAppConfig reads via ConfigLexicon.
lib/Service/ContextChat/ContextChatSettingsService.php Migrates Context Chat indexing default to typed boolean appconfig key.
lib/Service/Classification/ClassificationSettingsService.php Migrates classification default to typed boolean appconfig key.
lib/Service/AntiSpamService.php Routes antispam forwarding config through lexicon keys and typed appconfig API.
lib/Service/AiIntegrations/AiIntegrationsService.php Switches llm_processing to typed boolean appconfig read.
lib/Listener/NewMessagesSummarizeListener.php Uses lexicon key constant for the LLM processing toggle check.
lib/Integration/MicrosoftIntegration.php Moves Microsoft OAuth config storage/reads to typed IAppConfig + lexicon keys.
lib/Integration/GoogleIntegration.php Moves Google OAuth config storage/reads to typed IAppConfig + lexicon keys.
lib/Controller/SettingsController.php Updates admin endpoints to write typed appconfig values using lexicon keys.
lib/Controller/PageController.php Uses typed appconfig reads (lexicon keys) for initial-state values used by the frontend.
lib/Controller/AccountsController.php Gates manual account creation via typed boolean appconfig setting.
lib/ConfigLexicon.php Adds the central typed declaration of Mail app appconfig keys (type/default/definition).
lib/AppInfo/Application.php Registers the Mail ConfigLexicon during app bootstrap.

public function testIsIndexingEnabledByDefault(): void {
$this->appConfig->expects(self::once())
->method('getValueBool')
->with(Application::APP_ID, 'index_context_chat_default', false)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why would you not use the const also in the tests?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good point

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agree, makes it easier to understand the whole impact of a config key.

@SebastianKrupinski

Copy link
Copy Markdown
Contributor

Lol. I started a PR for the DAV app to do the exact same thing yesterday 🤣

@ChristophWurst

Copy link
Copy Markdown
Member Author

Tests are now using the constants too

Declare the Mail app's appconfig keys in a ConfigLexicon with their
type, default and definition, and route call sites through key constants
and the typed IAppConfig API instead of scattered string literals and
mixed accessors.

Scope is appconfig values only. User preferences (stored via
IUserPreferences with hyphenated keys) remain out of scope and unchanged.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com>
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
@kesselb
kesselb force-pushed the refactor/config-lexicon branch from e696ef9 to af4ff37 Compare July 23, 2026 11:47
@kesselb
kesselb enabled auto-merge July 23, 2026 11:47
@kesselb
kesselb merged commit f989109 into main Jul 23, 2026
55 of 61 checks passed
@kesselb
kesselb deleted the refactor/config-lexicon branch July 23, 2026 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants