[ai] settings: Replace deprecated URI with URL in avatar/logo settings. by Aditya-XR · Pull Request #40191 · zulip/zulip

Aditya-XR · 2026-09-23T04:47:37Z

Zulip uses "URL" rather than the deprecated "URI" (#23380), so this renames the DEFAULT_AVATAR_URI and DEFAULT_LOGO_URI server settings to DEFAULT_AVATAR_URL and DEFAULT_LOGO_URL, one commit per setting.

Servers upgrading from an older release may still set the old names in /etc/zulip/settings.py. To keep those working, computed_settings.py copies the old value into the new setting when only the old one is set, like the Google OAuth settings fallback there. The rest of the code reads only the new names.

The third commit adds a system check (zulip.E008) for a server that sets both names to different values, where the old one would otherwise be silently ignored. It's separate so it can be dropped.

Fixes: part of #23380. The Home Assistant docs fix that was first in this PR is now #40197.

How changes were tested:

test_server_settings.py: new tests reload computed_settings to check that the old name is used when the new one is unset, and that the new one wins when both are set.
test_upload.py: the default avatar, realm icon and realm logo tests now set the new names.
test_checks.py: the check reports both settings (including the Docker wording), and stays quiet when only one name is set.

Screenshots and screen captures:

None, no visual changes.

Self-review checklist

Self-reviewed the changes for clarity and maintainability
(variable names, code reuse, readability, etc.).
Followed the AI use policy.

Communicate decisions, questions, and potential concerns.

Explains differences from previous plans (e.g., issue description).
Highlights technical choices and bugs encountered.
Calls out remaining decisions and concerns.
Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

Each commit is a coherent idea.
Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

Visual appearance of the changes.
Responsiveness and internationalization.
Strings and tooltips.
End-to-end functionality of buttons, interactions and flows.
Corner cases, error conditions, and easily imagined bugs.

Copilot

Copilot review overview

🟡 Changes recommended

Add a settings-layer test covering the legacy-to-canonical computed-settings fallback.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

Add settings-layer test for legacy URI to URL compatibility · New

What changed in this PR

Replaces deprecated avatar/logo URI settings with URL equivalents while preserving legacy compatibility and fixing the Home Assistant webhook documentation.

Changes:

Adds URL settings and legacy fallbacks.
Updates avatar/logo consumers and tests.
Uses the canonical {{ api_url }} template.

File	Summary
`zproject/prod_settings_template.py`	Updates the documented avatar setting.
`zproject/default_settings.py`	Defines URL-named settings.
`zproject/computed_settings.py`	Maps legacy settings to canonical names.
`zerver/webhooks/homeassistant/doc.md`	Uses the canonical API URL template.
`zerver/tests/test_upload.py`	Tests URL settings and legacy fallbacks.
`zerver/lib/realm_logo.py`	Uses the URL logo setting with fallback.
`zerver/lib/realm_icon.py`	Uses the URL avatar setting with fallback.
`zerver/lib/avatar.py`	Uses the URL avatar setting with fallback.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Aditya-XR · 2026-09-23T06:02:03Z

Ready for review! Fixed the CI provisioning and test issues, ensured full backward compatibility with test coverage for legacy \DEFAULT_AVATAR_URI\ and \DEFAULT_LOGO_URI, and verified all 26 CI checks pass.

laurynmm · 2026-09-23T14:51:00Z

Could you please update your commit message to match the commit style guidelines?

The integration documentation change should probably be done in a separate pull request. And make sure you include a screenshot of the fixed documentation.

Aditya-XR · 2026-09-23T16:41:41Z

@laurynmm Thanks for the review! I've made the following changes:

Rewrote both commit messages to follow the commit message
guidelines: wrapped the body text, explained the reason for the
rename and the backward-compatibility fallback, and dropped the
repeated boilerplate.
Removed the Home Assistant documentation change from this PR and
opened it separately as integrations: Fix API URL in Home Assistant documentation. #40197, with before/after screenshots of
the rendered page.

This PR now only contains the DEFAULT_AVATAR_URI and
DEFAULT_LOGO_URI renames. The code is unchanged from the previous
version; only the commit messages were updated.

Aditya-XR · 2026-10-02T13:45:03Z

Rebased on main, and dropped the second fallback I had in avatar.py, realm_icon.py and realm_logo.py: computed_settings.py already maps the old names, so those branches only ran in tests. I also moved the description onto the PR template.

laurynmm · 2026-10-05T13:38:53Z

Thanks for opening that other PR and updating the commit messages @Aditya-XR!

@Vector73 would you have time to do a review on these changes as the last person to look at and work on this area in the codebase?

The core team is starting to prep for the 13.0 release and as the issue these changes address isn't on the release priorities board, I don't anticipate anyone having availability to review in the near future. No worries if not.

Vector73

LGTM. One question: Are we going to migrate other server settings with _URI_ in the names in follow-up PRs?

Aditya-XR · 2026-10-10T01:35:15Z

Thanks for the review @Vector73!

Yes, I'd like to do the rest as follow-ups. Looking through zproject/, the ones left are CAMO_URI, EXTERNAL_URI_SCHEME, REALM_MOBILE_REMAP_URIS, and ROOT_DOMAIN_URI (that last one is computed, so it shouldn't need a fallback for the old name). AUTH_LDAP_SERVER_URI will have to stay since that name comes from django-auth-ldap.

