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

Update BlockAttachment to not send invalid JSON due to fields attribute #473

Merged
merged 1 commit into from Aug 29, 2019

Conversation

paul-griffith
Copy link
Contributor

Summary

Fixes a bug in #400; the base Attachment class that BlockAttachment inherits from will populate the fields attribute with an empty list, which then gets sent in the JSON to Slack. That makes the JSON invalid, meaning the BlockAttachment class as it is now cannot be used without manually adjusting the JSON/dict returned.

This PR updates the class, as well as adds a test that would have caught this.

Requirements (place an x in each [ ])

@paul-griffith
Copy link
Contributor Author

I started using 2.1.0 internally and immediately ran into this 🤦‍♂️

@codecov
Copy link

codecov bot commented Jul 2, 2019

Codecov Report

Merging #473 into master will increase coverage by 0.35%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #473      +/-   ##
==========================================
+ Coverage   69.58%   69.94%   +0.35%     
==========================================
  Files          15       15              
  Lines        1654     1657       +3     
  Branches       91       91              
==========================================
+ Hits         1151     1159       +8     
+ Misses        481      476       -5     
  Partials       22       22
Impacted Files Coverage Δ
slack/web/classes/attachments.py 98.8% <100%> (+6.21%) ⬆️

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 df5351f...decad0e. Read the comment docs.

@RodneyU215
Copy link
Contributor

Thanks for your diligence Paul!

@RodneyU215 RodneyU215 added Priority: High bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented Version: 2x labels Aug 29, 2019
@RodneyU215 RodneyU215 merged commit aabf2d0 into slackapi:master Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug M-T: A confirmed bug report. Issues are confirmed when the reproduction steps are documented Version: 2x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants