Send email alert on low priced bounties #3174

danlipert · 2018-12-11T06:46:22Z

Description

This PR adds an alert email to the Gitcoin admins when a low-priced bounty is created. A LOW_BOUNTY_THRESHOLD is set, and bounties created with an ~~amount~~ USDT value lower than this threshold trigger the alert.

Checklist

Read and conforms to the contributor guidelines
Read and conforms to the style guidelines
Linter status: 100% pass
Added relevant tests
Tests pass and code coverage has not decreased
Changes don't break existing behavior
Commit message follows commit guidelines
Code documentation
Completed: Code review by core team and any requested changes

Affected core subsystem(s)

Mails
~~Bounty Requests~~
Dashboard models

~~Note: idna 2.7 was pinned in the requirements to solve a requirements conflict between the newly released idna 2.8 and the requests library, which requires idna<=2.7.~~

Refers/Fixes

Refs: #2808

Testing and Sign-off

Two functional tests were added as part of this PR, one to check that the current behavior has not changed, and one that triggers the alert email and verifies that the additional alert email is sent.

Contributor

Read and followed the Contributor Guidelines
Tested all changes locally
Verified existing functionality
Ran make test and everything passed!

Reviewer

Affirm contributor guidelines have been followed and requested changes made
CI tests and linting pass
No conflicts (migrations, files, etc)
Regression tested against staging

Funder

Validated requested changes were made to specification
Bounty payout released to the contributor

stickler-ci · 2018-12-11T06:46:34Z

app/bounty_requests/tests/test_bounty_requests.py

