feature(network-visualization): Add button to export HTML from Shiny#176
Merged
tonywu1999 merged 6 commits intodevelfrom Mar 5, 2026
Merged
feature(network-visualization): Add button to export HTML from Shiny#176tonywu1999 merged 6 commits intodevelfrom
tonywu1999 merged 6 commits intodevelfrom
Conversation
📝 WalkthroughWalkthroughReplaces widget-based HTML export with a direct call to Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant Server
participant Exporter as exportNetworkToHTML
participant FS as FileSystem
UI->>Server: click "Download HTML"
Server->>Server: validate render_data
alt render_data NULL
Server-->>UI: show error notification
else render_data present
Server->>Exporter: exportNetworkToHTML(render_data, displayLabelType, tempFile)
Exporter->>FS: write temp HTML
Exporter-->>Server: return tempFile path
Server->>FS: copy tempFile -> userFile
Server-->>UI: serve download
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@R/module-visualize-network-server.R`:
- Around line 627-638: tmp_file is not unique (uses Sys.Date()) and file.copy
result is unchecked; change tmp_file creation to use a unique temp file (e.g.,
tempfile with pattern and .html extension) so concurrent downloads won't
collide, ensure exportNetworkToHTML writes to that tmp_file, then check the
result of file.exists(tmp_file) and the boolean returned by file.copy(tmp_file,
file) and fail fast (stop or throw) with a clear error if either check indicates
the export or copy failed.
- Around line 629-635: The unqualified call to exportNetworkToHTML will fail
because MSstatsBioNet is in Imports and exportNetworkToHTML isn’t imported;
change the call to use the namespace qualifier
MSstatsBioNet::exportNetworkToHTML (update the call where exportNetworkToHTML is
invoked with nodes = render_data$nodes_table, edges = render_data$edges_table,
nodeFontSize = 12, displayLabelType = input$displayLabelType, filename =
tmp_file), and also update the equivalent generated code string earlier (the
codes string around line 544) to use MSstatsBioNet::exportNetworkToHTML so both
runtime and generated code reference the fully qualified function name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67cc42bc-4f5f-4538-89e0-0d5158fa6d29
📒 Files selected for processing (2)
R/module-visualize-network-server.RR/module-visualize-network-ui.R
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
R/module-visualize-network-server.R (2)
236-242:⚠️ Potential issue | 🔴 CriticalUse
MSstatsBioNet::exportNetworkToHTMLexplicitly at call sites.The direct call on Line 236 (and generated call on Line 562) should be namespace-qualified to prevent runtime lookup issues when the symbol is not imported into the package namespace.
#!/bin/bash # Verify whether exportNetworkToHTML is imported and where it is called unqualified. set -euo pipefail echo "== NAMESPACE imports for MSstatsBioNet ==" rg -n 'importFrom\(MSstatsBioNet|import\(MSstatsBioNet\)|exportNetworkToHTML' NAMESPACE -C2 || true echo echo "== Roxygen importFrom in server module ==" rg -n '@importFrom MSstatsBioNet' R/module-visualize-network-server.R -C1 || true echo echo "== Unqualified call sites ==" rg -nP '\bexportNetworkToHTML\s*\(' R/module-visualize-network-server.R -C2 || trueProposed fix
- exportNetworkToHTML( + MSstatsBioNet::exportNetworkToHTML( nodes = render_data$nodes_table, edges = render_data$edges_table, nodeFontSize = 12, displayLabelType = displayLabelType, filename = tmp_file )- codes <- paste(codes, "exportNetworkToHTML(subnetwork$nodes, subnetwork$edges, displayLabelType=", displayLabelTypeStr, ")\n", sep = "") + codes <- paste(codes, "MSstatsBioNet::exportNetworkToHTML(subnetwork$nodes, subnetwork$edges, displayLabelType=", displayLabelTypeStr, ")\n", sep = "")Also applies to: 562-562
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@R/module-visualize-network-server.R` around lines 236 - 242, The call to exportNetworkToHTML should be namespace-qualified to avoid runtime lookup failures; replace unqualified calls like exportNetworkToHTML(...) with MSstatsBioNet::exportNetworkToHTML(...) at all call sites (e.g., in the block using render_data$nodes_table and render_data$edges_table and the other generated call around the symbol noted later), ensuring you update every occurrence so the function is resolved from the MSstatsBioNet namespace.
234-245:⚠️ Potential issue | 🟠 MajorAvoid date-based temp filenames and fail fast on copy/export failures.
Line 234 can collide across concurrent downloads, and Line 244 ignores copy failure. This can silently return bad downloads.
Proposed fix
- tmp_file <- file.path(tempdir(), paste0("network-", Sys.Date(), ".html")) + tmp_file <- tempfile(pattern = "network-", fileext = ".html") exportNetworkToHTML( nodes = render_data$nodes_table, edges = render_data$edges_table, nodeFontSize = 12, displayLabelType = displayLabelType, filename = tmp_file ) - - file.copy(tmp_file, file) + + if (!file.exists(tmp_file)) { + stop("Failed to generate HTML export file.") + } + copied <- file.copy(tmp_file, file, overwrite = TRUE) + if (!copied) { + stop("Failed to prepare HTML download file.") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@R/module-visualize-network-server.R` around lines 234 - 245, The temp filename uses a date-based name (tmp_file) which can collide in concurrent downloads and the code ignores failures from exportNetworkToHTML and file.copy; replace the date-based tmp_file with a unique tempfile() name, ensure exportNetworkToHTML errors are propagated or its return is checked, and check the boolean result of file.copy (or capture exceptions) and stop()/throw an error on failure so the download fails fast; refer to tmp_file, exportNetworkToHTML, and file.copy to locate the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@R/module-visualize-network-server.R`:
- Around line 639-644: The downloadHandler content currently swallows errors by
only calling showNotification in the error handler, so failing exports can
produce empty/broken files; update the tryCatch around networkVisualization() /
export_network_html(...) inside the downloadHandler content function to rethrow
the error after calling showNotification (e.g., call stop(e) or rethrow the
original error) so the download is aborted and the error propagates to the
caller; keep showNotification(e$message, type="error") for user feedback and
then rethrow the same error.
---
Duplicate comments:
In `@R/module-visualize-network-server.R`:
- Around line 236-242: The call to exportNetworkToHTML should be
namespace-qualified to avoid runtime lookup failures; replace unqualified calls
like exportNetworkToHTML(...) with MSstatsBioNet::exportNetworkToHTML(...) at
all call sites (e.g., in the block using render_data$nodes_table and
render_data$edges_table and the other generated call around the symbol noted
later), ensuring you update every occurrence so the function is resolved from
the MSstatsBioNet namespace.
- Around line 234-245: The temp filename uses a date-based name (tmp_file) which
can collide in concurrent downloads and the code ignores failures from
exportNetworkToHTML and file.copy; replace the date-based tmp_file with a unique
tempfile() name, ensure exportNetworkToHTML errors are propagated or its return
is checked, and check the boolean result of file.copy (or capture exceptions)
and stop()/throw an error on failure so the download fails fast; refer to
tmp_file, exportNetworkToHTML, and file.copy to locate the changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bd1cee16-c0e0-4c25-862d-0173f5b8ab9d
📒 Files selected for processing (2)
R/module-visualize-network-server.Rtests/testthat/test_network_visualization.R
tonywu1999
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation and Context
This PR adds HTML export support to the network visualization module so users can download interactive network visualizations as standalone HTML files in addition to the existing code-download feature. The goal is to allow saving and sharing rendered networks outside the Shiny app.
Solution summary:
Detailed Changes
R/module-visualize-network-server.R
R/module-visualize-network-ui.R
Public API / exports
Diff size (per manifest summary): roughly +39/-2 in server file and +5/-3 in UI file (lines changed reported in raw summary).
Unit Tests
Coding Guidelines / Style Violations