feat: event template save and use #106
Conversation
📝 WalkthroughWalkthroughThis PR introduces a comprehensive Event Template feature enabling users to create Buzz Events from predefined templates and save existing events as templates. It adds a new Event Template doctype with three child doctypes, backend APIs for template-based creation and saving, frontend UI dialogs for template selection and field selection, role-based permissions (Event Manager, System Manager), and extensive unit test coverage for all flows and edge cases. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Client)
participant UI as Buzz Event List UI
participant Dialog as Template Dialog
participant Server as Backend API
participant DB as Database
User->>UI: Click "Create from Template"
UI->>Dialog: Show template selection dialog
Dialog->>Server: Fetch available templates
Server->>DB: Query Event Template
DB-->>Server: Template list
Server-->>Dialog: Return templates
User->>Dialog: Select template
Dialog->>Server: Fetch selected template details
Server->>DB: Load Event Template + child docs
DB-->>Server: Template data (ticket types, add-ons, custom fields, counts)
Server-->>Dialog: Template schema & document counts
Dialog->>Dialog: Render option checkboxes (event fields, payment_gateways, ticket_types, add_ons, custom_fields)
Dialog->>Dialog: Render mandatory field inputs (category, host)
User->>Dialog: Select options & enter mandatory fields
User->>Dialog: Click "Create"
Dialog->>Server: Call create_from_template(template_name, options, additional_fields)
rect rgb(200, 220, 255)
note over Server: Permission checks
Server->>Server: Verify Event Template read + Buzz Event create
end
Server->>DB: Insert new Buzz Event (with copied fields)
Server->>DB: Copy payment_gateways, sponsor_deck_attachments
Server->>DB: Create linked documents (ticket types, add-ons, custom fields)
DB-->>Server: Event created & related docs linked
Server-->>Dialog: Return new event name
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->>Form: Render "Save as Template" button (on refresh)
User->>Form: Click "Save as Template"
Form->>Dialog: Show save-as-template dialog
Dialog->>Server: Fetch current event + related document counts
Server->>DB: Load Buzz Event, ticket types, add-ons, custom fields
DB-->>Server: Event details & counts
Server-->>Dialog: Event data + counts
Dialog->>Dialog: Render field groups (event_details, ticketing_settings, sponsorship_settings)
Dialog->>Dialog: Display related document sections with counts
User->>Dialog: Select fields to copy & name template
User->>Dialog: Click "Save as Template"
Dialog->>Server: Call create_template_from_event(event_name, template_name, options)
rect rgb(200, 220, 255)
note over Server: Permission checks
Server->>Server: Verify Buzz Event read + Event Template create
end
Server->>DB: Insert new Event Template
Server->>DB: Copy selected scalar fields from event
Server->>DB: Copy child tables (payment_gateways, sponsor_deck_attachments)
Server->>DB: Import linked documents (ticket types, add-ons, custom fields)
DB-->>Server: Template created & documents linked
Server-->>Dialog: Return new template name
Dialog->>User: Redirect to new Event Template
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
Saving a template
Using a template
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.