Fix optional account

[narekhovhannisyan]

Motivation

The Mailtrap client was inconsistently handling the accountId parameter across different API classes. While the main client class required accountId to be mandatory, all API resource classes and base classes were treating it as optional, creating a mismatch in the API design. Additionally, users should be able to use the client for basic email sending without needing to provide an accountId, as it's only required for accessing account-specific APIs like contacts, templates, etc.

fixes #90

Changes

• Made accountId optional in MailtrapClientConfig and MailtrapClient to allow basic email sending without account ID
• Updated all API resource classes (Contacts, ContactLists, ContactFields, Templates, Suppressions, AccountAccesses, Permissions, Inboxes, Projects, Messages, Attachments, SendingDomains) to require accountId in constructors
• Updated all base API classes (ContactsBaseAPI, ContactListsBaseAPI, ContactFieldsBaseAPI, TemplatesBaseAPI, SuppressionsBaseAPI) to require accountId in constructors
• Updated GeneralAPI and TestingAPI to require accountId in constructors
• Implemented validateAccountIdPresence() method that throws MailtrapError when accountId is missing, used by all API getters
• Removed unnecessary this.client and this.accountId fields from ContactFieldsBaseAPI to eliminate redundant state storage
• Updated all client getters (testing, general, contacts, contactLists, contactFields, templates, suppressions) to validate accountId presence before instantiating APIs

How to test

• Test that MailtrapClient can be instantiated without accountId for basic email sending
• Test that accessing API getters (client.contacts, client.templates, etc.) throws MailtrapError when accountId is missing
• Test that API getters work correctly when accountId is provided
• Test that all existing email sending functionality continues to work without accountId
• Test that all API resource classes properly require accountId in their constructors
• Run existing test suite to ensure no regressions

Summary by CodeRabbit

• Documentation

  • Minor README formatting update under compatibility notes.

• Refactor

  • Many API clients now require an explicit account ID at construction and no longer store it internally; several account-scoped APIs are lazily instantiated and validate account presence.

• Tests

  • Expanded coverage for account-scoped initialization, lazy loading, edge cases (null/undefined/0), and backward compatibility.

• Impact

  • Instantiation now requires an account ID; runtime behavior and responses are preserved.

[coderabbitai]

Walkthrough

Refactors to require a concrete accountId across many resource and base API constructors, removes stored client/accountId instance fields, updates MailtrapClient.validateAccountIdPresence() to return the accountId for getters, adds lazy account-scoped getters in GeneralAPI, expands tests for General, and a minor README whitespace change.

Changes

