feat: event template save and use #106
Conversation
📝 WalkthroughWalkthroughAdds an Event Template system: new Event Template doctype with three child doctypes, backend APIs to create events from templates and templates from events, frontend dialogs for create/save flows, list/form UI buttons, tests, and ESLint/global and app JS include updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Client)
participant ListUI as Buzz Event List UI
participant Dialog as Template Dialog
participant Server as Backend API
participant DB as Database
User->>ListUI: Click "Create from Template"
ListUI->>Dialog: Open template selection dialog
Dialog->>Server: request templates
Server->>DB: query Event Template + children
DB-->>Server: return templates
Server-->>Dialog: templates & counts
User->>Dialog: Select template + options
Dialog->>Server: call create_from_template(template_name, options, additional_fields)
rect rgba(200,220,255,0.5)
note over Server: permission checks, field mapping
end
Server->>DB: insert Buzz Event, copy child records, create linked docs
DB-->>Server: created event id
Server-->>Dialog: return new event id
Dialog->>User: redirect to new Buzz Event
sequenceDiagram
participant User as User (Client)
participant Form as Buzz Event Form
participant Dialog as Save Template Dialog
participant Server as Backend API
participant DB as Database
User->>Form: Open existing Buzz Event
Form->>User: show "Save as Template" button
User->>Form: Click "Save as Template"
Form->>Dialog: open save-dialog
Dialog->>Server: fetch event + related counts
Server->>DB: load Buzz Event + children
DB-->>Server: event data + counts
Server-->>Dialog: event data
User->>Dialog: choose fields, name template
Dialog->>Server: call create_template_from_event(event_name, template_name, options)
rect rgba(200,220,255,0.5)
note over Server: permission checks, copy scalars & child tables
end
Server->>DB: insert Event Template + child rows
DB-->>Server: template created
Server-->>Dialog: return template id
Dialog->>User: redirect to Event Template
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 3
🧹 Nitpick comments (8)
buzz/events/doctype/event_template_add_on/event_template_add_on.json (2)
50-57: Consider making the default currency configurable.The currency field defaults to "INR" which may not be suitable for all deployments. Consider either removing the hardcoded default or deriving it from system settings (e.g.,
frappe.defaults.get_global_default("currency")), similar to how Event or Company currency defaults are typically handled.
71-72:index_web_pages_for_searchis unnecessary for child table DocTypes.Since this is a child table (
istable: 1), it won't have its own web pages to index. This flag can be safely set to0or removed.buzz/events/doctype/buzz_event/buzz_event.py (1)
133-244: Consider extracting shared field mapping to reduce duplication.The
field_mapdictionary and the field-copying logic here are duplicated increate_template_from_eventinevent_template.py. If the list of copyable fields changes, both places need updating.Consider extracting the shared mapping to a constant or utility function to maintain consistency:
🔎 Suggested approach
# At module level or in a shared utils file TEMPLATE_FIELD_MAP = { "category": "category", "host": "host", "banner_image": "banner_image", # ... rest of fields }Then both functions can import and use this shared mapping.
buzz/public/js/events/create_from_template.js (5)
112-132: Consider adding error handling for the template fetch.If the API call fails (e.g., network error, permission denied), the user receives no feedback.
🔎 Suggested improvement
frappe.call({ method: "frappe.client.get", args: { doctype: "Event Template", name: template_name, }, callback: function (r) { if (r.message) { buzz.events.render_template_options(dialog, r.message); } }, + error: function () { + frappe.msgprint(__("Failed to load template. Please try again.")); + }, });
370-411: Add error handling for event creation API call.Similar to the template fetch, the
create_from_templatecall should handle errors to provide user feedback on failure.🔎 Suggested improvement
frappe.call({ method: "buzz.events.doctype.buzz_event.buzz_event.create_from_template", args: { template_name: template_name, options: JSON.stringify(options), additional_fields: JSON.stringify(additional_fields), }, freeze: true, freeze_message: __("Creating Event..."), callback: function (r) { if (r.message) { dialog.hide(); frappe.show_alert({ message: __("Event created successfully"), indicator: "green", }); frappe.set_route("Form", "Buzz Event", r.message); } }, + error: function () { + frappe.msgprint(__("Failed to create event. Please try again.")); + }, });
604-662: Consider batching the three count API calls into one.Making three separate API calls to fetch counts for related documents adds latency and overhead. A single server method could return all counts at once.
🔎 Example approach
Create a server-side method that returns all counts:
# In event_template.py or buzz_event.py @frappe.whitelist() def get_event_related_counts(event_name): return { "ticket_types": frappe.db.count("Event Ticket Type", {"event": event_name}), "add_ons": frappe.db.count("Ticket Add-on", {"event": event_name}), "custom_fields": frappe.db.count("Buzz Custom Field", {"event": event_name}), }Then use a single call in JavaScript:
frappe.call({ method: "buzz.events.doctype.buzz_event.buzz_event.get_event_related_counts", args: { event_name: doc.name }, callback: function (r) { if (r.message) { // Update all three placeholders with the returned counts } }, });
477-493: Code duplication in field rendering loops.The same HTML generation pattern is repeated three times for event_fields, ticketing_fields, and sponsor_fields. Since
render_field_groupalready exists for the create-from-template dialog, consider reusing it here or extracting a shared helper.Also applies to: 508-524, 539-558
686-716: Add error handling for save-as-template API call.Consistent with other API calls, this should handle errors gracefully.
🔎 Suggested improvement
frappe.call({ method: "buzz.events.doctype.event_template.event_template.create_template_from_event", args: { event_name: frm.doc.name, template_name: values.template_name, options: JSON.stringify(options), }, freeze: true, freeze_message: __("Creating Template..."), callback: function (r) { if (r.message) { dialog.hide(); frappe.show_alert({ message: __("Template {0} created successfully", [r.message]), indicator: "green", }); frappe.set_route("Form", "Event Template", r.message); } }, + error: function () { + frappe.msgprint(__("Failed to create template. Please try again.")); + }, });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.eslintrcPLAN_EVENT_TEMPLATE.mdbuzz/events/doctype/buzz_event/buzz_event.jsbuzz/events/doctype/buzz_event/buzz_event.pybuzz/events/doctype/buzz_event/buzz_event_list.jsbuzz/events/doctype/event_template/__init__.pybuzz/events/doctype/event_template/event_template.jsonbuzz/events/doctype/event_template/event_template.pybuzz/events/doctype/event_template/test_event_template.pybuzz/events/doctype/event_template_add_on/__init__.pybuzz/events/doctype/event_template_add_on/event_template_add_on.jsonbuzz/events/doctype/event_template_add_on/event_template_add_on.pybuzz/events/doctype/event_template_custom_field/__init__.pybuzz/events/doctype/event_template_custom_field/event_template_custom_field.jsonbuzz/events/doctype/event_template_custom_field/event_template_custom_field.pybuzz/events/doctype/event_template_ticket_type/__init__.pybuzz/events/doctype/event_template_ticket_type/event_template_ticket_type.jsonbuzz/events/doctype/event_template_ticket_type/event_template_ticket_type.pybuzz/hooks.pybuzz/public/js/events/create_from_template.js
🧰 Additional context used
🧬 Code graph analysis (5)
buzz/events/doctype/buzz_event/buzz_event.py (2)
buzz/public/js/events/create_from_template.js (13)
template_name(113-113)template_name(371-371)options(372-372)options(687-687)additional_fields(383-383)value(315-315)value(478-478)value(509-509)value(540-540)label(323-323)label(480-480)label(511-511)label(545-545)dashboard/src/composables/useCustomFields.js (1)
placeholder(108-108)
buzz/events/doctype/event_template_add_on/event_template_add_on.py (1)
buzz/public/js/events/create_from_template.js (2)
options(372-372)options(687-687)
buzz/events/doctype/event_template/test_event_template.py (2)
buzz/events/doctype/buzz_event/buzz_event.py (1)
create_from_template(134-244)buzz/events/doctype/event_template/event_template.py (1)
create_template_from_event(57-194)
buzz/events/doctype/event_template_custom_field/event_template_custom_field.py (2)
buzz/public/js/events/create_from_template.js (6)
label(323-323)label(480-480)label(511-511)label(545-545)options(372-372)options(687-687)dashboard/src/composables/useCustomFields.js (1)
placeholder(108-108)
buzz/events/doctype/event_template/event_template.py (4)
buzz/events/doctype/event_payment_gateway/event_payment_gateway.py (1)
EventPaymentGateway(8-23)buzz/events/doctype/event_template_custom_field/event_template_custom_field.py (1)
EventTemplateCustomField(8-32)buzz/events/doctype/event_template_ticket_type/event_template_ticket_type.py (1)
EventTemplateTicketType(8-28)buzz/proposals/doctype/sponsorship_deck_item/sponsorship_deck_item.py (1)
SponsorshipDeckItem(8-23)
🪛 markdownlint-cli2 (0.18.1)
PLAN_EVENT_TEMPLATE.md
39-39: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
47-47: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
58-58: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
68-68: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
79-79: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
117-117: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
121-121: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
484-484: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
556-556: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.10)
buzz/events/doctype/event_template_custom_field/event_template_custom_field.py
17-17: Undefined name Booking
(F821)
17-17: Undefined name Ticket
(F821)
21-21: Undefined name Data
(F821)
21-21: Undefined name Phone
(F821)
21-21: Undefined name Email
(F821)
21-21: Undefined name Select
(F821)
21-21: Undefined name Date
(F821)
21-21: Undefined name Number
(F821)
buzz/events/doctype/event_template/event_template.py
34-34: Syntax error in forward annotation: Unexpected token at the end of an expression
(F722)
34-34: Undefined name Online
(F821)
⏰ 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 (24)
buzz/events/doctype/event_template_add_on/__init__.py (1)
1-2: Standard doctype package structure - LGTM!The
__init__.pyfile follows Frappe conventions with an appropriate copyright header. This minimal structure is standard and correct for a doctype package.buzz/events/doctype/event_template_custom_field/__init__.py (1)
1-2: Standard copyright header looks good.The header follows Frappe conventions for doctype modules.
buzz/events/doctype/event_template/__init__.py (1)
1-2: Standard copyright header looks good.The header follows Frappe conventions for doctype modules.
.eslintrc (1)
27-29: ESLint global addition forbuzzis correct.Adding
buzzto the globals list prevents false ESLint undefined-variable warnings. Thebuzzglobal is properly initialized by the Frappe framework when the app is loaded (defined inhooks.pyasapp_name = "buzz"), and the codebase actively uses it throughout (e.g.,frappe.provide("buzz.events")andbuzz.api.*method calls in the dashboard).buzz/events/doctype/event_template_ticket_type/__init__.py (1)
1-2: LGTM!Standard package initializer with license header.
buzz/events/doctype/buzz_event/buzz_event.js (1)
41-50: LGTM!The "Save as Template" button is correctly guarded to only appear for existing (saved) documents, and properly grouped under "Actions". The translation wrapper
__()is correctly used.buzz/events/doctype/buzz_event/buzz_event_list.js (1)
1-9: LGTM!The list view customization correctly checks for "Event Template" read permission before showing the "Create from Template" button, ensuring proper access control.
buzz/hooks.py (1)
67-67: LGTM!The
app_include_jscorrectly references the new template dialog script, which provides thebuzz.eventsnamespace used by the form and list view customizations.buzz/events/doctype/event_template_ticket_type/event_template_ticket_type.py (1)
1-28: LGTM!Standard Frappe DocType class with auto-generated type hints for static type checking. The structure follows the established pattern for child table DocTypes.
buzz/events/doctype/event_template_custom_field/event_template_custom_field.py (1)
1-32: LGTM!Standard Frappe DocType class with auto-generated type hints. The static analysis warnings about "undefined names" (e.g.,
Booking,Ticket,Data) are false positives—these are valid string literals withinDF.Literal[]type annotations, not variable references.buzz/events/doctype/event_template_add_on/event_template_add_on.py (1)
1-29: LGTM!Standard Frappe child DocType model with auto-generated type hints. The structure correctly follows Frappe conventions for a data container without custom logic.
buzz/events/doctype/event_template/event_template.json (1)
1-303: LGTM - Well-structured DocType definition.The Event Template DocType is well-organized with appropriate:
- Field groupings via tabs and sections
- Conditional visibility (
depends_on) for related fields- Role-based permissions for Event Manager and System Manager
- Unique naming via
template_namefieldOne minor observation:
index_web_pages_for_search: 1at line 262 is typically used for public-facing content. Since templates are internal configuration, this indexing may be unnecessary.buzz/events/doctype/event_template/event_template.py (1)
9-53: LGTM!Standard Frappe Document class with auto-generated type hints. The static analysis warning about line 34 (
medium: DF.Literal["In Person", "Online"]) is a false positive - this is valid Frappe type annotation syntax.buzz/events/doctype/event_template_ticket_type/event_template_ticket_type.json (1)
1-79: LGTM!Well-defined child DocType for template ticket types with:
- Appropriate required fields (
title,currency)- Non-negative constraints on
priceandmax_tickets_available- Conditional visibility for
auto_unpublish_after- Editable grid enabled for inline editing
buzz/events/doctype/event_template_custom_field/event_template_custom_field.json (1)
1-103: LGTM!Well-structured child DocType for template custom fields with:
- Comprehensive field set matching
Buzz Custom Field- Sensible defaults (
enabled=1,fieldtype=Data,applied_to=Booking)- Required
labelfield and non-negativeorderconstraintPLAN_EVENT_TEMPLATE.md (1)
1-1260: Comprehensive implementation plan.The document provides thorough coverage of the feature including:
- DocType structure and field definitions
- UI/UX flow with dialog layouts
- Backend API specifications
- Permission model
- Extensive test cases
Minor markdown formatting issues flagged by linting (blank lines around tables, code block languages) are cosmetic and don't affect the document's utility as a planning reference.
buzz/events/doctype/event_template/test_event_template.py (6)
11-34: LGTM! Well-structured test setup and teardown.The class setup creates necessary fixtures with
ignore_permissions=True, andtearDowncorrectly usesfrappe.db.rollback()to ensure test isolation. This follows standard Frappe test conventions.
38-139: Template creation tests look comprehensive.The tests cover basic template creation and all three child table types (ticket types, add-ons, custom fields). Assertions verify both count and field values, which is good practice.
143-280: Good coverage ofcreate_from_templatescenarios.Tests cover all options, partial options, and no linked documents scenarios. The assertions correctly verify that:
- Selected fields are copied
- Unselected fields are not copied
- Linked documents are created or not based on options
284-376: Save-as-template tests verify both directions of the flow.Tests correctly verify that event fields and linked documents are copied to template child tables. The filtering approach on line 340 (
[t for t in template.template_ticket_types if t.title == "Premium"]) handles the presence of default ticket types.
380-461: Round-trip test is valuable for integration validation.This test validates the full Event → Template → Event flow, ensuring data fidelity through both transformations. The ordered assertions on ticket types verify the complete data propagation.
465-498: Edge case tests provide good boundary coverage.Tests verify:
- Minimal template creation works
template_namevalidation (raisesValidationErrordue to autoname)- Duplicate name handling (raises
DuplicateEntryError)buzz/public/js/events/create_from_template.js (2)
1-41: LGTM! Clean constant definitions for field groups and mandatory fields.The field groups align with the backend
field_mapin the relevant code snippets, and the centralized definition makes maintenance easier.
43-110: Dialog structure looks good with proper field dependencies.The use of
depends_on: "eval:doc.template"for conditional visibility and the dynamic field rendering approach is appropriate for this UI.
| event.insert() | ||
|
|
||
| # Create linked documents (Ticket Types, Add-ons, Custom Fields) | ||
| if options.get("ticket_types"): | ||
| for tt in template.template_ticket_types: | ||
| ticket_type = frappe.new_doc("Event Ticket Type") | ||
| ticket_type.event = event.name | ||
| ticket_type.title = tt.title | ||
| ticket_type.price = tt.price | ||
| ticket_type.currency = tt.currency | ||
| ticket_type.is_published = tt.is_published | ||
| ticket_type.max_tickets_available = tt.max_tickets_available | ||
| ticket_type.auto_unpublish_after = tt.auto_unpublish_after | ||
| ticket_type.insert() |
There was a problem hiding this comment.
Default records created alongside template data may cause duplicates.
When event.insert() is called (line 200), the after_insert hook in BuzzEvent automatically creates a default "Normal" ticket type (see create_default_records at lines 79-85). If the user also selects ticket_types to copy from the template, they'll end up with both the default ticket type AND the template's ticket types.
Consider either:
- Skipping default record creation when creating from a template
- Documenting this behavior for users
- Passing a flag to suppress defaults
🔎 Option 1: Add flag to skip defaults
event.insert()
+# Note: after_insert creates default ticket type. When copying from template,
+# consider whether this is desired behavior.
# Create linked documents (Ticket Types, Add-ons, Custom Fields)
if options.get("ticket_types"):Or modify create_default_records to check a flag:
def create_default_records(self):
if getattr(self, '_skip_default_records', False):
return
# ... existing logic| if not frappe.has_permission("Event Template", "create"): | ||
| frappe.throw(_("You don't have permission to create templates")) | ||
|
|
||
| event = frappe.get_doc("Buzz Event", event_name) |
There was a problem hiding this comment.
Missing permission check for source event read access.
The function checks Event Template create permission but doesn't verify the user has read access to the source Buzz Event. This could allow users to extract event configuration data they shouldn't have access to.
For consistency with create_from_template (which checks both template read and event create permissions), add a read permission check:
🔎 Proposed fix
if not frappe.has_permission("Event Template", "create"):
frappe.throw(_("You don't have permission to create templates"))
+if not frappe.has_permission("Buzz Event", "read", doc=event_name):
+ frappe.throw(_("You don't have permission to read this event"))
+
event = frappe.get_doc("Buzz Event", event_name)📝 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.
| if not frappe.has_permission("Event Template", "create"): | |
| frappe.throw(_("You don't have permission to create templates")) | |
| event = frappe.get_doc("Buzz Event", event_name) | |
| if not frappe.has_permission("Event Template", "create"): | |
| frappe.throw(_("You don't have permission to create templates")) | |
| if not frappe.has_permission("Buzz Event", "read", doc=event_name): | |
| frappe.throw(_("You don't have permission to read this event")) | |
| event = frappe.get_doc("Buzz Event", event_name) |
🤖 Prompt for AI Agents
In buzz/events/doctype/event_template/event_template.py around lines 69 to 72,
the code checks create permission for "Event Template" but does not verify read
access to the source "Buzz Event" before loading it; add a read-permission check
for the source event prior to calling frappe.get_doc. Specifically, before
fetching the event, call frappe.has_permission("Buzz Event", "read", event_name)
(or equivalent) and if it returns False raise a permission exception (e.g.,
frappe.throw(_("You don't have permission to read the source event"))), so the
source event contents are not exposed to unauthorized users.
| for (let field of group.fields) { | ||
| let value = template[field]; | ||
| let has_value = value !== null && value !== undefined && value !== "" && value !== 0; | ||
|
|
||
| // For child tables, check length | ||
| if (Array.isArray(value)) { | ||
| has_value = value.length > 0; | ||
| } |
There was a problem hiding this comment.
Zero value treated as "not set" may cause unexpected behavior.
The check value !== 0 means fields with a legitimate value of 0 (e.g., tax_percentage: 0 meaning no tax) will show as "Not set" and remain unchecked. If 0 is a valid value for any of these fields, consider checking only for null/undefined/empty string.
🔎 Suggested fix
- let has_value = value !== null && value !== undefined && value !== "" && value !== 0;
+ let has_value = value !== null && value !== undefined && value !== "";
+ // For boolean fields, 0 should be treated as a set value (false)
+ // Only treat as unset if truly empty📝 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.
| for (let field of group.fields) { | |
| let value = template[field]; | |
| let has_value = value !== null && value !== undefined && value !== "" && value !== 0; | |
| // For child tables, check length | |
| if (Array.isArray(value)) { | |
| has_value = value.length > 0; | |
| } | |
| for (let field of group.fields) { | |
| let value = template[field]; | |
| let has_value = value !== null && value !== undefined && value !== ""; | |
| // For boolean fields, 0 should be treated as a set value (false) | |
| // Only treat as unset if truly empty | |
| // For child tables, check length | |
| if (Array.isArray(value)) { | |
| has_value = value.length > 0; | |
| } |
🤖 Prompt for AI Agents
In buzz/public/js/events/create_from_template.js around lines 314 to 321, the
current presence check treats the numeric 0 as "not set" (value !== 0), causing
legitimate zero values to be ignored; update the logic to consider only null,
undefined, and empty string as absent (and keep the Array.isArray length check
for child tables) so numeric 0 is treated as a valid value.
|
In "Create from Template" dialog, fields marked as "Not set" are still checkable. |
|
When |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@buzz/public/js/events/create_from_template.js`:
- Around line 267-306: The update_mandatory_fields_visibility function only
enforces fields from buzz.events.MANDATORY_FIELDS; add logic so when
auto_send_pitch_deck is enabled (check
dialog.get_field("auto_send_pitch_deck").get_value() or
template.auto_send_pitch_deck) you also treat sponsor_deck_email_template and
sponsor_deck_reply_to as required candidates: if those template values are
missing or their checkboxes are unchecked, push them onto missing_fields and set
dialog.get_field("<field>").df.hidden = 0, df.reqd = 1 and refresh(), otherwise
hide and un-require them; do this by extending the existing loop or adding a
small conditional block after it inside update_mandatory_fields_visibility and
ensure dialog contains hidden fields for sponsor_deck_email_template and
sponsor_deck_reply_to so they can be shown.
♻️ Duplicate comments (1)
buzz/public/js/events/create_from_template.js (1)
314-321: Zero value treated as "not set" - previously flagged.The check
value !== 0causes fields with legitimate value0(e.g.,tax_percentage: 0) to show as "Not set". This was noted in a previous review.
🧹 Nitpick comments (4)
buzz/public/js/events/create_from_template.js (4)
120-131: Consider adding error handling for the API call.If the
frappe.client.getcall fails, the user receives no feedback. Consider adding an error callback to display an appropriate message.💡 Suggested improvement
frappe.call({ method: "frappe.client.get", args: { doctype: "Event Template", name: template_name, }, callback: function (r) { if (r.message) { buzz.events.render_template_options(dialog, r.message); } }, + error: function () { + frappe.msgprint(__("Failed to load template. Please try again.")); + }, });
391-411: Consider adding error handling for the server call.Similar to the template fetch, if
create_from_templatefails, the user receives no feedback while the freeze message disappears silently.💡 Suggested improvement
frappe.call({ method: "buzz.events.doctype.buzz_event.buzz_event.create_from_template", args: { template_name: template_name, options: JSON.stringify(options), additional_fields: JSON.stringify(additional_fields), }, freeze: true, freeze_message: __("Creating Event..."), callback: function (r) { if (r.message) { dialog.hide(); frappe.show_alert({ message: __("Event created successfully"), indicator: "green", }); frappe.set_route("Form", "Buzz Event", r.message); } }, + error: function () { + frappe.msgprint(__("Failed to create event. Please try again.")); + }, });
604-662: Consider batching the async count fetches.Three separate
frappe.client.get_countcalls are made sequentially. While this works, batching them could reduce network overhead and improve perceived performance.💡 Suggested approach using Promise.all
// Batch all count fetches Promise.all([ frappe.call({ method: "frappe.client.get_count", args: { doctype: "Event Ticket Type", filters: { event: doc.name } }, }), frappe.call({ method: "frappe.client.get_count", args: { doctype: "Ticket Add-on", filters: { event: doc.name } }, }), frappe.call({ method: "frappe.client.get_count", args: { doctype: "Buzz Custom Field", filters: { event: doc.name } }, }), ]).then(([ticketTypes, addOns, customFields]) => { // Update all three placeholders updateOptionHtml($wrapper, "#ticket-types-option", "ticket_types", __("Ticket Types"), ticketTypes.message || 0); updateOptionHtml($wrapper, "#add-ons-option", "add_ons", __("Add-ons"), addOns.message || 0); updateOptionHtml($wrapper, "#custom-fields-option", "custom_fields", __("Custom Fields"), customFields.message || 0); });
697-716: Consider adding error handling for the save template call.Consistent with other server calls in this file, adding an error callback would improve user experience when the operation fails.
💡 Suggested improvement
frappe.call({ method: "buzz.events.doctype.event_template.event_template.create_template_from_event", args: { event_name: frm.doc.name, template_name: values.template_name, options: JSON.stringify(options), }, freeze: true, freeze_message: __("Creating Template..."), callback: function (r) { if (r.message) { dialog.hide(); frappe.show_alert({ message: __("Template {0} created successfully", [r.message]), indicator: "green", }); frappe.set_route("Form", "Event Template", r.message); } }, + error: function () { + frappe.msgprint(__("Failed to create template. Please try again.")); + }, });
| buzz.events.update_mandatory_fields_visibility = function (dialog, template) { | ||
| let missing_fields = []; | ||
|
|
||
| for (let field of buzz.events.MANDATORY_FIELDS) { | ||
| let template_has_value = template[field] && template[field] !== ""; | ||
| let checkbox = dialog | ||
| .get_field("field_options") | ||
| .$wrapper.find(`.template-option[data-option="${field}"]`); | ||
| let is_checked = checkbox.is(":checked"); | ||
|
|
||
| // If template doesn't have value OR checkbox is unchecked, we need to ask user | ||
| if (!template_has_value || !is_checked) { | ||
| missing_fields.push(field); | ||
| dialog.get_field(field).df.hidden = 0; | ||
| dialog.get_field(field).df.reqd = 1; | ||
| dialog.get_field(field).refresh(); | ||
| } else { | ||
| dialog.get_field(field).df.hidden = 1; | ||
| dialog.get_field(field).df.reqd = 0; | ||
| dialog.get_field(field).refresh(); | ||
| } | ||
| } | ||
|
|
||
| // Show/hide the section | ||
| if (missing_fields.length > 0) { | ||
| dialog.get_field("missing_fields_section").df.hidden = 0; | ||
| dialog.get_field("missing_fields_section").refresh(); | ||
| dialog | ||
| .get_field("missing_fields_info") | ||
| .$wrapper.html( | ||
| `<p class="text-muted small">${__( | ||
| "The following required fields are not set in the template or not selected. Please fill them in:" | ||
| )}</p>` | ||
| ); | ||
| } else { | ||
| dialog.get_field("missing_fields_section").df.hidden = 1; | ||
| dialog.get_field("missing_fields_section").refresh(); | ||
| dialog.get_field("missing_fields_info").$wrapper.html(""); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Missing conditional required fields logic for auto_send_pitch_deck.
Per the PR review comment, when auto_send_pitch_deck is checked but sponsor_deck_email_template or sponsor_deck_reply_to are unchecked, they should dynamically appear in the "Required Fields" section. The current implementation only handles the static MANDATORY_FIELDS list.
💡 Suggested approach
buzz.events.update_mandatory_fields_visibility = function (dialog, template) {
let missing_fields = [];
+
+ // Check conditional dependencies
+ let auto_send_checkbox = dialog
+ .get_field("field_options")
+ .$wrapper.find('.template-option[data-option="auto_send_pitch_deck"]');
+ let auto_send_checked = auto_send_checkbox.is(":checked");
+
+ // Determine which fields are conditionally required
+ let conditional_required = [];
+ if (auto_send_checked) {
+ conditional_required.push("sponsor_deck_email_template", "sponsor_deck_reply_to");
+ }
+
+ let all_required = [...buzz.events.MANDATORY_FIELDS, ...conditional_required];
- for (let field of buzz.events.MANDATORY_FIELDS) {
+ for (let field of all_required) {
let template_has_value = template[field] && template[field] !== "";
// ... rest of the logicNote: You may also need to add corresponding hidden dialog fields for sponsor_deck_email_template and sponsor_deck_reply_to if they can be user-provided when missing from the template.
🤖 Prompt for AI Agents
In `@buzz/public/js/events/create_from_template.js` around lines 267 - 306, The
update_mandatory_fields_visibility function only enforces fields from
buzz.events.MANDATORY_FIELDS; add logic so when auto_send_pitch_deck is enabled
(check dialog.get_field("auto_send_pitch_deck").get_value() or
template.auto_send_pitch_deck) you also treat sponsor_deck_email_template and
sponsor_deck_reply_to as required candidates: if those template values are
missing or their checkboxes are unchecked, push them onto missing_fields and set
dialog.get_field("<field>").df.hidden = 0, df.reqd = 1 and refresh(), otherwise
hide and un-require them; do this by extending the existing loop or adding a
small conditional block after it inside update_mandatory_fields_visibility and
ensure dialog contains hidden fields for sponsor_deck_email_template and
sponsor_deck_reply_to so they can be shown.
Saving a template
Using a template
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.