I'll wait for this one to be merged and then do them one at a time, following the same approach as here.

laurynmm

@Aditya-XR - I did a quick review of the first commit. Anything I noted there should also be applied to the second commit. Let me know if you have any questions about my review feedback.

laurynmm · 2026-10-19T17:27:12Z

    ZULIP_SERVICE_SUBMIT_USAGE_STATISTICS,
    ZULIP_SERVICES_URL,
 )
+from .configured_settings import DEFAULT_AVATAR_URL as DEFAULT_AVATAR_URL


Isn't the list of settings above this being imported from configured_settings?

Moved DEFAULT_AVATAR_URL and DEFAULT_LOGO_URL into the existing import list above.

To make that work, the tests now read the value from what importlib.reload() returns — mypy won't let them read an imported name straight off computed_settings unless it's explicitly re-exported.

laurynmm · 2026-10-19T17:30:29Z

        STATIC_URL = "http://localhost:9991/static/"

+if DEFAULT_AVATAR_URL is None and DEFAULT_AVATAR_URI is not None:
+    DEFAULT_AVATAR_URL = DEFAULT_AVATAR_URI


This should probably have an explanation here, like we do for the Google Auth replacement.

Added a comment modeled after the Google auth fallback. I placed it once right above the block so the upcoming renames (CAMO_URI, etc.) can sit underneath it without repeating the explanation for each one.

laurynmm · 2026-10-19T17:32:08Z

 # File uploads and avatars
 # TODO: Rename MAX_FILE_UPLOAD_SIZE to have unit in name.
+DEFAULT_AVATAR_URL: str | None = None
 DEFAULT_AVATAR_URI: str | None = None


If we still need this setting here, then we need to document that it's deprecated.

laurynmm · 2026-10-19T17:50:55Z

+                "http://example.com/legacy-avatar.svg",
+            )
+
+        # Canonical DEFAULT_AVATAR_URL takes precedence when both are set


This probably won't happen in practice, but we should probably discuss if this should be logged if overridden or if it should be a configuration error that's noted when trying to start up the server.

Right now, the new name silently wins if both are set, which matches how the existing Google auth and push bouncer fallbacks behave.

Between logging and a config error, a startup check in zerver/checks.py felt like the more reliable option:

When settings are being loaded, logging isn't initialized yet, and admins can easily miss a log line during an upgrade anyway.

Both upgrades and restart-server run ./manage.py check before restarting the server, so a check will catch the conflict early with a clear message, before the new code starts running.

I went ahead and wired that up in a separate 3rd commit — it only triggers if both settings are configured to different values (and ignores the default values).

Do you think Error is the right severity here, or would a Warning be better? Or would you rather we just keep the silent fallback behavior like the other settings and drop the 3rd commit?

Zulip's naming convention is to use "URL" rather than "URI", so we rename this setting to DEFAULT_AVATAR_URL. Existing installations may still set DEFAULT_AVATAR_URI in their /etc/zulip/settings.py, so we continue to support the old name: if DEFAULT_AVATAR_URL is unset, computed_settings.py falls back to the value of DEFAULT_AVATAR_URI. Fixes part of zulip#23380.

Zulip's naming convention is to use "URL" rather than "URI", so we rename this setting to DEFAULT_LOGO_URL. Existing installations may still set DEFAULT_LOGO_URI in their /etc/zulip/settings.py, so we continue to support the old name: if DEFAULT_LOGO_URL is unset, computed_settings.py falls back to the value of DEFAULT_LOGO_URI. Fixes part of zulip#23380.

