Conversation
📝 WalkthroughWalkthroughA new Registration Report feature is introduced for Zoom Webinar integrations. The report consists of a Python backend with data retrieval and column definitions, a JSON configuration file, and a JavaScript frontend filter definition. Additionally, a timestamp was updated in the Zoom Webinar Registration DocType. Changes
Sequence DiagramsequenceDiagram
actor Client
participant JS as registration_report.js
participant Frappe as Frappe Framework
participant Python as registration_report.py
participant DB as Database
Client->>JS: Select webinar filter
JS->>Frappe: Submit filter (webinar_id)
Frappe->>Python: Call execute(filters)
Python->>Python: Call get_columns()
Python->>Python: Call get_data(filters)
Python->>DB: Query registrations by webinar
DB-->>Python: Registration records
Python->>DB: Query UTM parameters
DB-->>Python: UTM data
Python->>Python: Merge UTM data into registrations
Python-->>Frappe: (columns, data)
Frappe-->>Client: Render report with data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
🤖 Fix all issues with AI agents
In
`@zoom_integration/zoom_integration/report/registration_report/registration_report.py`:
- Around line 16-21: The column label "User Email" doesn't match the linked user
field "user" from the Zoom Webinar Registration doctype; either rename the
column label to "User" to reflect the existing field, or change the report to
select the actual email by updating get_data() to join/select zwreg.email and
keep the label "User Email"; locate the column definition with "fieldname":
"user" and the get_data() SQL/query function and apply one of these two fixes
accordingly.
- Around line 118-127: The SQL in utm_query references a non-existent child
table 'tabUtm Parameters' so the query will fail; update the code to use the
existing child table used for key/value pairs (e.g., 'tabZoom Webinar Additional
Param') or create a new DocType named "Utm Parameters" as a child of Zoom
Webinar Registration. If refactoring, change utm_query to select parent, `key`,
value FROM the existing child table and keep the same WHERE clauses (parenttype
= 'Zoom Webinar Registration' AND parent IN %(parents)s), and confirm the field
names (`key` and value) match the actual fields on the "Zoom Webinar Additional
Param" DocType; if you create the DocType, ensure it's defined as a child table
linked to Zoom Webinar Registration with the expected fields so the existing
utm_query can run unchanged.
🧹 Nitpick comments (2)
zoom_integration/zoom_integration/report/registration_report/registration_report.py (2)
91-92: Consider validating the required filter.If
filters.get("webinar")isNone, the query will attempt to matchwebinar = NULL, which may return no results or unexpected behavior. Since the JS defines this filter as required (reqd: 1), this is unlikely in normal usage, but defensive validation would be safer.Suggested validation
def get_data(filters): + if not filters.get("webinar"): + return [] values = {"webinar": filters.get("webinar")}
95-108: Consider filtering by docstatus.The query fetches all registrations regardless of
docstatus. SinceZoom Webinar Registrationis submittable (is_submittable: 1), you may want to exclude draft or cancelled records.Add docstatus filter
FROM `tabZoom Webinar Registration` zwreg WHERE zwreg.webinar = %(webinar)s + AND zwreg.docstatus = 1 ORDER BY zwreg.user
| { | ||
| "label": _("User Email"), | ||
| "fieldname": "user", | ||
| "fieldtype": "Data", | ||
| "width": 200, | ||
| }, |
There was a problem hiding this comment.
Column label mismatch with actual data.
The label says "User Email" but the fieldname user fetches the user field from Zoom Webinar Registration, which is a Link to the User doctype (not the email address itself). The DocType has a separate email field that contains the actual email.
Consider either:
- Change the label to "User" to match the actual data, or
- Fetch
zwreg.emailin the SQL query if you want to display the email address
Option 1: Fix the label
{
- "label": _("User Email"),
+ "label": _("User"),
"fieldname": "user",
- "fieldtype": "Data",
+ "fieldtype": "Link",
+ "options": "User",
"width": 200,
},Option 2: Fetch email instead
Update the column definition:
{
"label": _("User Email"),
- "fieldname": "user",
+ "fieldname": "email",
"fieldtype": "Data",
"width": 200,
},And update the SQL query in get_data():
- zwreg.user,
+ zwreg.email,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| "label": _("User Email"), | |
| "fieldname": "user", | |
| "fieldtype": "Data", | |
| "width": 200, | |
| }, | |
| { | |
| "label": _("User"), | |
| "fieldname": "user", | |
| "fieldtype": "Link", | |
| "options": "User", | |
| "width": 200, | |
| }, |
🤖 Prompt for AI Agents
In
`@zoom_integration/zoom_integration/report/registration_report/registration_report.py`
around lines 16 - 21, The column label "User Email" doesn't match the linked
user field "user" from the Zoom Webinar Registration doctype; either rename the
column label to "User" to reflect the existing field, or change the report to
select the actual email by updating get_data() to join/select zwreg.email and
keep the label "User Email"; locate the column definition with "fieldname":
"user" and the get_data() SQL/query function and apply one of these two fixes
accordingly.
| utm_query = """ | ||
| SELECT | ||
| parent, | ||
| `key`, | ||
| value | ||
| FROM `tabUtm Parameters` | ||
| WHERE | ||
| parenttype = 'Zoom Webinar Registration' | ||
| AND parent IN %(parents)s | ||
| """ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for Utm Parameters doctype definition and references
echo "=== Searching for Utm Parameters references ==="
rg "Utm Parameters" --type=py --type=json -n
echo ""
echo "=== Searching for utm-related files ==="
fd "utm" -i
echo ""
echo "=== Looking for Zoom Webinar Registration doctype definition ==="
fd "zoom.*webinar.*registration" -iRepository: BuildWithHussain/zoom_integration
Length of output: 810
🏁 Script executed:
cat zoom_integration/zoom_integration/doctype/zoom_webinar_registration/zoom_webinar_registration.jsonRepository: BuildWithHussain/zoom_integration
Length of output: 3765
🏁 Script executed:
#!/bin/bash
# Search for any utm-related doctype or field definition
rg -i "utm" --type=json --type=py -n
echo ""
echo "=== Searching for all table field definitions that use Table fieldtype ==="
rg "\"Table\"" --type=json -B 5 | grep -E "(fieldname|options|Table)" | head -40Repository: BuildWithHussain/zoom_integration
Length of output: 3574
🏁 Script executed:
#!/bin/bash
# Find all doctype definitions
echo "=== All DocTypes in the repository ==="
find zoom_integration -name "*.json" -path "*/doctype/*" | grep -v test | xargs -I {} sh -c 'grep -l "\"doctype\": \"DocType\"" {} && echo "File: {}"' 2>/dev/null | grep -v "^File:" | sort
echo ""
echo "=== Check if Utm Parameters doctype exists ==="
find zoom_integration -name "*utm*.json" -o -name "*Utm*.json" 2>/dev/null
echo ""
echo "=== List all doctype directories ==="
ls -1 zoom_integration/zoom_integration/doctype/Repository: BuildWithHussain/zoom_integration
Length of output: 903
🏁 Script executed:
cat zoom_integration/zoom_integration/doctype/zoom_webinar_additional_param/zoom_webinar_additional_param.jsonRepository: BuildWithHussain/zoom_integration
Length of output: 1018
tabUtm Parameters table does not exist in the codebase.
The query will fail at runtime. The codebase only defines 6 DocTypes, and "Utm Parameters" is not among them. Either create this DocType as a child table for Zoom Webinar Registration, or refactor to use the existing "Zoom Webinar Additional Param" table (which already stores key-value pairs) for UTM data.
🤖 Prompt for AI Agents
In
`@zoom_integration/zoom_integration/report/registration_report/registration_report.py`
around lines 118 - 127, The SQL in utm_query references a non-existent child
table 'tabUtm Parameters' so the query will fail; update the code to use the
existing child table used for key/value pairs (e.g., 'tabZoom Webinar Additional
Param') or create a new DocType named "Utm Parameters" as a child of Zoom
Webinar Registration. If refactoring, change utm_query to select parent, `key`,
value FROM the existing child table and keep the same WHERE clauses (parenttype
= 'Zoom Webinar Registration' AND parent IN %(parents)s), and confirm the field
names (`key` and value) match the actual fields on the "Zoom Webinar Additional
Param" DocType; if you create the DocType, ensure it's defined as a child table
linked to Zoom Webinar Registration with the expected fields so the existing
utm_query can run unchanged.
1 participant
Report
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.