fix: correct errors in System Configuration docs (#718)

[DeepDiver1975]

Fixes #718.

Corrects the factual and formatting errors reported in #718 for the new 7.1 "System Configuration" page. All changes were verified against the client source on branch 7.1 (src/libsync/config/appconfig.cpp / appconfig.h and openidconfig.cpp).

Changes

1. ServerUrl placeholder brackets removed — the INI and registry examples used a concrete value, so the <> (placeholder notation) was wrong. The ClientId/ClientSecret placeholders keep their brackets. Matches the bracket-free examples in appconfig.h.
2. Precedence statement corrected — system configuration always takes precedence over the built-in defaults (the non-OIDC values are assigned unconditionally in AppConfig::AppConfig()). OIDC is the one exception, applied only when the keys form a valid configuration.
3. Resolved config file paths stated — the "Configuration File Format" section now lists the concrete macOS and Linux paths instead of leaving the reader to resolve the templated path snippets.
4. OIDC validity rule fixed — "all keys must be defined" was false. OpenIdConfig::isValid() is !_clientId.isEmpty() && !_ports.isEmpty(), and ports defaults to 0 (any port) when empty, so in practice only ClientId must be non-empty.
5. Note added: a valid OIDC config replaces the built-in OIDC config entirely — if (systemConfig.isValid()) _openIdConfig = systemConfig; is a wholesale replacement, not a merge. Unset keys use their empty/default value (e.g. an empty Prompt, so no prompt is sent).
6. Casing made consistent with the client — the client derives the macOS/Linux file paths from the lowercase Theme::appName() (/etc/owncloud/owncloud.ini, /Library/Preferences/com.owncloud.desktopclient/owncloud.ini), while the Windows registry path uses the camel-case vendor/app name. The page previously templated all three with the camel-case {ini_name}, rendering wrong-cased file paths that also contradicted the new resolved-paths list. A page-local {app_name_lc} attribute now drives the macOS/Linux paths (Windows unchanged). The shared conditional_naming.adoc partial was intentionally left untouched.
7. Typo fix: "OpenID Connectsettings" → "OpenID Connect settings".

Related client bug (not fixed here)

While verifying, I found that AppConfig::loadOpenIdConfigFromSystemConfig() reads the OIDC Prompt from OidcPortsKey instead of OidcPromptKey, so the configured prompt is never read from system config. Filed upstream as owncloud/client#12557.

Validation

npm run antora-local currently fails in this environment due to a Node 18 / undici incompatibility (ReferenceError: File is not defined) that triggers before any content is processed — unrelated to these changes. Instead, the page was rendered directly with asciidoctor (the project's pinned version) for both targetbuild=oc and targetbuild=kw: no errors or warnings, and the resolved paths render correctly and consistently for both brands.

🤖 Generated with Claude Code

[DeepDiver1975]

🤖 Automated review by Claude Code review agent.

Overview

This PR corrects factual and formatting errors in the new 7.1 System Configuration page (modules/ROOT/pages/system_configuration.adoc), fixing #718. I verified every claim against the client source on branch 7.1 (src/libsync/config/appconfig.cpp, appconfig.h, openidconfig.cpp) and the conditional_naming.adoc partial. The corrections are accurate and the AsciiDoc is sound. This is a solid, well-justified documentation fix.

Code quality / style

• The new {app_name_lc} attribute is scoped page-locally via ifeval blocks per targetbuild (oc/kw), mirroring the existing pattern. Leaving the shared conditional_naming.adoc partial untouched is the right call — {ini_name} is camel-case there (ownCloud / Kiteworks) and other pages depend on it.
• The explanatory comment above the new attribute clearly documents why lowercase is needed (derived from Theme::appName()), which prevents this regressing later.
• subs="attributes+" is correctly applied to the new resolved-paths list and INI/registry blocks so attributes expand.
• The <<OpenID Connect>> xref target matches the in-page === OpenID Connect heading. Good.

Verification of the factual corrections

All confirmed against 7.1 source:

1. Casing / paths — AppConfig::configPath() builds macOS as /Library/Preferences/{orgDomainName}/{appName}.ini and Linux as /etc/{appName}/{appName}.ini, where appName() is lowercase; Windows uses vendor() + appNameGUI() (camel-case). The old {ini_name}.ini rendered ownCloud.ini, which was wrong. The {app_name_lc} fix (and Windows left on {bin_name}/{ini_name}) is correct.
2. Precedence — _serverUrl, _allowServerURLChange, _skipUpdateCheck are assigned unconditionally from the QSettings system config, overriding theme/built-in defaults. Correct.
3. OIDC validity — OpenIdConfig::isValid() is !_clientId.isEmpty() && !_ports.isEmpty(). Crucially, loadOpenIdConfigFromSystemConfig() appends 0 ("any port") whenever ports parse empty, so _ports is never empty for a system config — in practice only a non-empty ClientId is required. The rewritten text captures this precisely. The old "all keys must be defined" was indeed false.
4. Wholesale replacement — if (systemConfig.isValid()) _openIdConfig = systemConfig; is a full assignment, not a merge. The new NOTE is accurate.
5. Upstream bug correctly excluded — confirmed: loadOpenIdConfigFromSystemConfig() reads prompt from OidcPortsKey instead of OidcPromptKey. Documenting intended behavior and filing the code bug separately (owncloud/client#12557) is the right separation of concerns.
6. Typo fix "OpenID Connectsettings" → "OpenID Connect settings" is correct.

Specific suggestions (non-blocking)

• INI section name for OIDC. The client keys are OpenIDConnect/... (e.g. OpenIDConnect/ClientId), i.e. the INI section header is [OpenIDConnect] (no space). It's worth confirming the page's OIDC example/section header renders [OpenIDConnect] and not [OpenID Connect], since a space there would be a real, copy-paste-breaking error. This is outside the current diff but is the same class of bug #718 was about.
• NOTE example wording. The example ("if only ClientId and Ports are set, the Prompt is empty") describes intended behavior, which is correct as documentation. Given the live Prompt/Ports key-mixup bug, a reader testing today would observe the Ports string leaking into Prompt. Optional: a one-line forward-reference to the known bug here would preempt confusion, though it's reasonable to keep the doc aspirational and track the fix upstream.
• Resolved-paths list duplication. The macOS/Linux paths now appear both in the per-platform [source] blocks and again in the "Configuration File Format" resolved list. This is helpful redundancy, but if the path scheme ever changes, both spots must be updated. Minor — acceptable for readability.

Potential issues / risks

• No broken xrefs, no AsciiDoc syntax errors spotted. Attribute substitution is correctly gated.
• The {app_name_lc} attribute is defined for both oc and kw; if a third targetbuild is ever introduced the attribute would be undefined and render literally. Low risk given the current two-brand setup, but a fallback default (e.g. set before the ifeval blocks) would be defensive.
• Antora build was not run end-to-end (Node 18/undici incompatibility per the PR description); direct asciidoctor rendering for both brands is a reasonable substitute and the attribute logic is simple enough to trust.

Verdict

Accurate, well-scoped, and faithful to the client source. Recommend merge after a quick confirmation of the OIDC INI section header ([OpenIDConnect]).

[DeepDiver1975]

🤖 Automated review by Claude Code review agent.

Overview

This PR fixes factual and formatting errors on the 7.1 "System Configuration" page (modules/ROOT/pages/system_configuration.adoc), single file, +36/-12.

Since the previous review (cc2c0be → 011ee89): exactly one new commit, docs: clarify macOS config directory is the reverse-domain identifier. It adds a single sentence to the macOS section:

The directory {ini_rev_domain} is the application's reverse-domain identifier. This directory and file must be accessible to the application for reading purposes.

This is a small, accurate clarification with no regressions to the previously verified content.

Prior-flagged item: [OpenIDConnect] INI section header

Status: confirmed CORRECT. I verified against the client source on branch 7.1 (src/libsync/config/appconfig.h). The OIDC keys are defined with the OpenIDConnect/ group prefix (no space), e.g.:

inline static const QString OidcClientIdKey = QStringLiteral("OpenIDConnect/ClientId");

The class doc comment in appconfig.h likewise uses [OpenIDConnect] for INI and ...\OpenIDConnect for the registry. The doc's INI example ([OpenIDConnect]) and registry example (...\OpenIDConnect) both match exactly. No change needed — this item is resolved/accurate.

Verification of the substantive corrections

I re-verified the load-bearing claims against the client source:

• OIDC validity rule — OpenIdConfig::isValid() is !_clientId.isEmpty() && !_ports.isEmpty(). loadOpenIdConfigFromSystemConfig() appends port 0 when no ports are parsed, so _ports is never empty in practice and only a non-empty ClientId is effectively required. The page's wording ("At a minimum, ClientId must be set"; "Ports may be omitted or empty (the application then selects any available port)") is accurate.
• Wholesale replacement — if (systemConfig.isValid()) _openIdConfig = systemConfig; is an assignment, not a merge. The new NOTE block describing "in its entirety" / "not merged" is correct.
• Precedence — non-OIDC system values are assigned unconditionally in the constructor; OIDC is the conditional exception. The revised Introduction matches this.

Code quality / style (AsciiDoc / Antora)

• AsciiDoc is well-formed: the new :app_name_lc: attribute is defined for both oc and kw via ifeval, mirroring the existing brand-conditional pattern. The subs="attributes+" on the resolved-paths list and code blocks is required for attribute substitution and is present.
• The internal xref <<OpenID Connect>> resolves to the existing === OpenID Connect section title — valid.
• No broken external links or xrefs introduced.
• Registry block correctly retains +...+ passthrough fencing and subs="attributes+,+macros".

Specific suggestions (non-blocking)

1. The Prompt example value select_account consent mixes two prompt values; for an OIDC prompt parameter this is space-delimited per spec, which the docs state correctly — fine. No action.
2. Minor consistency: the macOS resolved path bullet and the macOS source block now both render {ini_rev_domain}, but only the source block previously did; the added explanatory sentence is the right fix and keeps them consistent. Good.
3. Consider (future, out of scope) cross-linking the upstream client bug noted in the PR description (owncloud/client#12557, Prompt read from OidcPortsKey) so the Prompt doc row doesn't silently mislead until the client is fixed — but the docs themselves are correct.

Potential issues / risks

• The PR description notes npm run antora-local fails due to a Node 18 / undici incompatibility unrelated to the change; rendering was instead validated directly with asciidoctor. This is an environment limitation, not a content defect. CI on the repo should still exercise the real Antora build.
• Low risk overall: single-file documentation change, brand-conditional logic follows established patterns, and all corrected facts check out against the client source.

Recommendation: approve. Accurate, well-scoped documentation fix; the previously flagged [OpenIDConnect] header is confirmed correct and the only new commit is a benign clarification.