-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MM-23110 CSV Exporter #3
Conversation
- Build an iterator from the posts of the channel and pass it to the exporter
- Add FileName function - Change batch size owner - Move functions to correct file - Fix go.mod - Document public and important functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Just a few comments below.
Use CloseWithError in the writer end of the pipe to inform the reader end that there was an error in the process. Use a specific type of error, sent by the writer and checked by the reader, to make sure that only one error post is sent to the user.
After a meaningful conversation with @cpoile, I'm reimplementing the users cache to something much simpler, rebuilt in every command and not shared at all. I'll ping you when this is ready, @lieut-data! |
This reverts commit e5ff6f0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @agarciamontoro! 🎉 Non-blocking comment below:
@agarciamontoro, propose we merge as-is, and schedule a PM review alongside cutting the "first release". I think that would make this ready to merge if you're ok with that approach. |
Sounds good, yup. Merging! |
* Use io.Pipe to export and upload concurrently * Isolate exporter interface and CSV implementation * Make exporters independent of the plugin client - Build an iterator from the posts of the channel and pass it to the exporter * Reorganize, clean and document code - Add FileName function - Change batch size owner - Move functions to correct file - Fix go.mod - Document public and important functions * Test CSVExporter * Increase number of posts retrieved per API call * Use errors New and Wrap instead of fmt.Errorf * Convert creation timestamps to time.Time * Rename CSVExporter to CSV * Move exportedPost to TestExport * Add a users cache * go mod tidy * Propagate the writer error to the reader Use CloseWithError in the writer end of the pipe to inform the reader end that there was an error in the process. Use a specific type of error, sent by the writer and checked by the reader, to make sure that only one error post is sent to the user. * Add "CSV" to tests name * Modify users cache to a local, per export, map This reverts commit e5ff6f0.
* Use io.Pipe to export and upload concurrently * Isolate exporter interface and CSV implementation * Make exporters independent of the plugin client - Build an iterator from the posts of the channel and pass it to the exporter * Reorganize, clean and document code - Add FileName function - Change batch size owner - Move functions to correct file - Fix go.mod - Document public and important functions * Test CSVExporter * Increase number of posts retrieved per API call * Use errors New and Wrap instead of fmt.Errorf * Convert creation timestamps to time.Time * Rename CSVExporter to CSV * Move exportedPost to TestExport * Add a users cache * go mod tidy * Propagate the writer error to the reader Use CloseWithError in the writer end of the pipe to inform the reader end that there was an error in the process. Use a specific type of error, sent by the writer and checked by the reader, to make sure that only one error post is sent to the user. * Add "CSV" to tests name * Modify users cache to a local, per export, map This reverts commit e5ff6f0.
* MM-23044: Bootstrap channel export plugin and add a dummy exporter (#1) * Bootstrap channel export plugin - Delete the webapp directory, as for now, we only need the server part - Adapt plugin.json and regenerates server/manifest.go - Add mattermost-plugin-api dependency - Adapt README from the starter template * Set up the plugin structure and a dummy exporter - Ensure a bot when activating the plugin. - Add a command, /export, to export a channel. For the moment, it just outputs an empty JSON. * Use plugin-ci orb * Fix bot description Co-Authored-By: Jesse Hallam <[email protected]> * Reword autocomplete description Co-Authored-By: Jesse Hallam <[email protected]> * Remove unused code * Address PR minor comments - Make err handling more idiomatic - Rename exportCommandTrigger variable * Decouple communication to user and export process Co-authored-by: Jesse Hallam <[email protected]> * go mod tidy (#2) * MM-23110 CSV Exporter (#3) * Use io.Pipe to export and upload concurrently * Isolate exporter interface and CSV implementation * Make exporters independent of the plugin client - Build an iterator from the posts of the channel and pass it to the exporter * Reorganize, clean and document code - Add FileName function - Change batch size owner - Move functions to correct file - Fix go.mod - Document public and important functions * Test CSVExporter * Increase number of posts retrieved per API call * Use errors New and Wrap instead of fmt.Errorf * Convert creation timestamps to time.Time * Rename CSVExporter to CSV * Move exportedPost to TestExport * Add a users cache * go mod tidy * Propagate the writer error to the reader Use CloseWithError in the writer end of the pipe to inform the reader end that there was an error in the process. Use a specific type of error, sent by the writer and checked by the reader, to make sure that only one error post is sent to the user. * Add "CSV" to tests name * Modify users cache to a local, per export, map This reverts commit e5ff6f0. * MM-23729 Add mocking and test exporter functions (#4) * Use io.Pipe to export and upload concurrently * Isolate exporter interface and CSV implementation * Make exporters independent of the plugin client - Build an iterator from the posts of the channel and pass it to the exporter * Reorganize, clean and document code - Add FileName function - Change batch size owner - Move functions to correct file - Fix go.mod - Document public and important functions * Test CSVExporter * Increase number of posts retrieved per API call * Use errors New and Wrap instead of fmt.Errorf * Convert creation timestamps to time.Time * Rename CSVExporter to CSV * Move exportedPost to TestExport * Add a users cache * go mod tidy * Propagate the writer error to the reader Use CloseWithError in the writer end of the pipe to inform the reader end that there was an error in the process. Use a specific type of error, sent by the writer and checked by the reader, to make sure that only one error post is sent to the user. * Add "CSV" to tests name * Modify users cache to a local, per export, map This reverts commit e5ff6f0. * Mock mattermost-plugin-api services * Decouple exporter functions from plugin The motivation behind this was to make the functions easily testable, but a better outcome is the decoupling of these functions from the plugin structure. * Test toExportedPost * Test channelPostsiterator * Rename apiwrapper package to pluginapi * Remove mocks' Finish: no longer needed in Go 1.14 In Go versions 1.14+, when passing a *testing.T into gomock.NewController(t), the old call to ctrl.Finish() is no longer needed. * test time conversion * fix time conversion * Use utils.TimeFromMillis instead of millisToUnix Co-authored-by: Jesse Hallam <[email protected]> * MM-24785: expose /api/v1/export (#5) * MM-24785: expose /api/v1/export * improved error handling on Write * simplified permissions handling * idiomatic empty string checking * simplify NewMattermostServerClient * clarify setupAPI workings * MM-24877: require E20 license (#7) * MM-24877: require E20 license * check license immediately after authorization * Added no-op webapp (#8) * MM-24712: export username not nickname (#6) The CSV export of a channel mistakenly included the nickname of the corresponding user, not the username. Fixes: https://mattermost.atlassian.net/browse/MM-24712 * bump to v0.2.0 * MM-25396: emit RECEIVED_WEBAPP_PLUGIN (#9) As part of https://mattermost.atlassian.net/browse/MM-17087, the webapp no longer emits `RECEIVED_WEBAPP_PLUGIN`. This makes it impossible to use the Redux store alone to detect the presence of plugins installed /after/ a page load. (The webapp still tracks all such plugins via `RECEIVED_WEBAPP_PLUGINS` on page load.) Emit the event ourselves to better interop with the incident response plugin while we wait for the webapp to fix this. Fixes: https://mattermost.atlassian.net/browse/MM-25396 * bump to v0.2.1 * Honour privacy settings (#11) * Honour privacy settings. Honour `PrivacySettings.ShowEmailAddress` for non-system administrators. Fixes: #10 * simplify showEmailAddress * clarify tests * bump to v0.2.2 * Update from starter template (#12) Co-authored-by: Alejandro García Montoro <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Michael Kochell <[email protected]> Co-authored-by: gubbins <[email protected]> * Bootstrap tests-e2e directory * Configure typescript for e2e tests * Fix dependencies, imports and types * Remove triple slash reference rule from eslint * First test working Also, I understood what was happening with the imports * Declare support functions with types * Implement test case 20 * Implement test case 21 * Implement test case 23 * Implement test case 29 * Implement test case 30 * Implement test case 9 * Implement test case 10 * Implement test case 11 * Implement test case 12 * Implement test case 13 * Implement test case 14 * Implement test case 15 * Implement test case 16 * Implement test case 15 (the second one) * Implement test case 17 * Implement test case 18 * Clean and document code * Move e2e-specific ignores to tests-e2e/.gitignore * Refactor visitDMWithBot * Use constants for open and private channels * Use baseUrl in file download regexp * Fix versions in package.json * Add visibility verification to fileAttachmentList Co-authored-by: Prapti <[email protected]> * Add visibility verification to fileAttachmentList Co-authored-by: Prapti <[email protected]> * Verify the channel has loaded before unarchiving * Fix missing opening curly brace * Switch Dm users in the ID of the sidebar Co-authored-by: Prapti <[email protected]> * Switch Dm users in the ID of the sidebar Co-authored-by: Prapti <[email protected]> * Add @tpes/node as dev dependency * Set the Message Display to Standard * Update mattermost-redux * Use apiUpdateConfig to enable archived channels * Use API to set message display preference * Verify that the server is licensed through the API * Set the teammate name format to username * Set sysadmin's teammate name format to username Co-authored-by: Jesse Hallam <[email protected]> Co-authored-by: Ben Schumacher <[email protected]> Co-authored-by: Maria A Nunez <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Michael Kochell <[email protected]> Co-authored-by: gubbins <[email protected]> Co-authored-by: Prapti <[email protected]>
Summary
This PR implements the first version of the CSV exporter.
The structure of the columns is adapted from the existing bulk export feature, removing the unneeded fields in the export of a specific channel. The columns are:
Implementation Details
The
Exporter
interface is designed so we can add more exporters in the future. It has a couple of functions:FileName
, that returns the name of the file given the name of the channel (the CSV implements this by just adding the.csv
extension to the channel name)Export
, that receives a function that returns a batch of new posts every time it is called (until there are no more posts in the channel), and aio.Writer
, used by theExporter
to write the exported posts.The architecture of the plugin is as follows:
channelPostsIterator
builds an iterator on the posts of the channel (implemented with a simple closure).io.Pipe
to concurrently export the channel and upload the file:pipeWriter
is passed to the implementation of the Exporter, along with the channel iterator built withchannelPostsIterator
.pipeReader
is passed to theUploadFile
API function.Screenshots
After executing the /export command
DM with the bot, once the file is sent
CSV example
Dummy example to see the CSV columns. There is only one user posting, apart from the system, and the first exported post is a reply to the second one.
Ticket Link
https://mattermost.atlassian.net/browse/MM-23110