fix: check registration exist or not#4
Conversation
📝 WalkthroughWalkthroughAdds a deduplication check in Changes
Sequence Diagram(s)(omitted — change is a small control-flow dedupe and CI runtime update, not warranting a multi-component sequence diagram) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @zoom_integration/zoom_integration/doctype/zoom_webinar/zoom_webinar.py:
- Around line 281-289: The if-block checking for duplicates has misaligned
indentation causing ruff-format failure; locate the block that calls
frappe.db.exists with keys ("Zoom Webinar Registration", {"registrant_id":
registrant.get("id"), "webinar": self.name}) and re-indent so the if line, its
condition block, and the subsequent continue statement are at the same
indentation level (i.e., the continue is inside the if body). Ensure the
conditional and continue are consistently indented with the surrounding loop
(where registrant is iterated) so formatting and linting pass.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
zoom_integration/zoom_integration/doctype/zoom_webinar/zoom_webinar.py
🧰 Additional context used
🪛 GitHub Actions: Linters
zoom_integration/zoom_integration/doctype/zoom_webinar/zoom_webinar.py
[error] 278-278: pre-commit hook ruff-format reformatted 1 file and failed. Hook 'ruff-format' changed files; run 'pre-commit run --all-files' to re-verify. Process completed with exit code 1.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
zoom_integration/zoom_integration/doctype/zoom_webinar/zoom_webinar.py (1)
281-288: Good deduplication check, but progress reporting may be misleading.The existence check is an efficient way to avoid unnecessary document creation overhead. However, the current implementation silently skips duplicates without tracking them, which causes the final message (lines 350-355) to incorrectly attribute skipped duplicates to "errors."
Consider tracking duplicates separately to provide accurate feedback:
♻️ Suggested improvement for accurate reporting
batch_size = ATTENDANCE_SYNC_BATCH_SIZE processed_count = 0 + skipped_duplicates = 0 for i in range(0, len(details), batch_size): batch = details[i : i + batch_size] for registrant in batch: try: if frappe.db.exists( "Zoom Webinar Registration", { "registrant_id": registrant.get("id"), "webinar": self.name, }, ): + skipped_duplicates += 1 continueThen update the final message to distinguish duplicates from errors:
if processed_count < len(details): + error_count = len(details) - processed_count - skipped_duplicates frappe.msgprint( - f"Synced {processed_count} out of {len(details)} registrants. " - f"{len(details) - processed_count} records had errors and were skipped. " - "Check the Error Log for details." + f"Synced {processed_count} out of {len(details)} registrants. " + f"{skipped_duplicates} duplicates skipped. " + + (f"{error_count} records had errors (see Error Log)." if error_count > 0 else "") )
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Around line 51-55: The workflow uses actions/setup-node@v3 which is outdated
for Node.js 24; update the action reference from actions/setup-node@v3 to
actions/setup-node@v4 to ensure compatibility with current runners, and while
editing also consider updating related action versions for consistency (e.g.,
actions/checkout@v3 → actions/checkout@v4 and actions/setup-python@v4 →
actions/setup-python@v5) so all setup actions target their current major
releases.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml
🧰 Additional context used
🪛 actionlint (1.7.10)
.github/workflows/ci.yml
52-52: the runner of "actions/setup-node@v3" 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). (1)
- GitHub Check: Server
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
46-49: Python 3.14 is the latest stable release — looks good.Python 3.14 was released on 7 October 2025 and is the current stable version with significant improvements including template string literals, deferred evaluation of annotations, and support for subinterpreters in the standard library.
Ensure that
frappe-benchand other dependencies are compatible with Python 3.14, as some libraries may lag behind on compatibility with the latest Python release.
| - name: Setup Node | ||
| uses: actions/setup-node@v3 | ||
| with: | ||
| node-version: 18 | ||
| node-version: 24 | ||
| check-latest: true |
There was a problem hiding this comment.
Node.js 24 is LTS — update actions/setup-node to v4.
Node.js 24.x has entered Long Term Support (LTS) with the codename 'Krypton' and will continue to receive updates through to the end of April 2028. The version choice is appropriate.
However, static analysis flagged that actions/setup-node@v3 is too old to run on current GitHub Actions runners. Update to v4 for compatibility.
🔧 Suggested fix
- name: Setup Node
- uses: actions/setup-node@v3
+ uses: actions/setup-node@v4
with:
node-version: 24
check-latest: trueFor consistency, also consider updating the other actions:
actions/checkout@v3→actions/checkout@v4(line 39)actions/setup-python@v4→actions/setup-python@v5(line 47)
🧰 Tools
🪛 actionlint (1.7.10)
52-52: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In @.github/workflows/ci.yml around lines 51 - 55, The workflow uses
actions/setup-node@v3 which is outdated for Node.js 24; update the action
reference from actions/setup-node@v3 to actions/setup-node@v4 to ensure
compatibility with current runners, and while editing also consider updating
related action versions for consistency (e.g., actions/checkout@v3 →
actions/checkout@v4 and actions/setup-python@v4 → actions/setup-python@v5) so
all setup actions target their current major releases.
2 participants
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.