test: add regression coverage for account recovery without a name by abishekgiri · Pull Request #11645 · appwrite/appwrite

abishekgiri · 2026-03-26T01:43:04Z

Summary

The original issue reported that account.createRecovery could fail when the account had no name set.

The current recovery flow already handles that case, but there was no regression coverage for accounts upgraded from anonymous sessions without an explicit name.

Changes

Added an Account e2e regression test for the anonymous-to-email upgrade path with no explicit name
Verified /account/recovery returns 201 and sends the recovery email successfully for that account state

Testing

docker compose exec appwrite test /usr/src/code/tests/e2e/Services/Account --filter='/(testCreateAccountRecovery|testCreateAccountRecoveryWithoutName|testConvertAnonymousAccount)/'

All tests passed:

4 tests
93 assertions

coderabbitai · 2026-03-26T01:43:24Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1734fc9a-5a54-4357-b63a-b4a4327f2966

📥 Commits

Reviewing files that changed from the base of the PR and between 6641ee0 and 38fb89d.

📒 Files selected for processing (1)

tests/e2e/Services/Account/AccountCustomClientTest.php

✅ Files skipped from review due to trivial changes (1)

tests/e2e/Services/Account/AccountCustomClientTest.php

📝 Walkthrough

Walkthrough

Adds a new end-to-end test method that verifies account recovery for an account with an empty name field. The test creates an anonymous session, updates the account email and password, calls the /account/recovery endpoint, and checks the recovery response ($id, empty secret, expire). It then fetches the recovery email and asserts the recipient name is empty, the subject matches the project reset subject, and the email body contains the expected reset text.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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
Title check	✅ Passed	The title directly describes the main change: adding regression coverage for account recovery when accounts have no name set.
Description check	✅ Passed	The description explains the issue, why the test is needed, the specific changes made, and testing results—all directly related to the changeset.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/e2e/Services/Account/AccountCustomClientTest.php (1)
1985-1985: Increase email entropy to reduce parallel-test collision risk.

Line 1985 uses uniqid() alone; in parallel execution this can still collide and cause intermittent 409s. Consider aligning with the stronger pattern already used in this file (for example around Line 90).
♻️ Proposed refactor
-        $email = uniqid() . 'recovery-null-name@localhost.test';
+        $email = uniqid('', true) . getmypid() . bin2hex(random_bytes(4)) . 'recovery-null-name@localhost.test';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/e2e/Services/Account/AccountCustomClientTest.php` at line 1985, The
$email assignment using uniqid() (variable $email in AccountCustomClientTest) is
too low-entropy for parallel runs; replace the uniqid() call with a stronger
unique generator used elsewhere in this test (e.g., combine uniqid('', true) or,
better, bin2hex(random_bytes(...)) or random_int-based entropy) so the email
string is much less likely to collide; update the single-line assignment that
builds the recovery email to use that stronger generator.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/e2e/Services/Account/AccountCustomClientTest.php`:
- Line 1981: Add a method-level PHPDoc block above the public test method
testCreateAccountRecoveryWithoutName describing that this is a regression test
which ensures account recovery creation fails or behaves correctly when the name
field is omitted; include a short summary, `@covers` or `@group` tag if used in the
repo, and `@return` void to match other tests' docblocks so the new test follows
the project's PHPDoc conventions.

---

Nitpick comments:
In `@tests/e2e/Services/Account/AccountCustomClientTest.php`:
- Line 1985: The $email assignment using uniqid() (variable $email in
AccountCustomClientTest) is too low-entropy for parallel runs; replace the
uniqid() call with a stronger unique generator used elsewhere in this test
(e.g., combine uniqid('', true) or, better, bin2hex(random_bytes(...)) or
random_int-based entropy) so the email string is much less likely to collide;
update the single-line assignment that builds the recovery email to use that
stronger generator.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 43ba3640-5548-4831-b395-2a5403227777

📥 Commits

Reviewing files that changed from the base of the PR and between 1ab05ac and 6641ee0.

📒 Files selected for processing (1)

tests/e2e/Services/Account/AccountCustomClientTest.php

tests/e2e/Services/Account/AccountCustomClientTest.php

abishekgiri · 2026-03-26T01:46:47Z

Hi! This PR adds regression coverage for the account recovery flow when the user has no name set, including cases where the account was upgraded from an anonymous session.

I verified the behavior locally using the e2e tests listed above. Please let me know if there are any edge cases or improvements I should address. Thanks!

greptile-apps · 2026-03-26T01:46:50Z

Greptile Summary

This PR adds a focused e2e regression test (testCreateAccountRecoveryWithoutName) to AccountCustomClientTest that guards against a recurrence of issue #7597, where password recovery could fail for accounts upgraded from an anonymous session without an explicit name set.

The test correctly creates an anonymous session, upgrades it to an email account (without providing a name), asserts the name is empty string, then triggers a recovery flow — verifying both the 201 HTTP response and the delivery of the password-reset email to the nameless recipient.
The new test is well-isolated: it uses a uniqid()-prefixed email address, follows the same request-building pattern as surrounding tests, and uses the shared createAnonymousSession() helper consistently.
No changes to production code; test-only change with all 4 tests passing at 93 assertions.
One non-blocking suggestion: adding extractQueryParamsFromEmailLink token validation (as done in testCreateAccountRecovery) would make this regression test fully complete rather than just covering delivery.

Confidence Score: 5/5

Safe to merge — test-only change, all 4 tests pass, no production code modified.

The change is purely additive (one new test method), follows the established patterns of the surrounding test class, and introduces no risk to production code. The single P2 suggestion (adding token extraction) is a coverage nicety, not a blocking concern.

No files require special attention.

Important Files Changed

Filename	Overview
tests/e2e/Services/Account/AccountCustomClientTest.php	Adds testCreateAccountRecoveryWithoutName regression test that converts an anonymous session to email (no name) and verifies recovery succeeds with correct empty-name email delivery.

_{Reviews (1): Last reviewed commit: "Merge branch '1.9.x' into fix-7597-accou..." | Re-trigger Greptile}

tests/e2e/Services/Account/AccountCustomClientTest.php

test: add account recovery regression for users without a name

6641ee0

Merge branch '1.9.x' into fix-7597-account-create-recovery-null-name

38fb89d

coderabbitai bot reviewed Mar 26, 2026

View reviewed changes

tests/e2e/Services/Account/AccountCustomClientTest.php Show resolved Hide resolved

greptile-apps bot reviewed Mar 26, 2026

View reviewed changes

tests/e2e/Services/Account/AccountCustomClientTest.php Show resolved Hide resolved

Merge branch '1.9.x' into fix-7597-account-create-recovery-null-name

9947f3f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: add regression coverage for account recovery without a name#11645

test: add regression coverage for account recovery without a name#11645
abishekgiri wants to merge 3 commits intoappwrite:1.9.xfrom
abishekgiri:fix-7597-account-create-recovery-null-name

abishekgiri commented Mar 26, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Mar 26, 2026 •

edited

Loading

Walkthrough

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

abishekgiri commented Mar 26, 2026

Uh oh!

greptile-apps bot commented Mar 26, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

abishekgiri commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Testing

Uh oh!

coderabbitai bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

abishekgiri commented Mar 26, 2026

Uh oh!

greptile-apps bot commented Mar 26, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

abishekgiri commented Mar 26, 2026 •

edited

Loading

coderabbitai bot commented Mar 26, 2026 •

edited

Loading