New Grants txn Failed Email #6791

sebastiantf · 2020-06-06T14:38:53Z

Description

This PR builds a new email to notify Grants contributors if their contribution txns have failed

Refers/Fixes

Fixes #6556

Testing

Gmail Web:

Gmail Mobile:

Web Preview:

codecov · 2020-06-06T14:45:02Z

Codecov Report

Merging #6791 into master will decrease coverage by 0.44%.
The diff coverage is 7.89%.

@@            Coverage Diff             @@
##           master    #6791      +/-   ##
==========================================
- Coverage   26.92%   26.48%   -0.45%     
==========================================
  Files         295      299       +4     
  Lines       28279    28829     +550     
  Branches     4146     4226      +80     
==========================================
+ Hits         7615     7636      +21     
- Misses      20390    20922     +532     
+ Partials      274      271       -3

Impacted Files	Coverage Δ
app/app/urls.py	`86.20% <ø> (ø)`
...oard/management/commands/grant_txn_failed_email.py	`0.00% <0.00%> (ø)`
app/marketing/mails.py	`10.94% <6.66%> (-0.06%)`	⬇️
app/retail/emails.py	`21.36% <20.00%> (-0.02%)`	⬇️
app/grants/clr.py	`0.00% <0.00%> (-18.05%)`	⬇️
app/dashboard/tasks.py	`33.33% <0.00%> (-3.16%)`	⬇️
app/economy/tx.py	`17.70% <0.00%> (-2.30%)`	⬇️
app/grants/models.py	`45.67% <0.00%> (-1.97%)`	⬇️
app/dashboard/admin.py	`64.73% <0.00%> (-1.36%)`	⬇️
app/grants/admin.py	`42.93% <0.00%> (-0.78%)`	⬇️
... and 23 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 6b76d0f...e444c77. Read the comment docs.

frankchen07 · 2020-06-08T21:00:50Z

design looks great.

what are the specific cases where someone would get sent this email?

gas too low, txn pending forever
not enough gas, txn fails
what else?

owocki · 2020-06-08T21:14:00Z

gas too low, txn pending forever
not enough gas, txn fails
not enough tokens, txn fails
some other contract error (this seems to happen a lot with USDC)
other

owocki · 2020-06-08T22:42:28Z

@frankchen07 i take it u think we should highlight why the contribs failed and recommend a remeditation step?

for pending tx the remeditation step is diff than the 'failed' step

frankchen07 · 2020-06-15T06:15:45Z

sorry, missed this, yes I think having a more detailed explanation might be good, but it'll depend on how complicated it is to pull that information. usually I just go to etherscan and its a quick google to figure things out.

owocki · 2020-06-16T22:24:03Z

ahh merge conflicts... poo. @sebastiantf can you fix!??

sebastiantf · 2020-06-17T06:11:51Z

@owocki I have rebased and fixed merge conflicts

owocki · 2020-06-22T18:08:59Z

@danlipert @thelostone-mc can this get into the next release pls? 🙏

thelostone-mc · 2020-06-23T06:28:56Z

app/retail/templates/emails/template.html

+    .grant-txn-msg {
+      width: 60%;
+    }
+  }


^ could we move this to the needed email files!
Else it would be imported by every mail

@thelostone-mc Gmail seems to have an issue while using media queries in emails. It only considers the first set of media queries it sees and ignores media queries seen anywhere else afterwards. I found this behaviour while working on the daily email redesign and also found on the Internet that others have experienced this behaviour too.

The workaround is to include all the media queries in the first place (first <style> tag) and that would be the one in template.html. This is why the media queries and the corresponding styles are all placed in template.html itself.

Okay that makes sense !
But if by placing it here, wouldn't every other email which has media query styles not be applied (not sure if there are other mails which get affected by this )

If we are doing this, we should collect it from all emails and add that in

Could we move these though into the grant specific mail ?

