Skip to content

fix(demo): wire basic demo zip button to a real DownloadHandler#198

Open
paodb wants to merge 1 commit into
masterfrom
issue-192
Open

fix(demo): wire basic demo zip button to a real DownloadHandler#198
paodb wants to merge 1 commit into
masterfrom
issue-192

Conversation

@paodb
Copy link
Copy Markdown
Member

@paodb paodb commented May 20, 2026

The "Download zip file" anchor in GridExporterDemo was created with an empty href (new Anchor("", icon) + download=true), so clicking it caused the browser to save the current page's HTML instead of a zip.

The fix in this PR, replaces the empty href with a DownloadHandler that streams a ZIP of the four exported formats (xlsx, docx, pdf, csv) on demand.

Close #192

Summary by CodeRabbit

  • Improvements

    • ZIP export now bundles multiple formats (XLSX, DOCX, PDF, CSV) into a single downloadable archive for convenient multi-format exports.
    • Downloads are streamed from the server for a smoother, more reliable download experience.
  • Bug Fixes

    • Improved ZIP download handling to prevent premature stream closure and ensure all included files are delivered intact.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 71cdbd32-fe59-441f-bbf4-2b91d277ba0a

📥 Commits

Reviewing files that changed from the base of the PR and between 6db08bb and 71cae93.

📒 Files selected for processing (1)
  • src/test/java/com/flowingcode/vaadin/addons/gridexporter/GridExporterDemo.java

Walkthrough

GridExporterDemo wires a Vaadin DownloadHandler that streams a ZipOutputStream containing four export entries (.xlsx, .docx, .pdf, .csv). A FilterOutputStream wrapper prevents inner writers from closing the ZIP stream. The demo Anchor is updated to use the handler (no explicit download attribute).

Changes

Server-side ZIP Download Implementation

Layer / File(s) Summary
Server-side ZIP download implementation
src/test/java/com/flowingcode/vaadin/addons/gridexporter/GridExporterDemo.java
Adds imports for DownloadHandler, ZipOutputStream, ZipEntry, and FilterOutputStream (lines 37–49). Implements a DownloadHandler that creates a ZIP, writes four export-format entries using existing stream writers, wraps the ZIP stream in a non-closing FilterOutputStream to prevent inner writers from closing the ZIP, and updates the ZIP Anchor to set href to the handler while removing the explicit download attribute (lines 106–136).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • javier-godoy
  • mlopezFC
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: wiring the demo ZIP download button to a functional DownloadHandler instead of an empty href.
Linked Issues check ✅ Passed The PR implementation directly addresses issue #192 by replacing the non-functional empty href with a DownloadHandler that streams a ZIP file containing the four export formats (xlsx, docx, pdf, csv).
Out of Scope Changes check ✅ Passed All changes are focused on fixing the ZIP download functionality in GridExporterDemo and are directly related to resolving issue #192; no out-of-scope modifications are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-192

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/test/java/com/flowingcode/vaadin/addons/gridexporter/GridExporterDemo.java`:
- Around line 110-113: The anonymous FilterOutputStream assigned to nonClosing
overrides close() but leaves it empty; change close() to call flush() (and leave
out closing zos) so buffered data is flushed before callers invoke
close/closeEntry; update the override in the nonClosing FilterOutputStream (the
anonymous class wrapping zos) to call flush() and add a short comment explaining
that close is overridden to flush but not close the underlying ZipOutputStream
to satisfy the SonarCloud finding.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 606bdef3-a09e-4d07-825e-ca11e7459d02

📥 Commits

Reviewing files that changed from the base of the PR and between 6db08bb and 4124f8c.

📒 Files selected for processing (1)
  • src/test/java/com/flowingcode/vaadin/addons/gridexporter/GridExporterDemo.java

@sonarqubecloud
Copy link
Copy Markdown

@paodb
Copy link
Copy Markdown
Member Author

paodb commented May 20, 2026

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

✅ Actions performed

Full review triggered.

@paodb paodb marked this pull request as ready for review May 20, 2026 23:18
@paodb paodb requested review from javier-godoy and scardanzan May 20, 2026 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To Do

Development

Successfully merging this pull request may close these issues.

Demo: download zip file does not work

1 participant