app/controllers/api/databases.php (1)

4724-4729: Avoid undefined index notice when checking $permissions in bulk update

if ($data['$permissions']) may trigger an “undefined index” notice. Use a safe check.

-        if ($data['$permissions']) {
+        if (isset($data['$permissions']) && $data['$permissions']) {
             $validator = new Permissions();
             if (!$validator->isValid($data['$permissions'])) {
                 throw new Exception(Exception::GENERAL_BAD_REQUEST, $validator->getDescription());
             }
         }

Low impact, but keeps logs clean.

tests/e2e/Services/Databases/DatabasesBase.php (1)

1733-1765: Strengthen "no-change" upsert verification by asserting $updatedAt is unchanged.

To verify the controller fallback truly avoids a write when no attributes change, also assert the document’s $updatedAt didn’t mutate.

Apply this diff:

-        /**
+        /**
          * Resend same document for no change
          * Had to add all attributes because of partial comparison $old->getAttributes() == $document->getAttributes()
          * If nothing is upserted there will be a regular call to getDocument
          */
+        // Capture timestamp to ensure idempotent upsert doesn't mutate metadata
+        $updatedAtBefore = $document['body']['$updatedAt'];
 
         $document = $this->client->call(Client::METHOD_PUT, '/databases/' . $databaseId . '/collections/' . $data['moviesId'] . '/documents/' . $documentId, array_merge([
             'content-type' => 'application/json',
             'x-appwrite-project' => $this->getProject()['$id'],
         ], $this->getHeaders()), [
             'data' => [
                 'title' => 'Thor: Ragnarok',
                 'releaseYear' => 2000,
                 'integers' => [],
                 'birthDay' => null,
                 'duration' => null,
                 'starringActors' => [],
                 'actors' => [],
                 'tagline' => '',
                 'description' => '',
             ],
             'permissions' => [
                 Permission::read(Role::users()),
                 Permission::update(Role::users()),
                 Permission::delete(Role::users()),
             ],
         ]);
 
         $this->assertEquals(200, $document['headers']['status-code']);
         $this->assertEquals('Thor: Ragnarok', $document['body']['title']);
         $this->assertCount(3, $document['body']['$permissions']);
+        // No-op upsert should not bump system timestamp
+        $this->assertEquals($updatedAtBefore, $document['body']['$updatedAt']);

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

tests/e2e/Services/Databases/DatabasesBase.php (1)

1761-1764: Previous failing placeholder assertion is gone — resolved.

The hard-coded failing assertion reported earlier no longer exists in this block. Good cleanup.

tests/e2e/Services/Databases/DatabasesBase.php (3)

1733-1738: Reword to describe observable behavior, not internals.

The comment references internal implementation ($old->getAttributes() == $document->getAttributes()) which can drift. Prefer stating the behavioral contract we’re validating (idempotent upsert returns the current document without mutating timestamps).

Apply this comment tweak:

-        /**
-         * Resend same document for no change
-         * Had to add all attributes because of partial comparison $old->getAttributes() == $document->getAttributes()
-         * If nothing is upserted there will be a regular call to getDocument
-         */
+        /**
+         * Resend the same fields to verify a no-op upsert:
+         * server should return the existing document (no mutation) and not bump $updatedAt.
+         * When no changes are detected, API should behave like a GET of the current document.
+         */

---

1739-1759: Ensure a true “no-change” upsert: reuse fetched data to avoid null/[]/'' drift.

Hard-coding defaults (e.g., empty arrays/strings) can differ from stored values (often null), accidentally turning this into an update instead of a no-op. Build the payload from the previously fetched document body and strip system fields so the PUT is guaranteed identical.

Apply this diff to construct an idempotent payload and reuse permissions:

-        $document = $this->client->call(Client::METHOD_PUT, '/databases/' . $databaseId . '/collections/' . $data['moviesId'] . '/documents/' . $documentId, array_merge([
+        // Build idempotent payload from the current document snapshot
+        $sameData = $document['body'];
+        foreach (['$id', '$databaseId', '$collectionId', '$permissions', '$createdAt', '$updatedAt', '$sequence', '$tenant'] as $sys) {
+            unset($sameData[$sys]);
+        }
+        $samePermissions = $document['body']['$permissions'] ?? [
+            Permission::read(Role::users()),
+            Permission::update(Role::users()),
+            Permission::delete(Role::users()),
+        ];
+
+        $document = $this->client->call(Client::METHOD_PUT, '/databases/' . $databaseId . '/collections/' . $data['moviesId'] . '/documents/' . $documentId, array_merge([
             'content-type' => 'application/json',
             'x-appwrite-project' => $this->getProject()['$id'],
         ], $this->getHeaders()), [
-            'data' => [
-                'title' => 'Thor: Ragnarok',
-                'releaseYear' => 2000,
-                'integers' => [],
-                'birthDay' => null,
-                'duration' => null,
-                'starringActors' => [],
-                'actors' => [],
-                'tagline' => '',
-                'description' => '',
-            ],
-            'permissions' => [
-                Permission::read(Role::users()),
-                Permission::update(Role::users()),
-                Permission::delete(Role::users()),
-            ],
+            'data' => $sameData,
+            'permissions' => $samePermissions,
         ]);

Optionally also assert that $updatedAt didn’t change (see next comment) and that the response body still matches the snapshot for key user fields.

---

1761-1764: Make idempotency observable: assert $updatedAt is unchanged.

If the upsert is a true no-op, $updatedAt should remain the same. Capture it before the second PUT and compare after.

Patch this block to include the timestamp assertion:

         $this->assertEquals(200, $document['headers']['status-code']);
         $this->assertEquals('Thor: Ragnarok', $document['body']['title']);
         $this->assertCount(3, $document['body']['$permissions']);
+        $this->assertEquals($prevUpdatedAt, $document['body']['$updatedAt'], 'No-op upsert should not bump $updatedAt');

And record the baseline right after the first GET (shown here for clarity; add near Lines 1726–1731):

// After the initial GET of the upserted doc
$prevUpdatedAt = $document['body']['$updatedAt'] ?? null;
$this->assertNotNull($prevUpdatedAt, 'Precondition: $updatedAt should be present');

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.