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

[GH-529] No attachment text when error happens while uploading attachment #535

Merged
merged 3 commits into from
Jun 15, 2020

Conversation

hectorgabucio
Copy link
Contributor

Summary

Attachment text not shown if error occurs while adding attachment.

Ticket Link

Fixes #529

@hectorgabucio hectorgabucio changed the title no attachment text when error happens while uploading attachment [GH-529] No attachment text when error happens while uploading attachment May 2, 2020
@hectorgabucio
Copy link
Contributor Author

I'm a contributor, hector2 but I changed the Github username 🙄

@hanzei hanzei added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels May 2, 2020
@hanzei
Copy link
Collaborator

hanzei commented May 2, 2020

/check-cla

@codecov-io
Copy link

codecov-io commented May 2, 2020

Codecov Report

Merging #535 into master will increase coverage by 1.38%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #535      +/-   ##
==========================================
+ Coverage   33.92%   35.30%   +1.38%     
==========================================
  Files          37       37              
  Lines        4604     4605       +1     
==========================================
+ Hits         1562     1626      +64     
+ Misses       2893     2825      -68     
- Partials      149      154       +5     
Impacted Files Coverage Δ
server/issue.go 25.78% <0.00%> (+9.40%) ⬆️
server/subscribe.go 65.94% <0.00%> (+0.39%) ⬆️
server/http.go 14.94% <0.00%> (+1.14%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d776dd3...a9477f7. Read the comment docs.

@jfrerich jfrerich requested a review from levb May 5, 2020 00:44
Copy link
Contributor

@jfrerich jfrerich left a comment

Choose a reason for hiding this comment

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

Thanks, @hectorgabucio! I left a couple of suggestions!

server/issue.go Outdated Show resolved Hide resolved
server/issue_test.go Show resolved Hide resolved
server/issue_test.go Outdated Show resolved Hide resolved
server/issue_test.go Outdated Show resolved Hide resolved
server/issue_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jfrerich jfrerich left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and quick changes, @hectorgabucio! 🚀

@levb levb requested a review from DHaussermann May 6, 2020 16:34
@levb levb removed the 2: Dev Review Requires review by a core committer label May 6, 2020
@levb levb added this to the v2.5 milestone May 29, 2020
Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

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

Tested and passed

  • Tested no permission to attach - no longer see!!
  • Tested size exceeded - no longer see^
    No issues found.

Thanks @hectorgabucio for this fix!

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Jun 9, 2020
@jfrerich jfrerich merged commit fff968f into mattermost:master Jun 15, 2020
levb added a commit that referenced this pull request Aug 10, 2020
* Add .npmrc file for save-exact packages (#586)

* [GH-529] No attachment text when error happens while uploading attachment (#535)

* [GH-527] Fix README TOC anchor linking (#580)

* Fixed the merge and the merged test to reflect multi-instance changes

* fixed concurrency issues in the test

Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>
Co-authored-by: Hector <hectordorito5@gmail.com>
Co-authored-by: Jason Frerich <jason.frerich@mattermost.com>
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.

Jira will post "Attachment: !!" in a comment when attachment fails
6 participants