DEFAULT_AVATAR_URL and DEFAULT_LOGO_URL only fall back to their old names when they are not set themselves. So if a server sets both names to different values, the old one is silently ignored. Report that as a system check error. The upgrade process and restart-server both run these checks before restarting the server.

Aditya-XR · 2026-10-22T14:03:09Z

Thanks for the review, @laurynmm! I've rebased on main and updated both commits to address your feedback.

I also added a third commit to handle the case where both the old and new setting names are configured. I kept it separate so it's easy to drop if we decide we don't want it.

zulipbot added the size: L label Sep 23, 2026

Aditya-XR force-pushed the replace-uri-with-url-settings branch 3 times, most recently from 2dd2bd3 to febb1d2 Compare September 23, 2026 05:25

Aditya-XR marked this pull request as ready for review September 23, 2026 05:58

Copilot AI lite review requested due to automatic review settings September 23, 2026 05:58

Copilot started reviewing on behalf of Aditya-XR September 23, 2026 05:58

View session

Aditya-XR force-pushed the replace-uri-with-url-settings branch from febb1d2 to 2d3cdeb Compare September 23, 2026 05:58

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Comment thread zproject/computed_settings.py Resolved

Aditya-XR force-pushed the replace-uri-with-url-settings branch from 2d3cdeb to bbafb08 Compare September 23, 2026 06:19

zulipbot added size: XL and removed size: L labels Sep 23, 2026

Aditya-XR force-pushed the replace-uri-with-url-settings branch from bbafb08 to 8932bbf Compare September 23, 2026 06:52

laurynmm added the not ready Needs work to be ready for maintainer review. label Sep 23, 2026

Aditya-XR force-pushed the replace-uri-with-url-settings branch from 8932bbf to 182b4f8 Compare September 23, 2026 16:12

Aditya-XR changed the title ~~settings, docs: Replace deprecated URI with URL in avatar/logo settings and docs~~ settings: Replace deprecated URI with URL in avatar/logo settings. Sep 23, 2026

Aditya-XR mentioned this pull request Sep 23, 2026

integrations: Fix API URL in Home Assistant documentation. #40197

Merged

6 of 13 tasks

Aditya-XR force-pushed the replace-uri-with-url-settings branch from 182b4f8 to 3b14c15 Compare October 2, 2026 13:44

laurynmm added priority: low PRs for this issue may not receive timely reviews, especially toward the end of a release cycle and removed not ready Needs work to be ready for maintainer review. labels Oct 5, 2026

laurynmm requested a review from Vector73 October 5, 2026 13:39

Vector73 approved these changes Oct 9, 2026

View reviewed changes

Aditya-XR mentioned this pull request Oct 10, 2026

Stop using the deprecated "uri" terminology in favor of "url" #23380

Open

laurynmm reviewed Oct 19, 2026

View reviewed changes

laurynmm self-assigned this Oct 19, 2026

laurynmm added the maintainer review PR is ready for review by Zulip maintainers. label Oct 19, 2026

Aditya-XR changed the title ~~settings: Replace deprecated URI with URL in avatar/logo settings.~~ [ai] settings: Replace deprecated URI with URL in avatar/logo settings. Oct 22, 2026

Aditya-XR added 3 commits October 22, 2026 13:44

Aditya-XR force-pushed the replace-uri-with-url-settings branch from 3b14c15 to 755bfd2 Compare October 22, 2026 14:02

Aditya-XR mentioned this pull request Oct 28, 2026

[ai] settings: Rename ROOT_DOMAIN_URI to ROOT_DOMAIN_URL. #40517

Open

12 of 17 tasks

Conversation

Aditya-XR commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Aditya-XR commented Sep 23, 2026

Uh oh!

laurynmm commented Sep 23, 2026

Uh oh!

Aditya-XR commented Sep 23, 2026

Uh oh!

Aditya-XR commented Oct 2, 2026

Uh oh!

laurynmm commented Oct 5, 2026

Uh oh!

Vector73 left a comment

Choose a reason for hiding this comment

Uh oh!

Aditya-XR commented Oct 10, 2026

Uh oh!

laurynmm left a comment

Choose a reason for hiding this comment

Uh oh!

laurynmm Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

Aditya-XR Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

laurynmm Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

Aditya-XR Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

laurynmm Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

laurynmm Oct 19, 2026

Choose a reason for hiding this comment

Uh oh!

Aditya-XR Oct 22, 2026

Choose a reason for hiding this comment

Uh oh!

Aditya-XR commented Oct 22, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Aditya-XR commented Sep 23, 2026 •

edited

Loading