Skip to content

I37 bulk download - #39

Merged
QSparks merged 5 commits into
i34-controlled-startupfrom
i37-bulk-download
Aug 4, 2026
Merged

I37 bulk download#39
QSparks merged 5 commits into
i34-controlled-startupfrom
i37-bulk-download

Conversation

@QSparks

@QSparks QSparks commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Resolves #37

@QSparks
QSparks requested a review from Nospamas July 29, 2026 20:57
@QSparks QSparks self-assigned this Jul 29, 2026
Nospamas
Nospamas requested changes Jul 30, 2026
View reviewed changes

@Nospamas Nospamas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Setting to request changes for small feedback.

Comment thread app/src/services/timeseriesApi.js Outdated
const filename = `${outletId}_${direction}_${selections.model}_${selections.scenario}_${apiVariable}.nc`
.replace(/[^A-Za-z0-9_.-]+/g, "_");
const url = URL.createObjectURL(blob);
const a = document.createElement("a");

@Nospamas Nospamas Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seperation of concerns is a bit broken here, API surface maybe shouldn't be generating html content (leave that to JSX/React) but if you have good reason I'll allow it.

@QSparks QSparks Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I see what you mean. I had followed the existing patterns for the single-timeseries and GeoJSON downloads that used a temporary anchor to trigger the browser download.

I’ve moved this out of the API services and into a shared download utility, and updated the three download paths.
The single-timeseries API returns a download URL and filename, while the bulk NetCDF and GeoJSON APIs return a blob and filename. InteractionLayer and DataSelectionTable initiate their respective downloads.

@QSparks
QSparks requested a review from Nospamas July 31, 2026 20:16
Nospamas
Nospamas approved these changes Aug 4, 2026
View reviewed changes

@Nospamas Nospamas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks cleaner, approved 👍

@QSparks
QSparks merged commit 01fe88f into i34-controlled-startup Aug 4, 2026
1 check passed
QSparks added a commit that referenced this pull request Aug 4, 2026
@QSparks
I34 controlled startup (#36), I37 bulk download (#39)
a0bf4ee
QSparks added a commit that referenced this pull request Aug 4, 2026
@QSparks
I33 lighthouse (#35), I34 controlled startup (#36), I37 bulk download (…
87909b2 
…#39)
QSparks added a commit that referenced this pull request Aug 4, 2026
@QSparks
I31 (#32), I33 (#35), I34 (#36), I37 (#39)
729cb8b 
* I31 peace columbia (#32)
* I33 lighthouse (#35), 
* I34 controlled startup (#36)
* I37 bulk download (#39)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nospamas Nospamas Nospamas approved these changes

Assignees

@QSparks QSparks

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@QSparks @Nospamas