batches: implement export changeset bulk operation#56721
Conversation
75283a1 to
e9b815b
Compare
bae566e to
00b6fa7
Compare
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 69bb847...5a0bf02.
|
There was a problem hiding this comment.
Some comments inline, but first a general question: why is the backend side of this needed? It looks like this is just exporting information about all the changesets associated with a batch change, which can already be done with the batchChange { changesets { ... } } GraphQL query. A client can use that GraphQL query to export as whatever format they want, including CSV.
As a general rule, I think it's preferable to have the client format the data differently if that's how they want it. I don't think we want to cement the format of this API to CSV with a fixed set of columns. Instead, if there is information that is not available on the ChangeSet resolver, we should make those fields accessible via GraphQL. That way, the client can choose exactly which fields they care about, can format it however they want for export, and doesn't require downloading a potentially gigantic CSV string stuffed into a JSON reponse (and can instead be streamed with pagination).
We solve this differently for exporting search results, where we construct the CSV in the client. I'd recommend we look at that approach and consider doing something similar.
This slipped through the cracks of the walls of my notification system. I'll take a look at the approach for exporting search results. Thanks |
00b6fa7 to
e478a39
Compare
|
@camdencheek This is ready for review once more. I've moved the data generation to the client side and now support JSON and CSV. Here's a quick walkthrough of what it looks like. CleanShot.2023-10-13.at.17.30.41.mp4 |
| const element = document.createElement('a') | ||
| element.download = `batch_change_export.${selectedDataExportType}` | ||
| element.href = url | ||
| document.body.append(element) | ||
| element.click() |
There was a problem hiding this comment.
Should we remove the element after we're done with it?
There was a problem hiding this comment.
Oh, nice catch. I'll remove that.
There was a problem hiding this comment.
Super minor, I think this logic already exists in the DownloadFileButton component in the wildcard, I'm not sure about the context here and can we use it here instead of manual blob creation logic, but just FYI for further cases like this. https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/client/web/src/components/DownloadFileButton.tsx?L15:14
4e32f5d to
5a0bf02
Compare
Co-authored-by: topebali <topebali97@gmail.com>
This PR builds on the existing work done by @topebali in #55850. I made this a separate PR because I wanted to keep #55850 for reference since this PR introduces a new approach for creating data for download via the graphQL API.
We use Go's CSV module to generate and create a CSV based on the changeset IDs passed; this CSV is then returned to the UI as a string. We then take this string and create URL object to make it available for download.
output.mp4
Test plan