Skip to content

Conversation

@ArnavBallinCode
Copy link
Contributor

After PR #1258 fixed the export page HTTP 500 error, This pr fixes issue #1282 , export functionality which was broken for some of the exporters

Issues found and fixed

  • Code still referenced old item field instead of new product field
  • JSON_FIELD_AVAILABLE setting was undefined
  • Form data accessed without defaults
  • Invalid Query Syntax**: Wrong Django filter methods
  • Empty Data Handling**: No safeguards for reports with no data

Related Issues

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @ArnavBallinCode, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copy link
Member

@mariobehling mariobehling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please resolve conflicts.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes multiple exporter failures that occurred after a database migration changed the item field to product throughout the codebase. The changes address field reference errors, undefined settings, missing form data defaults, invalid query syntax, and inadequate handling of empty data scenarios.

  • Updates field references from item to product across all affected exporters
  • Adds defensive programming for form data access and empty data scenarios
  • Fixes Django ORM query syntax errors
  • Improves error handling in export services

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
app/eventyay/plugins/ticketoutputpdf/exporters.py Adds handling for empty ticket exports by tracking any_tickets flag
app/eventyay/plugins/reports/exporters.py Adds defensive checks for num data access to prevent KeyError on empty reports
app/eventyay/plugins/checkinlists/exporters.py Updates itemproduct field references and adds form_data defaults
app/eventyay/plugins/badges/exporters.py Relocates empty badge check before PDF write operation
app/eventyay/config/settings.py Defines missing JSON_FIELD_AVAILABLE setting
app/eventyay/base/services/export.py Enhances error handling with generic exception catch and user-friendly messages
app/eventyay/base/exporters/waitinglist.py Updates field references and fixes indentation in row construction
app/eventyay/base/exporters/orderlist.py Updates field references and adds form_data.get() defaults
app/eventyay/base/exporters/json.py Updates itemsproducts and item_idproduct_id references
app/eventyay/base/exporters/dekodi.py Fixes invalid Django query syntax from state= to state__in=

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +121 to +130
entry.created.astimezone(tz).strftime(datetime_format), # alternative: .isoformat(),
entry.name,
entry.email,
entry.phone,
str(entry.product) if entry.product else '',
str(entry.variation) if entry.variation else '',
entry.event.slug,
entry.event.name,
entry.subevent.name if entry.subevent else '',
event_for_date_columns.date_from.astimezone(tz).strftime(datetime_format),
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent indentation in list construction. Lines 121-130 should be indented to align with the opening bracket on line 120. All list items should have consistent indentation (either all at the same level as line 120, or all indented one level deeper).

Suggested change
entry.created.astimezone(tz).strftime(datetime_format), # alternative: .isoformat(),
entry.name,
entry.email,
entry.phone,
str(entry.product) if entry.product else '',
str(entry.variation) if entry.variation else '',
entry.event.slug,
entry.event.name,
entry.subevent.name if entry.subevent else '',
event_for_date_columns.date_from.astimezone(tz).strftime(datetime_format),
entry.created.astimezone(tz).strftime(datetime_format), # alternative: .isoformat(),
entry.name,
entry.email,
entry.phone,
str(entry.product) if entry.product else '',
str(entry.variation) if entry.variation else '',
entry.event.slug,
entry.event.name,
entry.subevent.name if entry.subevent else '',
event_for_date_columns.date_from.astimezone(tz).strftime(datetime_format),

Copilot uses AI. Check for mistakes.
Comment on lines +378 to +380
num_data = tup[0].num.get(l, (0, 0, 0)) if hasattr(tup[0], 'num') and tup[0].num else (0, 0, 0)
tdata[-1].append(str(num_data[0] if len(num_data) > 0 else 0))
tdata[-1].append(floatformat(num_data[2 if net else 1] if len(num_data) > (2 if net else 1) else 0, places))
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The defensive check if len(num_data) > 0 on line 379 is redundant since num_data is guaranteed to be a tuple of length 3 from the get() default (0, 0, 0) on line 378. The same applies to the length check on line 380. These checks can be simplified to directly access num_data[0] and num_data[2 if net else 1].

Copilot uses AI. Check for mistakes.
Comment on lines +384 to +386
num_data = item.num.get(l, (0, 0, 0)) if hasattr(item, 'num') and item.num else (0, 0, 0)
tdata[-1].append(str(num_data[0] if len(num_data) > 0 else 0))
tdata[-1].append(floatformat(num_data[2 if net else 1] if len(num_data) > (2 if net else 1) else 0, places))
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The defensive check if len(num_data) > 0 on line 385 is redundant since num_data is guaranteed to be a tuple of length 3 from the get() default (0, 0, 0) on line 384. The same applies to the length check on line 386. These checks can be simplified to directly access num_data[0] and num_data[2 if net else 1].

Copilot uses AI. Check for mistakes.
Comment on lines +391 to +393
num_data = var.num.get(l, (0, 0, 0)) if hasattr(var, 'num') and var.num else (0, 0, 0)
tdata[-1].append(str(num_data[0] if len(num_data) > 0 else 0))
tdata[-1].append(floatformat(num_data[2 if net else 1] if len(num_data) > (2 if net else 1) else 0, places))
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The defensive check if len(num_data) > 0 on line 392 is redundant since num_data is guaranteed to be a tuple of length 3 from the get() default (0, 0, 0) on line 391. The same applies to the length check on line 393. These checks can be simplified to directly access num_data[0] and num_data[2 if net else 1].

Copilot uses AI. Check for mistakes.
Comment on lines +403 to +404
tdata[-1].append(str(num_data[0] if len(num_data) > 0 else 0))
tdata[-1].append(floatformat(num_data[2 if net else 1] if len(num_data) > (2 if net else 1) else 0, places))
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The defensive check if len(num_data) > 0 on line 403 is redundant since num_data is guaranteed to be a tuple of length 3 from the get() default (0, 0, 0) on line 402. The same applies to the length check on line 404. These checks can be simplified to directly access num_data[0] and num_data[2 if net else 1].

Suggested change
tdata[-1].append(str(num_data[0] if len(num_data) > 0 else 0))
tdata[-1].append(floatformat(num_data[2 if net else 1] if len(num_data) > (2 if net else 1) else 0, places))
tdata[-1].append(str(num_data[0]))
tdata[-1].append(floatformat(num_data[2 if net else 1], places))

Copilot uses AI. Check for mistakes.
Comment on lines +235 to +237
if not any:
raise OrderError(_('None of the selected products is configured to print badges.'))

Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving the empty badge check before writing metadata prevents writing corrupt PDFs, but the check should occur before attempting to write to outbuffer (line 244). If any is False, output_pdf_writer has no pages added, and calling write(outbuffer) on line 244 may still produce an invalid or empty PDF before the error is raised on line 236. The check should be moved to before line 238 to prevent any write operations.

Suggested change
if not any:
raise OrderError(_('None of the selected products is configured to print badges.'))
# Check for empty badge set before writing metadata or PDF
if not any:
raise OrderError(_('None of the selected products is configured to print badges.'))

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Export Functionality Broken

2 participants