.etherscan-link { display: inline-block; font-family: 'Muli', sans-serif; color: #3E00FF; } .grant-txn-msg, .grant-txn-id { margin: 0 auto; } .grant-txn-id a { overflow-wrap: break-word; }

Could we move these though into the grant specific mail ?

.etherscan-link { display: inline-block; font-family: 'Muli', sans-serif; color: #3E00FF; } .grant-txn-msg, .grant-txn-id { margin: 0 auto; } .grant-txn-id a { overflow-wrap: break-word; }

@thelostone-mc Done in e444c77

Okay that makes sense !
But if by placing it here, wouldn't every other email which has media query styles not be applied (not sure if there are other mails which get affected by this )

If we are doing this, we should collect it from all emails and add that in

I do doubt that the media query styles present inside specific email templates are actually being rendered correctly by Gmail. That may have to be tested and taken care of. But the commits in this PR wont affect the other emails.

thelostone-mc · 2020-06-23T06:29:41Z

app/retail/templates/emails/grant_txn_failed.html

+</div>
+
+<div style="margin-bottom: 3em; margin-top: 3em;">
+  <a class="button" href="#">{% trans "Resubmit Transaction" %}</a> <!-- TODO: Replace correct href to add the failed grant to cart -->


This needs to be fixed , the least we can do is link them to the grant so that they can add it themselves

@owocki any suggestions? Maybe use the bulk-add link?

yeah this needs to be updated before we deploy I think - @sebastiantf I think the bulk add link would be perfect

@danlipert @thelostone-mc Done in 902f548

thelostone-mc

Left a few comments

danlipert · 2020-06-23T08:38:59Z

app/retail/emails.py

+    params = {
+        'grant_title': grant.title,
+        'tx_id': tx_id,
+        'tx_url': "https://etherscan.io/tx/"+tx_id,


Note for the future: we'll have to update this to use other block explorers once we support other chains

danlipert · 2020-06-23T08:39:48Z

app/retail/templates/emails/grant_txn_failed.html

+</div>
+
+<div style="margin-bottom: 3em; margin-top: 3em;">
+  <a class="button" href="#">{% trans "Resubmit Transaction" %}</a> <!-- TODO: Replace correct href to add the failed grant to cart -->


yeah this needs to be updated before we deploy I think - @sebastiantf I think the bulk add link would be perfect

owocki · 2020-06-23T13:51:02Z

@sebastiantf can you revert the engineers feedback? i want to get this in ASAP since theres an ongoing grants round right now

owocki · 2020-06-23T14:00:19Z

sebastiantf · 2020-06-23T16:19:57Z

@sebastiantf can you revert the engineers feedback? i want to get this in ASAP since theres an ongoing grants round right now

@owocki Sorry I'm late. But I'm not sure I understand what you meant by 'engineers feedback'. Would you mind explaining?

owocki · 2020-06-23T17:06:28Z

no worries its solved now. feel free to hit me up on gitcoin.co/requestmoney

…

On Tue, Jun 23, 2020 at 10:20 AM Sebastian T F ***@***.***> wrote: @sebastiantf <https://github.com/sebastiantf> can you revert the engineers feedback? i want to get this in ASAP since theres an ongoing grants round right now @owocki <https://github.com/owocki> Sorry I'm late. But I'm not sure I understand what you meant by *'engineers feedback'*. Would you mind explaining? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6791 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAD5PCKAEYQLKYX56C3DWVLRYDI3ZANCNFSM4NWBVDBQ> .

-- @owocki <http://www.twitter.com/owocki>

_________________________________________ gitcoin is live and has generated over $4.6mm for Open Source Software - see our results <https://gitcoin.co/results>

* Fix Firefox issue that showed red border for decimal inputs in cart (#6958) * New Grants txn Failed Email (#6791) * setup web preview url * implement render_grant_txn_failed * build template grant_txn_failed.html * management command for grant_txn_failed_email * fix missing returns after resolving conflicts * add bulk_add_url * move email specific style to grant_txn_failed.html * new approval checker helper script * fix comparing strings on allowance * fix featured bounty and balance checker * fix eslint card.js * Improve cart gas estimates for DAI, aDAI, cDAI, and ANT (#6961) * Quotation on Profile handle (#6957) * Improve cart gas estimates for DAI, aDAI, cDAI, and ANT Co-authored-by: Paul <[email protected]> * wallet improvments (#6952) * wallet improvments * avoid form submission * bounty requesting wallet * quest js dupe * Fix for bottomVisible method scope. (#6964) * fix for projects, the bottomVisible method was defined incorrectly * fixes for paginaiton of projects, as well as sorting by winners as default * restoring docker-compose * Update manifest.json reverting manifest changes * Update top_nav.html revert top nav changes * position the loading screen better inside the project directory * formalizes the types of conversion rates in the DB * 1 thing * fixes A few annoying things about kudso 1. now proudly shows the kudos artist 2. allows u to mind them in one fell swoop + change onwnership 3. stores the IPFS metadata in our DB for future keeping Co-authored-by: Matt <[email protected]> Co-authored-by: Sebastian T F <[email protected]> Co-authored-by: Owocki <[email protected]> Co-authored-by: Paul <[email protected]> Co-authored-by: Andrew Redden <[email protected]>

sebastiantf requested review from danlipert, frankchen07, octavioamu, PixelantDesign and thelostone-mc as code owners June 6, 2020 14:38

sebastiantf changed the title ~~Grants txn failed email~~ New Grants txn Failed Email Jun 6, 2020

sebastiantf mentioned this pull request Jun 6, 2020

As a user who created a grant transaction that failed or is pending forever, I want a notification so I can resubmit it. #6556

Closed

PixelantDesign approved these changes Jun 9, 2020

View reviewed changes

sebastiantf added 5 commits June 17, 2020 11:07

setup web preview url

d7c4554

implement render_grant_txn_failed

8611632

build template grant_txn_failed.html

ec5a0e5

management command for grant_txn_failed_email

9486ad9

fix missing returns after resolving conflicts

2f60d92

sebastiantf force-pushed the grants-txn-failed-email branch from 20b2b76 to 2f60d92 Compare June 17, 2020 06:10

thelostone-mc reviewed Jun 23, 2020

View reviewed changes

thelostone-mc requested changes Jun 23, 2020

View reviewed changes

danlipert reviewed Jun 23, 2020

View reviewed changes

danlipert suggested changes Jun 23, 2020

View reviewed changes

danlipert marked this pull request as draft June 23, 2020 08:43

sebastiantf added 2 commits June 23, 2020 17:37

add bulk_add_url

902f548

move email specific style to grant_txn_failed.html

e444c77

sebastiantf marked this pull request as ready for review June 23, 2020 12:20

thelostone-mc approved these changes Jun 23, 2020

View reviewed changes

thelostone-mc changed the base branch from master to stable June 23, 2020 13:56

octavioamu approved these changes Jun 23, 2020

View reviewed changes

thelostone-mc merged commit 997673e into gitcoinco:stable Jun 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Grants txn Failed Email #6791

New Grants txn Failed Email #6791

sebastiantf commented Jun 6, 2020

codecov bot commented Jun 6, 2020 •

edited

Loading

frankchen07 commented Jun 8, 2020

owocki commented Jun 8, 2020 •

edited

Loading

owocki commented Jun 8, 2020

frankchen07 commented Jun 15, 2020

owocki commented Jun 16, 2020

sebastiantf commented Jun 17, 2020

owocki commented Jun 22, 2020

thelostone-mc Jun 23, 2020

sebastiantf Jun 23, 2020

thelostone-mc Jun 23, 2020

thelostone-mc Jun 23, 2020

sebastiantf Jun 23, 2020

sebastiantf Jun 23, 2020

thelostone-mc Jun 23, 2020

sebastiantf Jun 23, 2020

danlipert Jun 23, 2020

sebastiantf Jun 23, 2020

thelostone-mc left a comment

danlipert Jun 23, 2020

danlipert Jun 23, 2020

owocki commented Jun 23, 2020

owocki commented Jun 23, 2020

sebastiantf commented Jun 23, 2020

owocki commented Jun 23, 2020 via email

New Grants txn Failed Email #6791

New Grants txn Failed Email #6791

Conversation

sebastiantf commented Jun 6, 2020

Description

Refers/Fixes

Testing

codecov bot commented Jun 6, 2020 • edited Loading

Codecov Report

frankchen07 commented Jun 8, 2020

owocki commented Jun 8, 2020 • edited Loading

owocki commented Jun 8, 2020

frankchen07 commented Jun 15, 2020

owocki commented Jun 16, 2020

sebastiantf commented Jun 17, 2020

owocki commented Jun 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thelostone-mc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owocki commented Jun 23, 2020

owocki commented Jun 23, 2020

sebastiantf commented Jun 23, 2020

owocki commented Jun 23, 2020 via email

codecov bot commented Jun 6, 2020 •

edited

Loading

owocki commented Jun 8, 2020 •

edited

Loading