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

Discord test button and send webhook msg with avatar #1514

Conversation

kelvintyb
Copy link
Contributor

@kelvintyb kelvintyb commented Jun 20, 2018

Description

As detailed in #1434, this PR adds test functionality to the Discord integration as well as setting of the webhook avatar when posting a message through the webhook.

Checklist
  • linter status: 100% pass
  • changes don't break existing behavior
  • commit message follows commit guidelines
Affected core subsystem(s)

Backend (marketing/utils, marketing/views, dashboard/notifications)
Frontend (settings/discord)

Testing

Tested:
screen recording 2018-06-24 at 10 49 pm

Refers/Fixes

Fixes: #1434

kelvintyb and others added 10 commits June 3, 2018 20:27
* Add Profile.get_discord_repos for presentation of discord_repos
* Fix splitting of repo string before saving into discord/slack repos
* Remove "Test" button in Discord settings as it is out of scope
* Remove mutation for Bounty.bounty_owner_profile in migration
* Add discord_repos and discord_webhook_url fields for profile fixtures
@kelvintyb
Copy link
Contributor Author

For some reason, my User no longer has its Profile created after logging in via the Github integration on my dev environment, so am stuck on that before I can test the new functionality. Will update again later this week when I have more time to troubleshoot. Unsure if the issue is with my changes, none of them touch the Github integration.

@kelvintyb
Copy link
Contributor Author

Updated the first post with screen recording of manual test. Ready for review @mbeacom @owocki

@kelvintyb kelvintyb changed the title [WIP] Discord test button and send webhook msg with avatar Discord test button and send webhook msg with avatar Jun 24, 2018
@mbeacom mbeacom mentioned this pull request Jul 3, 2018
3 tasks
@ghost ghost assigned mbeacom Jul 3, 2018
@ghost ghost added the in progress label Jul 3, 2018
@codecov
Copy link

codecov bot commented Jul 3, 2018

Codecov Report

Merging #1514 into master will increase coverage by 0.04%.
The diff coverage is 12.08%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1514      +/-   ##
==========================================
+ Coverage   29.69%   29.73%   +0.04%     
==========================================
  Files         132      130       -2     
  Lines        9784     9733      -51     
  Branches     1270     1260      -10     
==========================================
- Hits         2905     2894      -11     
+ Misses       6773     6735      -38     
+ Partials      106      104       -2
Impacted Files Coverage Δ
app/app/urls.py 89.74% <ø> (ø) ⬆️
app/dashboard/views.py 14.55% <0%> (-0.07%) ⬇️
app/dashboard/helpers.py 26.89% <0%> (-0.09%) ⬇️
app/marketing/utils.py 29.09% <10.52%> (-3.88%) ⬇️
app/dashboard/models.py 55.97% <28.57%> (-0.28%) ⬇️
app/marketing/views.py 15.4% <8.69%> (-0.85%) ⬇️
app/dashboard/notifications.py 16.29% <9.67%> (-0.56%) ⬇️
app/faucet/views.py 28.94% <0%> (-0.39%) ⬇️
app/dashboard/utils.py 26.12% <0%> (ø) ⬆️
... and 15 more

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 a15f818...573d488. Read the comment docs.

Copy link
Contributor

@owocki owocki left a comment

Choose a reason for hiding this comment

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

LGTM

@mbeacom mbeacom merged commit 244d3b3 into gitcoinco:master Jul 6, 2018
@ghost ghost removed the in progress label Jul 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bring Discord Bot to Parity with Slack Bot
3 participants