fix: sync registrations too with attendance#3
Conversation
|
The registrant sync call should be in the backend sync registration API itself. |
|
Done @NagariaHussain |
|
Don't need the publish progress |
WalkthroughThree files are updated: a guard condition is reformatted for readability, a registration synchronization pre-step is added to the attendance sync workflow, and import statements are reformatted within a type-checking block. No behavioral changes occur outside the new sync call. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code graph analysis (1)zoom_integration/zoom_integration/doctype/zoom_webinar_registration/zoom_webinar_registration.py (1)
⏰ 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)
🔇 Additional comments (3)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
zoom_integration/zoom_integration/doctype/zoom_webinar/zoom_webinar.py (2)
333-334: Critical: Wrong flag set in registration sync method.The
sync_registrations_from_zoom()method incorrectly setsself.attendance_synced = 1. This flag should only be set by thesync_attendance()method. Consider introducing a separate flag (e.g.,registrations_synced) or removing this line entirely if registration sync status doesn't need to be tracked.🔎 Suggested fix
- self.attendance_synced = 1 - self.save()Or introduce a new flag if tracking registration sync status is needed:
- self.attendance_synced = 1 + self.registrations_synced = 1 self.save()
266-270: Removepublish_progresscalls per maintainer feedback.The maintainer commented "Don't need the publish progress" in the PR discussion. Since
sync_registrations_from_zoom()is now called from withinsync_attendance(), these progress updates may create confusing or duplicate progress indicators.🔎 Apply this diff to remove progress publishing
- frappe.publish_progress( - percent=5, - title="Creating Registrant Records", - description=f"Creating records for {len(details)} unique registrants...", - ) - # Process in batches to avoid memory issues# Update progress after each batch - progress = 10 + ((processed_count / len(details)) * 85) - frappe.publish_progress( - percent=progress, - title="Creating Registrant Records", - description=f"Processed {processed_count} of {len(details)} registrants...", - ) - # Commit batch to avoid long transactions- frappe.publish_progress( - percent=100, - title="Creating Registrant Records", - description=f"Successfully synced {processed_count} of {len(details)} registrants!", - ) - if processed_count < len(details):Also applies to: 324-328, 336-340
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
zoom_integration/utils.py(1 hunks)zoom_integration/zoom_integration/doctype/zoom_webinar/zoom_webinar.py(1 hunks)zoom_integration/zoom_integration/doctype/zoom_webinar_registration/zoom_webinar_registration.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
zoom_integration/zoom_integration/doctype/zoom_webinar_registration/zoom_webinar_registration.py (1)
zoom_integration/zoom_integration/doctype/zoom_webinar_additional_param/zoom_webinar_additional_param.py (1)
ZoomWebinarAdditionalParam(8-24)
⏰ 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 (3)
zoom_integration/zoom_integration/doctype/zoom_webinar_registration/zoom_webinar_registration.py (1)
16-19: LGTM! Improved import readability.The multi-line import format improves readability and follows Python conventions for long import paths.
zoom_integration/zoom_integration/doctype/zoom_webinar/zoom_webinar.py (1)
173-174: LGTM! Correct prerequisite sync.Adding the registration sync before attendance sync ensures that registration records exist before creating attendance records that reference them (line 195-197).
zoom_integration/utils.py (1)
13-17: LGTM! Improved guard condition readability.The multi-line format with explicit parentheses improves readability of the validation logic without changing behavior.
Sync registration before syncing attendance
Screen.Recording.2025-12-12.at.1.22.11.AM.mov
Summary by CodeRabbit
Improvements
Style
✏️ Tip: You can customize this high-level summary in your review settings.