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.

Sign up for GitHub

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

Jump to bottom

Slack corporate import #11704

Merged
merged 7 commits into from
Jul 31, 2019
Merged

Slack corporate import #11704

merged 7 commits into from
Jul 31, 2019

Conversation

svelle
Copy link
Member

@svelle svelle commented Jul 25, 2019

I got my hands on a current export and was able to fix a lot of issues that we had in the current state.
It's almost 3am here and I've been working on this for nearly 12 hours straight so there might be some parts that could be improved :D So I'll mark this as a WIP for now.
I hope I can straighten any eventual issues out tomorrow.

Also yay to slack for inconsistent channel models in their import files :)

@svelle svelle mentioned this pull request Jul 25, 2019
Merged
@amyblais amyblais added this to the v5.14.0 milestone Jul 25, 2019
@svelle
Copy link
Member Author

svelle commented Jul 25, 2019

Customer got back to me and it seems to have worked with their dataset, including attachments and image previews.

@lieut-data lieut-data added 2: Dev Review Requires review by a developer 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review CherryPick/Candidate A candidate for a quality or patch release, but not yet approved labels Jul 26, 2019
lieut-data
lieut-data requested changes Jul 26, 2019
View reviewed changes
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, @svelle! Just a few nits.

app/slackimport.go Outdated Show resolved Hide resolved
app/slackimport.go Outdated Show resolved Hide resolved
app/slackimport.go Outdated Show resolved Hide resolved
app/slackimport.go Outdated Show resolved Hide resolved
app/slackimport.go Outdated Show resolved Hide resolved
app/slackimport.go Outdated Show resolved Hide resolved
app/slackimport.go Show resolved Hide resolved
@svelle
Copy link
Member Author

svelle commented Jul 26, 2019

Thanks for the review @lieut-data , I have updated everything accordingly.
Let me know if you find anything else! :)

lieut-data
lieut-data approved these changes Jul 26, 2019
View reviewed changes
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.

Looks good! Thanks, @svelle :)

grundleborg
grundleborg suggested changes Jul 29, 2019
View reviewed changes
Copy link
Contributor

@grundleborg grundleborg left a comment

Choose a reason for hiding this comment

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

Just two small comment fixes/clarifications, otherwise looks fine to me.

app/slackimport.go Outdated Show resolved Hide resolved
app/slackimport.go Outdated Show resolved Hide resolved
@svelle @grundleborg
Apply suggestions from code review
362c68c 
Co-Authored-By: George Goldberg <george@gberg.me>
@lindalumitchell lindalumitchell removed the 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review label Jul 29, 2019
@lindalumitchell
Copy link
Contributor

Removed QA label; QA testing not needed pre-merge.

@grundleborg grundleborg added 4: Reviews Complete All reviewers have approved the pull request CherryPick/Approved Meant for the quality or patch release tracked in the milestone and removed 2: Dev Review Requires review by a developer CherryPick/Candidate A candidate for a quality or patch release, but not yet approved labels Jul 30, 2019
@grundleborg
Copy link
Contributor

@svelle looks good now. Can you please merge master into this branch and also get the unit tests passing, and then we're good to merge.

@svelle
Copy link
Member Author

svelle commented Jul 31, 2019

@grundleborg should be good to merge now. :)

@grundleborg grundleborg merged commit 2d0dd17 into mattermost:master Jul 31, 2019
@mattermost-build mattermost-build mentioned this pull request Jul 31, 2019
Merged
@mattermod mattermod added AutomatedCherryPick and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Jul 31, 2019
grundleborg pushed a commit that referenced this pull request Jul 31, 2019
@mattermost-build @grundleborg
Automated cherry pick of #11704 (#11739)
8fa7824 
* changes for channel and post import for corp slack

* add fix for missing dms in import

* add suggestions from PR review

* Apply suggestions from code review

Co-Authored-By: George Goldberg <george@gberg.me>

* fix gofmt
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Aug 5, 2019
@svelle svelle mentioned this pull request Aug 6, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lieut-data lieut-data lieut-data approved these changes

@grundleborg grundleborg grundleborg approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request AutomatedCherryPick Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation
Projects
None yet
Milestone
v5.14.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@svelle @lindalumitchell @grundleborg @lieut-data @mattermod @amyblais