Skip to content

Get first domain from list#2826

Merged
abnegate merged 1 commit intomainfrom
fix-domains
Feb 4, 2026
Merged

Get first domain from list#2826
abnegate merged 1 commit intomainfrom
fix-domains

Conversation

@abnegate
Copy link
Member

@abnegate abnegate commented Feb 4, 2026

What does this PR do?

(Provide a description of what this PR does.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

Summary by CodeRabbit

  • Bug Fixes
    • Improved domain variable processing to properly normalize multi-domain site configurations across console and project settings.

@appwrite
Copy link

appwrite bot commented Feb 4, 2026

Console (appwrite/console)

Project ID: 688b7bf400350cbd60e9

Sites (1)
Site Status Logs Preview QR
 console-stage
688b7cf6003b1842c9dc
Ready Ready View Logs Preview URL QR Code

Tip

Our Discord community has grown to 24K developers, and counting

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 4, 2026

Walkthrough

This PR introduces utility functions for normalizing console variables by extracting the first domain from comma-separated domain lists. A new public function normalizeConsoleVariables and private helper getFirstDomain are added to src/lib/helpers/domains.ts. These utilities are then integrated into two layout files where console variables are fetched, applying the normalization step after data retrieval by transforming raw variables before downstream use.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding logic to extract the first domain from comma-separated domain lists across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-domains

Important

Action Needed: IP Allowlist Update

If your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:

  • 136.113.208.247/32 (new)
  • 34.170.211.100/32
  • 35.222.179.152/32

Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/lib/helpers/domains.ts`:
- Around line 11-25: normalizeConsoleVariables currently converts an undefined
_APP_DOMAIN_SITES into an empty string, breaking callers that rely on nullish
coalescing; change normalizeConsoleVariables and getFirstDomain so normalization
only happens when a value is present: update getFirstDomain(domainList: string |
undefined) to return string | undefined and return undefined when domainList is
falsy, and in normalizeConsoleVariables only set _APP_DOMAIN_SITES:
getFirstDomain(variables._APP_DOMAIN_SITES) if variables._APP_DOMAIN_SITES !==
undefined (or spread the property conditionally) so undefined inputs remain
undefined and existing ?? fallbacks keep working; reference functions:
normalizeConsoleVariables and getFirstDomain.
🧹 Nitpick comments (1)
src/lib/helpers/domains.ts (1)

7-10: Trim non-essential doc comments per repo style.

These blocks describe straightforward behavior; consider removing to keep comments minimal.

💡 Suggested cleanup
-/**
- * Normalizes console variables by extracting the first domain from comma-separated domain lists.
- * This handles the backend sending `_APP_DOMAIN_SITES` (and similar fields) as comma-separated values.
- */
 export function normalizeConsoleVariables(
     variables: Models.ConsoleVariables
 ): Models.ConsoleVariables {
@@
-/**
- * Extracts the first domain from a potentially comma-separated list of domains.
- */
 function getFirstDomain(domainList: string | undefined): string {
As per coding guidelines, "Use minimal comments; only comment TODOs or complex logic".

Also applies to: 20-22

@abnegate abnegate merged commit f3f8768 into main Feb 4, 2026
4 checks passed
@abnegate abnegate deleted the fix-domains branch February 4, 2026 04:44
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