Cohort / File(s)	Summary
Client core flow src/lib/MailtrapClient.ts	validateAccountIdPresence() now returns number; MailtrapClient getters capture and pass that accountId into API constructors instead of reading a stored field.
Base APIs (require accountId, remove stored state) src/lib/api/ContactFields.ts, src/lib/api/ContactLists.ts, src/lib/api/Contacts.ts, src/lib/api/Suppressions.ts, src/lib/api/Templates.ts	Constructors changed from accountId?: number → accountId: number; removed client/accountId instance fields; instantiate underlying *Api with (client, accountId) and bind methods directly.
Aggregating / grouped APIs src/lib/api/General.ts, src/lib/api/Testing.ts	GeneralAPI adds nullable backing accountId, lazy getters for account-scoped APIs that enforce presence and return descriptive errors; TestingAPI inlines child API construction and requires accountId.
Resource APIs (require accountId, inline URL building) src/lib/api/resources/* — AccountAccesses.ts, Attachments.ts, ContactFields.ts, ContactLists.ts, Contacts.ts, Inboxes.ts, Messages.ts, Permissions.ts, Projects.ts, Suppressions.ts, Templates.ts	All resource constructors changed from optional to required accountId; removed stored accountId fields; URL templates now built directly from the constructor parameter.
Tests src/__tests__/lib/api/General.test.ts	Expanded tests for GeneralAPI: constructor with/without accountId, lazy instantiation behavior, errors when accountId missing, account discovery, and edge cases (undefined/null/0).
Docs README.md	Minor whitespace addition under “Compatibility with previous releases”.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant MC as MailtrapClient
  participant V as validateAccountIdPresence()
  participant Base as BaseAPI (e.g., ContactsBaseAPI)
  participant Res as ResourceApi (e.g., ContactsApi)

  Dev->>MC: access client.contacts (getter)
  MC->>V: validateAccountIdPresence()
  V-->>MC: accountId (number)
  MC->>Base: new ContactsBaseAPI(client, accountId)
  activate Base
  Base->>Res: new ContactsApi(client, accountId)
  Res-->>Base: instance (methods bound)
  Base-->>Dev: exposed methods (get/create/update/delete)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Contact lists api #65 — overlapping changes to ContactLists/Contacts API construction and MailtrapClient getters.
• Contact fields #89 — direct relation to ContactFields constructor/accountId changes and MailtrapClient validator usage.
• Restructure SDK #51 — related edits around MailtrapClient lazy instantiation and account-scoped APIs.

Suggested labels

feature request

Suggested reviewers

• VladimirTaytor
• mklocek
• leonid-shevtsov

Poem

I hop and fetch the ID so neat,
No undefined where URLs meet.
Constructors snug, the bindings click,
I nibble bugs — a tiny trick. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Out of Scope Changes Check	⚠️ Warning	The pull request includes extensive modifications beyond the scope of issue #90, such as changes to base API classes, the MailtrapClient’s optional handling for basic email sending, updates to GeneralAPI and TestingAPI, and a README adjustment, which are unrelated to making resource constructors require accountId.	Consider isolating the resource constructor signature changes into a dedicated PR for issue #90 and moving the client and base class adjustments to separate reviewable units.
Title Check	❓ Inconclusive	The title “Fix optional account” is too vague to convey the primary change of making accountId optional in the client and required in all API resource constructors, so it does not clearly summarize the main adjustments introduced by this pull request.	Please update the title to clearly reflect the change, for example “Make accountId optional in MailtrapClient and required in all API resource constructors.”

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues Check	✅ Passed	All resource class constructors under src/lib/api/resources have been updated to require a non-optional accountId, directly addressing issue #90 by enforcing compile-time safety and preventing undefined URL segments.
Description Check	✅ Passed	The description follows the repository template by including Motivation, Changes, and a How to Test checklist, and clearly outlines the purpose and steps for validation; the missing Images and GIFs section is non-critical.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ 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-optional-account-id

---

📜 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 2eab139 and 063e0bb.

📒 Files selected for processing (1)

• src/__tests__/lib/api/General.test.ts (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/tests/lib/api/General.test.ts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[narekhovhannisyan]

@coderabbitai check

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get PR details including title, body, changed files, and current status
gh pr view 91 --json title,body,state,isDraft,files,labels,reviews --repo railsware/mailtrap-nodejs

Unrecoverable error: Request timed out after 300000ms (requestId=a1774396-b9c8-4024-a18c-ba87fabb99ef)

---

🏁 Script executed:

#!/bin/bash
# Try getting just the changed files list
gh pr diff 91 --repo railsware/mailtrap-nodejs --name-only

Length of output: 605

---

🏁 Script executed:

#!/bin/bash
# Get the diff for the main MailtrapClient file
gh pr diff 91 --repo railsware/mailtrap-nodejs -- src/lib/MailtrapClient.ts

echo "--- SEPARATOR ---"

# Get diff for a few API files to understand the pattern
gh pr diff 91 --repo railsware/mailtrap-nodejs -- src/lib/api/General.ts src/lib/api/Testing.ts

Length of output: 91

---

🏁 Script executed:

#!/bin/bash
# Get the full PR diff (may be large but should work)
gh pr diff 91 --repo railsware/mailtrap-nodejs | head -200

Length of output: 6505

---

🏁 Script executed:

#!/bin/bash
# Get more of the diff to see the pattern in other files
gh pr diff 91 --repo railsware/mailtrap-nodejs | head -400 | tail -200

Length of output: 7805

---

🏁 Script executed:

#!/bin/bash
# Get the rest of the diff
gh pr diff 91 --repo railsware/mailtrap-nodejs | tail -200

Length of output: 7735

---

@narekhovhannisyan I've reviewed PR #91. Here's my analysis:

Summary

This PR fixes a type safety issue by making accountId required throughout the codebase instead of optional. The changes are consistent and well-implemented across all 19 files.

Key Changes

1. Core Validation Enhancement (MailtrapClient.ts)

// Before: void
private validateAccountIdPresence(): void {
  if (!this.accountId) {
    throw new MailtrapError(ACCOUNT_ID_MISSING);
  }
}

// After: returns number
private validateAccountIdPresence(): number {
  if (!this.accountId) {
    throw new MailtrapError(ACCOUNT_ID_MISSING);
  }
  return this.accountId;
}

All API getters now capture and use the validated accountId:

get testing() {
  const accountId = this.validateAccountIdPresence();
  return new TestingAPI(this.axios, accountId);
}

2. Type System Alignment

All API classes now require accountId as a non-optional parameter:

• Base API classes: ContactFieldsBaseAPI, ContactListsBaseAPI, etc.
• Resource API classes: AccountAccessesApi, AttachmentsApi, etc.

3. Code Cleanup

Removed unnecessary private fields where accountId was only used during initialization:

// Before
constructor(client: AxiosInstance, accountId?: number) {
  this.client = client;
  this.accountId = accountId;
  this.suppressionsURL = `${GENERAL_ENDPOINT}/api/accounts/${this.accountId}/suppressions`;
}

// After
constructor(client: AxiosInstance, accountId: number) {
  this.client = client;
  this.suppressionsURL = `${GENERAL_ENDPOINT}/api/accounts/${accountId}/suppressions`;
}

4. Bonus Improvement (PermissionsApi)

Refactored to reuse permissionsURL instead of reconstructing it:

// Before
const url = `${GENERAL_ENDPOINT}/api/accounts/${this.accountId}/account_accesses/${accountAccessId}/permissions/bulk`;

// After
const url = `${this.permissionsURL}/${accountAccessId}/permissions/bulk`;

Benefits

✅ Type Safety: TypeScript now correctly reflects that accountId is always required
✅ Runtime Safety: Validation ensures the requirement is enforced
✅ Cleaner Code: Reduced duplication and unnecessary private fields
✅ Consistency: Pattern applied uniformly across all API classes

Considerations

⚠️ This is a breaking change for anyone directly instantiating the API classes (though typically they should use MailtrapClient getters)

The changes look solid! 🎯

[mklocek]

👍

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (4)

src/lib/api/resources/ContactLists.ts (1)

17-19: Consistent with the required accountId pattern.

Same pattern as other API resources—accountId is now mandatory and used directly for URL construction.

src/lib/api/resources/Attachments.ts (1)

15-17: Consistent pattern applied.

src/lib/api/resources/Contacts.ts (1)

18-20: Consistent pattern applied.

src/lib/api/resources/Messages.ts (1)

21-23: Consistent pattern applied.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b56710f and b2d073c91114ae75a390ca90561e536eafbabe4e.

📒 Files selected for processing (20)

• README.md (1 hunks)
• src/lib/MailtrapClient.ts (2 hunks)
• src/lib/api/ContactFields.ts (1 hunks)
• src/lib/api/ContactLists.ts (1 hunks)
• src/lib/api/Contacts.ts (1 hunks)
• src/lib/api/General.ts (1 hunks)
• src/lib/api/Suppressions.ts (1 hunks)
• src/lib/api/Templates.ts (1 hunks)
• src/lib/api/Testing.ts (1 hunks)
• src/lib/api/resources/AccountAccesses.ts (1 hunks)
• src/lib/api/resources/Attachments.ts (1 hunks)
• src/lib/api/resources/ContactFields.ts (1 hunks)
• src/lib/api/resources/ContactLists.ts (1 hunks)
• src/lib/api/resources/Contacts.ts (1 hunks)
• src/lib/api/resources/Inboxes.ts (1 hunks)
• src/lib/api/resources/Messages.ts (1 hunks)
• src/lib/api/resources/Permissions.ts (2 hunks)
• src/lib/api/resources/Projects.ts (1 hunks)
• src/lib/api/resources/Suppressions.ts (1 hunks)
• src/lib/api/resources/Templates.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (8)

src/lib/api/Contacts.ts (2)

src/lib/MailtrapClient.ts (1)

• contacts (127-130)

src/lib/api/resources/Contacts.ts (1)

• ContactsApi (13-59)

src/lib/api/Testing.ts (4)

src/lib/api/resources/Projects.ts (1)

• ProjectsApi (10-63)

src/lib/api/resources/Inboxes.ts (1)

• InboxesApi (10-117)

src/lib/api/resources/Messages.ts (1)

• MessagesApi (16-152)

src/lib/api/resources/Attachments.ts (1)

• AttachmentsApi (10-37)

src/lib/MailtrapClient.ts (8)

src/lib/MailtrapError.ts (1)

• MailtrapError (1-1)

src/lib/api/Testing.ts (1)

• TestingAPI (8-23)

src/lib/api/General.ts (1)

• GeneralAPI (7-19)

src/lib/api/Contacts.ts (1)

• ContactsBaseAPI (5-21)

src/lib/api/ContactLists.ts (1)

• ContactListsBaseAPI (5-24)

src/lib/api/ContactFields.ts (1)

• ContactFieldsBaseAPI (5-24)

src/lib/api/Templates.ts (1)

• TemplatesBaseAPI (5-24)

src/lib/api/Suppressions.ts (1)

• SuppressionsBaseAPI (5-15)

src/lib/api/General.ts (2)

src/lib/api/resources/Accounts.ts (1)

• AccountsApi (10-29)

src/lib/api/resources/Permissions.ts (1)

• PermissionsApi (13-60)

src/lib/api/Suppressions.ts (2)

src/lib/MailtrapClient.ts (1)

• suppressions (159-162)

src/lib/api/resources/Suppressions.ts (1)

• SuppressionsApi (9-41)

src/lib/api/Templates.ts (2)

src/lib/MailtrapClient.ts (1)

• templates (151-154)

src/lib/api/resources/Templates.ts (1)

• TemplatesApi (13-69)

src/lib/api/ContactLists.ts (2)

src/lib/MailtrapClient.ts (1)

• contactLists (135-138)

src/lib/api/resources/ContactLists.ts (1)

• ContactListsApi (12-67)

src/lib/api/ContactFields.ts (2)

src/lib/MailtrapClient.ts (1)

• contactFields (143-146)

src/lib/api/resources/ContactFields.ts (1)

• ContactFieldsApi (13-71)

🔇 Additional comments (11)

src/lib/api/resources/AccountAccesses.ts (1)

19-21: LGTM! Type-safe URL construction.

The constructor now requires accountId, preventing runtime URL construction errors (e.g., /api/accounts/undefined/account_accesses). The URL is built directly from the constructor parameter, which is cleaner and enforces compile-time safety.

src/lib/api/resources/Permissions.ts (2)

18-20: LGTM! Required accountId prevents malformed URLs.

Constructor now requires accountId, ensuring type-safe URL construction.

---

36-36: Good refactor: reuses base URL.

The method now reuses this.permissionsURL instead of reconstructing the base path inline, which reduces duplication and improves maintainability.

src/lib/api/resources/ContactFields.ts (1)

18-20: LGTM! Consistent with the required accountId pattern.

The constructor changes across all API resource files are uniform and correct. Making accountId required prevents runtime URL construction errors and provides compile-time type safety.

src/lib/api/Contacts.ts (1)

14-20: LGTM! Clean refactor to require accountId.

The constructor now requires a concrete accountId and passes it directly to ContactsApi instead of storing it internally. The method binding pattern is correct and maintains proper context.

src/lib/MailtrapClient.ts (2)

92-96: Excellent improvement to return the validated accountId.

Changing the return type from void to number makes the validation more functional and explicit. Callers can immediately use the validated value without re-reading this.accountId.

---

112-161: LGTM! Consistent pattern across all API getters.

All seven API getters (testing, general, contacts, contactLists, contactFields, templates, suppressions) follow the same pattern:

1. Validate and capture the returned accountId
2. Pass it to the API constructor

This ensures type safety and runtime validation before API instantiation.

src/lib/api/resources/Inboxes.ts (1)

13-21: LGTM! Correctly stores accountId for runtime usage.

Unlike some other resources, InboxesApi stores accountId because the create method (line 27) needs it for URL construction. Making it required (non-optional) prevents runtime errors from undefined values.

src/lib/api/resources/Templates.ts (1)

18-21: LGTM! Efficient approach for construction-only usage.

The constructor correctly uses accountId directly for URL construction without storing it on the instance. This is appropriate since accountId is only needed once at initialization.

src/lib/api/Suppressions.ts (1)

10-14: LGTM! Consistent with the base API pattern.

The constructor correctly requires accountId, instantiates SuppressionsApi directly, and binds its methods without storing internal state. This matches the pattern established in other base API classes.

src/lib/api/Testing.ts (1)

17-22: LGTM! Consistent initialization pattern.

The constructor correctly requires accountId and passes it to all four resource API constructors (ProjectsApi, InboxesApi, MessagesApi, AttachmentsApi). This ensures all testing-related APIs have the required accountId at construction time.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (9)

src/__tests__/lib/api/General.test.ts (6)

42-49: Simplify toThrow usage; drop ESLint disable

You can assert getter throws without wrapping a block and disabling ESLint.

Apply:

-      expect(() => {
-        // eslint-disable-next-line @typescript-eslint/no-unused-expressions
-        generalAPI.accountAccesses;
-      }).toThrow(
+      expect(() => generalAPI.accountAccesses).toThrow(
         "Account ID is required for this operation. Please provide accountId when creating GeneralAPI instance."
       );

---

51-58: Same simplification for permissions getter

-      expect(() => {
-        // eslint-disable-next-line @typescript-eslint/no-unused-expressions
-        generalAPI.permissions;
-      }).toThrow(
+      expect(() => generalAPI.permissions).toThrow(
         "Account ID is required for this operation. Please provide accountId when creating GeneralAPI instance."
       );

---

89-96: Test title is misleading (not actually lazy with accountId)

Constructor eagerly instantiates accountAccesses when accountId is present. Reword title.

-      it("instantiates accountAccesses lazily when accountId is provided but not accessed during construction.", () => {
+      it("provides accountAccesses when accountId is provided.", () => {

---

97-104: Same: clarify permissions test title

-      it("instantiates permissions lazily when accountId is provided but not accessed during construction.", () => {
+      it("provides permissions when accountId is provided.", () => {

---

135-142: Simplify getter throw assertion (undefined accountId case)

-      expect(() => {
-        // eslint-disable-next-line @typescript-eslint/no-unused-expressions
-        generalAPI.accountAccesses;
-      }).toThrow();
+      expect(() => generalAPI.accountAccesses).toThrow();

---

144-151: Simplify getter throw assertion (null accountId case)

-      expect(() => {
-        // eslint-disable-next-line @typescript-eslint/no-unused-expressions
-        generalAPI.accountAccesses;
-      }).toThrow();
+      expect(() => generalAPI.accountAccesses).toThrow();

src/lib/api/General.ts (3)

23-31: Consider fully lazy instantiation for account-scoped APIs

Eager creation increases work and contradicts the lazy accessors. Let getters instantiate on first access.

-    // Only instantiate account-specific APIs if accountId is provided
-    if (this.accountId !== null) {
-      this.accountAccessesInstance = new AccountAccessesApi(
-        client,
-        this.accountId
-      );
-      this.permissionsInstance = new PermissionsApi(client, this.accountId);
-    }

Note: Adjust tests that assert “instantiates immediately” accordingly.

---

33-40: Prefer a project-specific error type for consistency

Throw the same error class used elsewhere (e.g., MailtrapError) to keep error handling uniform across SDK.

Replace new Error(...) with the shared SDK error (same message). Based on PR objectives.

---

8-16: Minor: mark immutable fields as readonly

Client and accounts aren’t reassigned; marking readonly helps prevent accidental mutation.

-  public accounts: AccountsApi;
+  public readonly accounts: AccountsApi;
-  private client: AxiosInstance;
+  private readonly client: AxiosInstance;

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2d073c91114ae75a390ca90561e536eafbabe4e and 15413d6.

📒 Files selected for processing (2)

• src/__tests__/lib/api/General.test.ts (1 hunks)
• src/lib/api/General.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

src/__tests__/lib/api/General.test.ts (1)

src/lib/api/General.ts (2)

• accountAccesses (42-52)
• permissions (54-61)

src/lib/api/General.ts (2)

src/lib/api/resources/Accounts.ts (1)

• AccountsApi (10-29)

src/lib/api/resources/Permissions.ts (1)

• PermissionsApi (13-60)

🔇 Additional comments (1)

src/lib/api/General.ts (1)

42-61: LGTM: Accounts API remains accessible without accountId

This addresses the earlier gating concern; per‑account APIs are guarded via presence check.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/__tests__/lib/api/General.test.ts (1)

75-111: Consider simplifying implementation-focused test descriptions.

While these tests provide good coverage, several descriptions are overly specific about implementation details:

• Line 89: "instantiates accountAccesses lazily when accountId is provided but not accessed during construction"
• Line 97: "instantiates permissions lazily when accountId is provided but not accessed during construction"
• Line 105: "covers checkAccountIdPresence return path when accountId is valid"

These tests verify behavior (getters return working API instances) rather than implementation (lazy vs. eager instantiation). Consider focusing descriptions on observable behavior.

Example revision:

-      it("instantiates accountAccesses lazily when accountId is provided but not accessed during construction.", () => {
+      it("provides working accountAccesses API when accountId is available.", () => {

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 15413d6 and d98c06a.

📒 Files selected for processing (2)

• src/__tests__/lib/api/General.test.ts (1 hunks)
• src/lib/api/General.ts (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/lib/api/General.ts

🧰 Additional context used

🧬 Code graph analysis (1)

src/__tests__/lib/api/General.test.ts (1)

src/lib/api/General.ts (2)

• accountAccesses (42-52)
• permissions (54-61)

🔇 Additional comments (3)

src/__tests__/lib/api/General.test.ts (3)

6-16: LGTM!

The test setup and initial property checks are well-structured and correctly verify that all necessary properties are present when accountId is provided.

---

25-73: Excellent coverage of accountId scenarios.

These test suites comprehensively verify:

• Constructor behavior without accountId
• Error handling for account-specific APIs without accountId
• Account discovery functionality that doesn't require accountId

The coverage aligns well with the PR objectives to make accountId optional in the client while requiring it for account-specific operations.

---

134-162: Excellent edge case coverage.

These tests properly verify boundary conditions:

• undefined accountId → should throw when accessing account-specific APIs
• null accountId → should throw when accessing account-specific APIs
• 0 accountId → should work (0 is a valid account ID despite being falsy)

The test at line 153 is particularly valuable as it confirms the implementation doesn't conflate 0 with missing accountId.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d98c06a and 2eab139.

📒 Files selected for processing (1)

• src/__tests__/lib/api/General.test.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/__tests__/lib/api/General.test.ts (1)

src/lib/api/General.ts (2)

• accountAccesses (42-52)
• permissions (54-61)

🔇 Additional comments (4)

src/__tests__/lib/api/General.test.ts (4)

6-37: LGTM! Clear separation of constructor scenarios.

The tests properly validate both constructor paths—with and without accountId—and confirm that account discovery works independently of account-specific APIs.

---

39-73: LGTM! Comprehensive error handling and account discovery coverage.

The tests correctly verify that accessing account-specific APIs without accountId throws descriptive errors, while the accounts API remains accessible for account discovery.

---

75-86: LGTM! Backward compatibility validated.

This test confirms that the refactored lazy instantiation preserves the existing API surface for account-specific operations.

---

132-141: LGTM! Critical edge case for accountId = 0.

This test ensures that 0 is treated as a valid accountId rather than a falsy value, which is important for correctness.