Skip to content

Conversation

@ItzNotABug
Copy link
Member

@ItzNotABug ItzNotABug commented Nov 19, 2025

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

  • Refactor
    • Standardized handling of optional billing and organization parameters across the app, simplifying invocation patterns and reducing redundant arguments.
  • Chore
    • Reorganized CI workflow steps for clearer structure and improved maintainability.

@ItzNotABug ItzNotABug self-assigned this Nov 19, 2025
@appwrite
Copy link

appwrite bot commented Nov 19, 2025

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

Each function runs in its own isolated container with custom environment variables

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 19, 2025

Walkthrough

This pull request standardizes a GitHub Actions workflow file and updates the billing SDK method signatures and their call sites. The workflow YAML was reorganized into clearer steps. In src/lib/sdk/billing.ts two parameters default values were changed from null to undefined for billingAddressId in createOrganization and updatePlan. Multiple pages and components were updated to remove or replace trailing null arguments (some to undefined) and one call added a taxId argument to align with the revised signatures.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Areas requiring extra attention:

  • src/lib/sdk/billing.ts: confirm changing billingAddressId default from null to undefined is intended and safe for callers and serialization.
  • Confirm all updated call sites match the new parameter ordering/count, especially where a trailing null was removed versus replaced with undefined.
  • src/routes/(console)/create-organization/+page.svelte: verify the newly included taxId argument is correct.
  • Files with cloud/non-cloud branching (onboarding, templates, deploy, apply-credit, change-plan): ensure behavior remains consistent after argument changes.
  • .github/workflows/dockerize-profiles.yml: verify step order, dependencies, and any required environment/outputs remain intact.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changeset: replacing null default parameters with undefined across billing-related SDK methods and their call sites.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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-organization-billing

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 731209d and 2ed1d8c.

📒 Files selected for processing (3)
  • src/lib/sdk/billing.ts (2 hunks)
  • src/routes/(console)/apply-credit/+page.svelte (2 hunks)
  • src/routes/(console)/organization-[organization]/change-plan/+page.svelte (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/routes/(console)/apply-credit/+page.svelte
  • src/lib/sdk/billing.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: e2e
  • GitHub Check: build
🔇 Additional comments (1)
src/routes/(console)/organization-[organization]/change-plan/+page.svelte (1)

173-177: The updatePlan calls are correctly aligned with the updated SDK signature.

Both changes properly align with the PR's null-to-undefined migration:

  • Downgrade path (line 176): Correctly removes the 4th argument, allowing billingAddressId to default to undefined as defined in the SDK.
  • Upgrade path (line 256): Correctly passes undefined for the 4th argument (billingAddressId), matching the SDK's default value.

The upgrade path explicitly provides all 8 arguments since it has trailing parameters; the downgrade path uses only 3, relying on SDK defaults for the remaining optional parameters.


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: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/lib/sdk/billing.ts (2)

488-498: Fix type mismatch: parameter type doesn't allow undefined.

The parameter billingAddressId is typed as string but has a default value of undefined, which is a type safety violation. TypeScript's type system requires the parameter type to explicitly allow undefined if that's a possible value.

Apply this diff to fix the type declaration:

     async createOrganization(
         organizationId: string,
         name: string,
         billingPlan: string,
         paymentMethodId: string,
-        billingAddressId: string = undefined,
+        billingAddressId?: string,
         couponId: string = null,
         invites: Array<string> = [],
         budget: number = undefined,
         taxId: string = null
     ): Promise<Organization | OrganizationError> {

Alternatively, use an explicit union type:

billingAddressId: string | undefined = undefined,

610-619: Fix type mismatch: parameter type doesn't allow undefined.

Same type safety issue as in createOrganization: the parameter billingAddressId is typed as string but defaults to undefined.

Apply this diff to fix the type declaration:

     async updatePlan(
         organizationId: string,
         billingPlan: string,
         paymentMethodId: string,
-        billingAddressId: string = undefined,
+        billingAddressId?: string,
         couponId: string = null,
         invites: Array<string> = [],
         budget: number = undefined,
         taxId: string = null
     ): Promise<Organization | OrganizationError> {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2f91d97 and 731209d.

📒 Files selected for processing (9)
  • .github/workflows/dockerize-profiles.yml (1 hunks)
  • src/lib/sdk/billing.ts (1 hunks)
  • src/routes/(console)/apply-credit/+page.svelte (1 hunks)
  • src/routes/(console)/create-organization/+page.svelte (1 hunks)
  • src/routes/(console)/onboarding/create-organization/+page.svelte (0 hunks)
  • src/routes/(console)/onboarding/create-project/+page.ts (0 hunks)
  • src/routes/(console)/organization-[organization]/change-plan/+page.svelte (1 hunks)
  • src/routes/(public)/functions/deploy/+page.ts (0 hunks)
  • src/routes/(public)/template-[template]/+page.ts (0 hunks)
💤 Files with no reviewable changes (4)
  • src/routes/(public)/template-[template]/+page.ts
  • src/routes/(console)/onboarding/create-project/+page.ts
  • src/routes/(console)/onboarding/create-organization/+page.svelte
  • src/routes/(public)/functions/deploy/+page.ts
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/dockerize-profiles.yml

17-17: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


19-19: the runner of "docker/setup-qemu-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


21-21: the runner of "docker/setup-buildx-action@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: e2e
  • GitHub Check: build
🔇 Additional comments (4)
.github/workflows/dockerize-profiles.yml (1)

15-49: Workflow structure reorganization looks good.

The restructuring of steps into explicit sequential stages (Checkout, QEMU setup, Buildx setup, Docker Hub login, metadata extraction, and build/push) improves readability and maintainability.

src/routes/(console)/organization-[organization]/change-plan/+page.svelte (1)

173-177: LGTM! Cleaner function call by relying on default parameter.

The removal of the trailing null argument correctly relies on the SDK's default value for billingAddressId, making the call more concise.

src/routes/(console)/apply-credit/+page.svelte (1)

132-142: LGTM! Argument updated to match SDK default.

The change from null to undefined for billingAddressId aligns with the SDK's updated default parameter value.

src/routes/(console)/create-organization/+page.svelte (1)

118-128: LGTM! Argument updated to match SDK default.

The change from null to undefined for billingAddressId aligns with the SDK's updated default parameter value.

@ItzNotABug ItzNotABug merged commit 6b3409d into main Nov 19, 2025
4 checks passed
@ItzNotABug ItzNotABug deleted the fix-organization-billing branch November 19, 2025 08:23
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.

3 participants