Skip to content
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-23044: Bootstrap channel export plugin and add a dummy exporter #1

Merged
merged 8 commits into from
Mar 26, 2020

Conversation

agarciamontoro
Copy link
Member

@agarciamontoro agarciamontoro commented Mar 16, 2020

Summary

This PR bootstraps the channel export plugin and adds the initial structure to start development.

For now, the plugin does one thing: it exposes one command, /export, through which the user can "export" the channel they execute the command in. Once the export is ready, a bot sends the file to the user. For the moment, the exported file is just an empty JSON file with name <epoch-timestamp>_<channel-ID>.json.

  • The first commit, afbe545, has no important modifications. It only adapts the README from the starter template and make the necessary changes to plugin.json, as well as adding mattermost-plugin-api as a dependency. It also deletes the webapp directory, which won't be of use in the near future. The third commit, 8c99d86, just adds the CI configuration.
  • The important commit here is 43a998b: it ensures that the bot exists when the plugin is activated and it adds the command /export, which asynchronously triggers the export process (this the dummy part) and get the bot to send it to the user once it's finished.

EDIT after reviews:

  1. First review

Ticket Link

https://mattermost.atlassian.net/browse/MM-23044

- 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
- Ensure a bot when activating the plugin.
- Add a command, /export, to export a channel. For the moment, it just
 outputs an empty JSON.
Copy link
Member

@lieut-data lieut-data left a comment

Choose a reason for hiding this comment

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

Nice work, @agarciamontoro! Really appreciate the high-level description of the changes, as well as the discrete commits.

A few comments inline.

server/plugin.go Outdated Show resolved Hide resolved
server/plugin_test.go Outdated Show resolved Hide resolved
server/activate_hooks.go Outdated Show resolved Hide resolved
server/activate_hooks.go Outdated Show resolved Hide resolved
server/activate_hooks.go Outdated Show resolved Hide resolved
server/command_hooks.go Outdated Show resolved Hide resolved
server/command_hooks.go Outdated Show resolved Hide resolved
server/command_hooks.go Outdated Show resolved Hide resolved
server/command_hooks.go Outdated Show resolved Hide resolved
server/command_hooks.go Outdated Show resolved Hide resolved
agarciamontoro and others added 4 commits March 17, 2020 10:04
Co-Authored-By: Jesse Hallam <[email protected]>
- Make err handling more idiomatic
- Rename exportCommandTrigger variable
@agarciamontoro
Copy link
Member Author

So I changed the structure here a bit. What I want is:

  1. that the user receives an ephemeral post as soon as they execute the command, letting the export and upload of the file run concurrently, as this process may take some time
  2. decouple the communication with the user and the export itself. I will work further on this in my next PR.

For now, the structure is as follows:

  • executeCommandExport is the owner of the communication with the user. It sets everything up (the channel to export and the channel to send the DM to), sending an ephemeral post in whether there's an error or the export process has begun successfully. The export process (namely, export the channel and upload the file) is triggered from a goroutine that still lives inside executeCommandExport, and that is in charge of informing the user of any error occurred during the export process, or to create the post with the successfully exported and uploaded file.
  • The exportChannel and uploadExportedChannelTo are the functions in charge of doing the real work. The former will absolutely be modified in the next PR, and will probably be something more complex than a simple function, in order to allow different exporters. The latter will probably remain similar, but I still need to investigate if it makes sense to design this so the exporting and the uploading is concurrent, so may change too.

Copy link
Member

@lieut-data lieut-data left a comment

Choose a reason for hiding this comment

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

Thanks, @agarciamontoro!

@agarciamontoro agarciamontoro added the 2: Dev Review Requires review by a core committer label Mar 23, 2020
Copy link

@alifarooq0 alifarooq0 left a comment

Choose a reason for hiding this comment

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

Nice work @agarciamontoro 🎉!

@agarciamontoro agarciamontoro changed the title Bootstrap channel export plugin and add a dummy exporter MM-23044: Bootstrap channel export plugin and add a dummy exporter Mar 25, 2020
@agarciamontoro agarciamontoro added 3: QA Review Requires review by a QA tester and removed 2: Dev Review Requires review by a core committer labels Mar 25, 2020
@agarciamontoro agarciamontoro requested a review from prapti March 25, 2020 09:19
@agarciamontoro
Copy link
Member Author

@prapti, I assigned you here to keep you in the loop, but I guess that a QA review at this early development phase may be redundant, as things are likely to change soon (in my next PR, for example). So feel free to mark it as done and we can outline a test plan to do when this is a bit stable.

@lieut-data lieut-data removed the 3: QA Review Requires review by a QA tester label Mar 26, 2020
@lieut-data lieut-data removed the request for review from prapti March 26, 2020 01:23
@lieut-data
Copy link
Member

Agreed, @agarciamontoro -- let's loop @prapti in when we're ready to cut a release.

@lieut-data lieut-data merged commit 091bb67 into master Mar 26, 2020
@lieut-data lieut-data deleted the MM-23044 branch March 26, 2020 01:23
@hanzei hanzei added the 4: Reviews Complete All reviewers have approved the pull request label Mar 26, 2020
@lieut-data lieut-data added this to the v0.1.0 milestone May 12, 2020
lieut-data added a commit that referenced this pull request Aug 5, 2020
* 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]>
lieut-data added a commit that referenced this pull request Aug 5, 2020
* 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]>
agarciamontoro added a commit that referenced this pull request Aug 11, 2020
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants