add webhook exist validation in bulkTrigger()

[shimonewman]

No description provided.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change modifies the triggerBulk method in the Documents Action class to make webhook queue triggering conditional. The webhook queue is now only triggered when the project contains a non-empty webhooks attribute, whereas it previously triggered unconditionally. Other queues handling events, realtime, and functions remain functionally unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to evaluate whether any provided description relates to the changeset.	Add a pull request description explaining the motivation, changes made, and any testing performed for this webhook validation fix.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding webhook existence validation in the bulkTrigger() method to conditionally trigger webhooks based on project configuration.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ 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-bulk-insert-webhook-validation

---

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

[github-actions]

Security Scan Results for PR

Docker Image Scan Results

Package	Version	Vulnerability	Severity
libpng	1.6.51-r0	CVE-2025-66293	HIGH
libpng-dev	1.6.51-r0	CVE-2025-66293	HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Action.php (2)

385-389: Move the webhook check outside the loop to avoid redundant evaluations.

The condition !empty($queueForEvents->getProject()->getAttribute('webhooks', [])) is evaluated for every document in the loop. Since the project's webhook configuration doesn't change during the loop, this check can be performed once before entering the loop, improving efficiency for bulk operations with many documents.

🔎 Proposed refactor to check webhooks once

+        $hasWebhooks = !empty($queueForEvents->getProject()->getAttribute('webhooks', []));
+
         foreach ($documents as $document) {
             $queueForEvents
                 ->setParam('documentId', $document->getId())
                 ->setParam('rowId', $document->getId())
                 ->setPayload($document->getArrayCopy());
 
             $queueForRealtime
                 ->from($queueForEvents)
                 ->trigger();
 
             $queueForFunctions
                 ->from($queueForEvents)
                 ->trigger();
 
-            if (!empty($queueForEvents->getProject()->getAttribute('webhooks', []))) {
+            if ($hasWebhooks) {
                 $queueForWebhooks
                     ->from($queueForEvents)
                     ->trigger();
             }
         }

---

392-395: Consider making webhook queue reset conditional to match trigger logic.

When webhooks are not configured, $queueForWebhooks->reset() is called at line 395 even though the webhook queue was not triggered in the loop. Since the queue is only modified when webhooks exist (line 385 condition), the reset can be made conditional to match the trigger pattern:

if (!empty($queueForEvents->getProject()->getAttribute('webhooks', []))) {
    $queueForWebhooks->reset();
}

This aligns with the principle that queues only require reset when they are actually triggered within a loop.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 73d3d15 and e8b8a8f.

📒 Files selected for processing (1)

• src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Action.php

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-11-12T04:07:12.576Z

Learnt from: abnegate
Repo: appwrite/appwrite PR: 10800
File: src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Bulk/Delete.php:153-154
Timestamp: 2025-11-12T04:07:12.576Z
Learning: In Appwrite's event queue system, `queueForRealtime`, `queueForFunctions`, and `queueForWebhooks` are copied from `queueForEvents`. Therefore, when resetting event queues in transaction staging paths, only `$queueForEvents->reset()` needs to be called in most cases. The other queues only require explicit reset when they are triggered in a loop.

Applied to files:

• src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Action.php

🧬 Code graph analysis (1)

src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Action.php (1)

src/Appwrite/Event/Event.php (1)

• from (614-624)

⏰ 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). (3)

• GitHub Check: Setup & Build Appwrite Image
• GitHub Check: Setup & Build Appwrite Image
• GitHub Check: scan

[github-actions]

✨ Benchmark results

• Requests per second: 1,209
• Requests with 200 status code: 217,746
• P99 latency: 0.160190591

⚡ Benchmark Comparison

Metric	This PR	Latest version
RPS	1,209	1,284
200	217,746	231,101
P99	0.160190591	0.158915252