+along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+"""
+import json


F401 'json' imported but unused

stickler-ci · 2018-12-11T06:46:34Z

app/bounty_requests/tests/test_bounty_requests.py

+
+from django.utils import timezone
+
+from bounty_requests.views import LOW_BOUNTY_THRESHOLD, bounty_request


F401 'bounty_requests.views.bounty_request' imported but unused

stickler-ci · 2018-12-11T06:46:35Z

app/marketing/mails.py

@@ -895,6 +895,27 @@ def new_bounty_request(model):
    finally:
        translation.activate(cur_language)

+def low_bounty_request(bounty):


E302 expected 2 blank lines, found 1

codecov · 2018-12-11T07:21:44Z

Codecov Report

Merging #3174 into master will increase coverage by 0.32%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3174      +/-   ##
==========================================
+ Coverage   30.11%   30.44%   +0.32%     
==========================================
  Files         193      193              
  Lines       14518    14532      +14     
  Branches     1900     1901       +1     
==========================================
+ Hits         4372     4424      +52     
+ Misses      10008     9966      -42     
- Partials      138      142       +4

Impacted Files	Coverage Δ
app/bounty_requests/views.py	`61.9% <100%> (+28.57%)`	⬆️
app/marketing/mails.py	`14.93% <100%> (+3.42%)`	⬆️
app/dashboard/embed.py	`28.16% <0%> (-3.45%)`	⬇️
app/dashboard/utils.py	`36.09% <0%> (+0.53%)`	⬆️
app/dashboard/models.py	`53.55% <0%> (+0.71%)`	⬆️
app/bounty_requests/forms.py	`100% <0%> (+48%)`	⬆️

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 3628c64...0ee3452. Read the comment docs.

codecov · 2018-12-11T07:21:44Z

Codecov Report

Merging #3174 into master will increase coverage by 0.11%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3174      +/-   ##
==========================================
+ Coverage   30.11%   30.23%   +0.11%     
==========================================
  Files         193      193              
  Lines       14577    14537      -40     
  Branches     1906     1901       -5     
==========================================
+ Hits         4390     4395       +5     
+ Misses      10049    10004      -45     
  Partials      138      138

Impacted Files	Coverage Δ
app/marketing/mails.py	`13.54% <100%> (+2.03%)`	⬆️
app/dashboard/models.py	`53.29% <100%> (+0.28%)`	⬆️
app/app/urls.py	`88.23% <0%> (-1.97%)`	⬇️
app/grants/views.py	`15.6% <0%> (-0.43%)`	⬇️
app/dashboard/admin.py	`69.72% <0%> (-0.28%)`	⬇️
app/grants/management/commands/subminer.py	`0% <0%> (ø)`	⬆️
app/grants/models.py	`60.58% <0%> (+0.03%)`	⬆️
app/retail/emails.py	`20.48% <0%> (+0.04%)`	⬆️
app/dashboard/views.py	`12.33% <0%> (+0.05%)`	⬆️
app/app/utils.py	`22.31% <0%> (+0.13%)`	⬆️
... and 4 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 91e91cf...9d19606. Read the comment docs.

mbeacom

Hey @danlipert - Thanks for the contribution!
Would it be possible for you to update this PR to send the email when any bounty is created that doesn't reach or exceed the minimum value? It appears this is setup to only check BountyRequest versus Bounty.

You could probably get away with simply adding a post_save signal for Bounty that checks if created and send out the email in the event the minimum isn't met.

@frankchen07 Just for clarity, it appears to me that #2808 is requesting an email notification if any bounty is created that doesn't reach or exceed the minimum. This isn't for bounty requests, right?

danlipert · 2018-12-12T16:46:33Z

@mbeacom My bad! I just realized this is the wrong model! I see the actual bounties in the dashboard app now - will get that change done as soon ASAP.

frankchen07 · 2018-12-12T23:19:28Z

Just for clarity, it appears to me that #2808 is requesting an email notification if any bounty is created that doesn't reach or exceed the minimum.

Correct.

This isn't for bounty requests, right?

Nope, not for bounty requests.

stickler-ci · 2018-12-13T06:57:20Z

app/dashboard/models.py

+    """Alert when a bounty with a low price is created."""
+    bounty = kwargs.get('instance')
+    usdt_value = bounty.get_value_in_usdt_now
+    if(kwargs.get('created') == True and usdt_value and usdt_value < LOW_BOUNTY_THRESHOLD):


E712 comparison to True should be 'if cond is True:' or 'if cond:'

stickler-ci · 2018-12-13T06:57:21Z

app/dashboard/tests/test_dashboard_models.py

+    @patch('marketing.mails.send_mail')
+    def test_low_bounty_alert(self, mock_send_mail):
+        """Test that an alert email is sent when a bounty with a low value is created"""
+        bounty = Bounty.objects.create(


F841 local variable 'bounty' is assigned to but never used

stickler-ci · 2018-12-13T06:57:21Z

app/dashboard/tests/test_dashboard_models.py

+    @patch('marketing.mails.send_mail')
+    def test_no_bounty_alert_for_reasonable_bounties(self, mock_send_mail):
+        """Test that no alert email is sent when a bounty with a reasonable price is created."""
+        bounty = Bounty.objects.create(


F841 local variable 'bounty' is assigned to but never used

danlipert · 2018-12-13T07:26:44Z

@mbeacom @frankchen07 Apologies for the confusion here - I've updated this PR with code that targets the correct model and uses the post_save signal to trigger the email. Please check it out and let me know what you think - thanks!

danlipert · 2019-06-25T13:47:41Z

stale

stickler-ci reviewed Dec 11, 2018

View reviewed changes

danlipert force-pushed the low-bounty-alert branch from a814ab4 to 0ee3452 Compare December 11, 2018 07:21

mbeacom requested review from a team December 11, 2018 19:24

mbeacom assigned mbeacom and unassigned mbeacom Dec 11, 2018

mbeacom added backend This needs backend expertise. Gitcoin Emails Gitcoin Emails Gitcoin Bounties Gitcoin Bounties labels Dec 11, 2018

mbeacom suggested changes Dec 12, 2018

View reviewed changes

mbeacom added the waiting on contributor label Dec 12, 2018

mbeacom changed the title ~~Send email alert on low priced bounties~~ [WOC] Send email alert on low priced bounties Dec 12, 2018

danlipert force-pushed the low-bounty-alert branch 2 times, most recently from 0ee3452 to c210ef7 Compare December 13, 2018 06:57

stickler-ci reviewed Dec 13, 2018

View reviewed changes

send alert for low priced bounty creation

f2644fc

danlipert force-pushed the low-bounty-alert branch from c210ef7 to f2644fc Compare December 13, 2018 07:15

Merge branch 'master' into low-bounty-alert

fb13048

mbeacom changed the title ~~[WOC] Send email alert on low priced bounties~~ Send email alert on low priced bounties Dec 14, 2018

mbeacom added needs review and removed waiting on contributor labels Dec 14, 2018

mbeacom requested a review from a team December 14, 2018 20:09

thelostone-mc approved these changes Dec 21, 2018

View reviewed changes

Merge branch 'master' into low-bounty-alert

9d19606

danlipert closed this Jun 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Send email alert on low priced bounties #3174

Send email alert on low priced bounties #3174

danlipert commented Dec 11, 2018 •

edited

Loading

stickler-ci Dec 11, 2018

stickler-ci Dec 11, 2018

stickler-ci Dec 11, 2018

codecov bot commented Dec 11, 2018

codecov bot commented Dec 11, 2018 •

edited

Loading

mbeacom left a comment

danlipert commented Dec 12, 2018

frankchen07 commented Dec 12, 2018

stickler-ci Dec 13, 2018

stickler-ci Dec 13, 2018

stickler-ci Dec 13, 2018

danlipert commented Dec 13, 2018

danlipert commented Jun 25, 2019


		from django.utils import timezone

		from bounty_requests.views import LOW_BOUNTY_THRESHOLD, bounty_request

Send email alert on low priced bounties #3174

Send email alert on low priced bounties #3174

Conversation

danlipert commented Dec 11, 2018 • edited Loading

Description

Checklist

Affected core subsystem(s)

Refers/Fixes

Testing and Sign-off

Contributor

Reviewer

Funder

stickler-ci Dec 11, 2018

Choose a reason for hiding this comment

stickler-ci Dec 11, 2018

Choose a reason for hiding this comment

stickler-ci Dec 11, 2018

Choose a reason for hiding this comment

codecov bot commented Dec 11, 2018

Codecov Report

codecov bot commented Dec 11, 2018 • edited Loading

Codecov Report

mbeacom left a comment

Choose a reason for hiding this comment

danlipert commented Dec 12, 2018

frankchen07 commented Dec 12, 2018

stickler-ci Dec 13, 2018

Choose a reason for hiding this comment

stickler-ci Dec 13, 2018

Choose a reason for hiding this comment

stickler-ci Dec 13, 2018

Choose a reason for hiding this comment

danlipert commented Dec 13, 2018

danlipert commented Jun 25, 2019

danlipert commented Dec 11, 2018 •

edited

Loading

codecov bot commented Dec 11, 2018 •

edited

Loading