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

Redesign Daily Email #6471

Merged
merged 46 commits into from
May 27, 2020
Merged

Conversation

sebastiantf
Copy link
Contributor

@sebastiantf sebastiantf commented Apr 19, 2020

Description

This PR updates the Daily Email template according to the new layout provided here at #6011

Note: This PR is built on top of the commits made in #6438. This branch will update its base branch once #6438 is approved and merged.

Refers/Fixes

Fixes #6011

Screenshots

Gmail Web:

daily_footer_navy_web

Gmail Mobile:

daily_footer_navy_mobile

@owocki
Copy link
Contributor

owocki commented Apr 20, 2020

very pumped for this. i think its going to increase DAUs a lot.

putting it on the @gitcoinco/engineers review board

@thelostone-mc
Copy link
Member

@owocki! will bump this up once we get #6438
Just waiting on @willsputra / @PixelantDesign to check it out and approve it in

@thelostone-mc thelostone-mc requested review from frankchen07 and a team and removed request for a team April 21, 2020 00:10
@PixelantDesign
Copy link
Contributor

looks good!

@willsputra
Copy link
Contributor

looks good to me!

Copy link
Contributor

@frankchen07 frankchen07 left a comment

Choose a reason for hiding this comment

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

LGTM

@owocki
Copy link
Contributor

owocki commented Apr 22, 2020

Environment:


Request Method: GET
Request URL: http://localhost:8000/_administration/email/new_bounty_daily

Django Version: 2.2.4
Python Version: 3.6.9
Installed Applications:
['corsheaders',
 'django.contrib.admin',
 'taskapp.celery.CeleryConfig',
 'django_celery_beat',
 'django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.messages',
 'collectfast',
 'django.contrib.staticfiles',
 'cacheops',
 'storages',
 'social_django',
 'cookielaw',
 'django.contrib.humanize',
 'django.contrib.sitemaps',
 'django.contrib.sites',
 'autotranslate',
 'django_extensions',
 'easy_thumbnails',
 'health_check',
 'health_check.db',
 'health_check.cache',
 'health_check.storage',
 'health_check.contrib.psutil',
 'health_check.contrib.s3boto3_storage',
 'app',
 'avatar',
 'retail',
 'rest_framework',
 'marketing',
 'economy',
 'dashboard',
 'chat',
 'quests',
 'enssubdomain',
 'faucet',
 'tdi',
 'gas',
 'git',
 'healthcheck.apps.HealthcheckConfig',
 'legacy',
 'chartit',
 'email_obfuscator',
 'linkshortener',
 'credits',
 'gitcoinbot',
 'dataviz',
 'impersonate',
 'grants',
 'kudos',
 'django.contrib.postgres',
 'bounty_requests',
 'perftools',
 'revenue',
 'event_ethdenver2019',
 'inbox',
 'feeswapper',
 'search',
 'oauth2_provider',
 'townsquare',
 'compliance',
 'django_nyt.apps.DjangoNytConfig',
 'mptt',
 'sekizai',
 'sorl.thumbnail',
 'wiki.apps.WikiConfig',
 'wiki.plugins.attachments.apps.AttachmentsConfig',
 'wiki.plugins.notifications.apps.NotificationsConfig',
 'wiki.plugins.images.apps.ImagesConfig',
 'wiki.plugins.macros.apps.MacrosConfig',
 'debug_toolbar']
Installed Middleware:
['corsheaders.middleware.CorsMiddleware',
 'django.middleware.security.SecurityMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'app.middleware.drop_accept_langauge',
 'django.middleware.locale.LocaleMiddleware',
 'django.middleware.common.CommonMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'oauth2_provider.middleware.OAuth2TokenMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.clickjacking.XFrameOptionsMiddleware',
 'ratelimit.middleware.RatelimitMiddleware',
 'social_django.middleware.SocialAuthExceptionMiddleware',
 'impersonate.middleware.ImpersonateMiddleware']



Traceback:

File "/usr/local/lib/python3.6/dist-packages/django/core/handlers/exception.py" in inner
  34.             response = get_response(request)

File "/usr/local/lib/python3.6/dist-packages/django/core/handlers/base.py" in _get_response
  115.                 response = self.process_exception_by_middleware(e, request)

File "/usr/local/lib/python3.6/dist-packages/django/core/handlers/base.py" in _get_response
  113.                 response = wrapped_callback(request, *callback_args, **callback_kwargs)

File "/usr/local/lib/python3.6/dist-packages/django/contrib/auth/decorators.py" in _wrapped_view
  21.                 return view_func(request, *args, **kwargs)

File "/code/app/marketing/views.py" in new_bounty_daily_preview
  909.     response_html, _ = render_new_bounty('[email protected]', new_bounties, all_bounties, offset=3, trending_quests=quests)

Exception Type: TypeError at /_administration/email/new_bounty_daily
Exception Value: render_new_bounty() got an unexpected keyword argument 'trending_quests'

i get this exception when going to http://localhost:8000/_administration/email/new_bounty_daily

@owocki
Copy link
Contributor

owocki commented Apr 22, 2020

./manage.py new_bounties_emailm

how is this email trggered? this command doesnt seemto work and when i hack it to work i get the following

Traceback (most recent call last):
  File "/code/app/marketing/management/commands/new_bounties_email.py", line 82, in handle
    new_bounty_daily(new_bounties, all_bounties, [to_email])
  File "/code/app/marketing/mails.py", line 1152, in new_bounty_daily
    html, text = render_new_bounty(to_email, bounties, old_bounties)
  File "/code/app/retail/emails.py", line 518, in render_new_bounty
    'title': upcoming_grant.title,

i think this PR needs more testing

@owocki
Copy link
Contributor

owocki commented Apr 22, 2020

is there an easy way to test this in common mail clients

@sebastiantf
Copy link
Contributor Author

sebastiantf commented Apr 23, 2020

i get this exception when going to http://localhost:8000/_administration/email/new_bounty_daily

@owocki Please try going to http://localhost:8000/_administration/email/new_bounty

This is because I've only modified new_bounty() in emails.py (Line 1296 in the diff) which is executed at /_administration/email/new_bounty, and not new_bounty_daily_preview() in views.py which is executed at /_administration/email/new_bounty_daily

@sebastiantf
Copy link
Contributor Author

sebastiantf commented Apr 23, 2020

I am in need of some help while testing.

While testing the template by sending the email using ./manage.py new_bounties_email, it seems the stylesheets that are imported in shared_template_head.html are not actually getting imported. They get imported fine while testing locally at http://localhost:8000/_administration/email/new_bounty

I had changed the BASE_URL in settings.py from http://localhost:8000/ to http://gitcoin.co/ for testing the email. Is that the right way or am I missing something?

@sebastiantf
Copy link
Contributor Author

I think the external stylesheets that are loaded in shared_template_head.html are somehow not being loaded by premailer

@owocki
Copy link
Contributor

owocki commented Apr 23, 2020

might be worth checking how the existing daily email (on master) does this. that loads the stylesheets fine.

@owocki
Copy link
Contributor

owocki commented Apr 23, 2020

I had changed the BASE_URL in settings.py from http://localhost:8000/ to http://gitcoin.co/ for testing the email. Is that the right way or am I missing something?

u dont have to do that. at leaset i can test on my local without it

@sebastiantf
Copy link
Contributor Author

sebastiantf commented Apr 23, 2020

u dont have to do that. at leaset i can test on my local without it

Without changing the BASE_URL none of the images are being loaded when I test it via sendgrid.

@sebastiantf
Copy link
Contributor Author

sebastiantf commented Apr 23, 2020

might be worth checking how the existing daily email (on master) does this. that loads the stylesheets fine.

I couldn't find the daily email loading any external stylesheets. I do not think the daily email (or any of the emails) are loading external stylesheets. Please correct me if I'm wrong.

Since I've added the latest activities into the daily email, I've imported external stylesheets which are being used in the Townsquare page to render the activity boxes via the shared_template_head.html:

<link rel="stylesheet" href="{% static "v2/css/bootstrap.min.css" %}" crossorigin="anonymous" data-premailer="ignore">
<link rel="stylesheet" href="{% static "v2/css/lib/typography.css" %}" data-premailer="ignore">
<link rel="stylesheet" href="{% static "v2/css/fontawesome-all.min.css" %}" data-premailer="ignore">
<link rel="stylesheet" href="{% static "v2/css/gitcoin.css" %}" data-premailer="ignore">
<link rel="stylesheet" href="{% static "v2/css/base.css" %}" data-premailer="ignore">
<link rel="stylesheet" href="{% static "v2/css/town_square.css" %}" data-premailer="ignore">
<link rel="stylesheet" href="{% static "v2/css/activity_stream.css" %}" data-premailer="ignore">

These stylesheets are not being correctly loaded, at least not the way they're loaded when visiting http://localhost:8000/_administration/email/new_bounty. I think this has something to do with premailer.

@owocki
Copy link
Contributor

owocki commented Apr 23, 2020 via email

@sebastiantf
Copy link
Contributor Author

I'm reworking the PR because I'm seeing inconsistencies between the output I receive at http://localhost:8000/_administration/email/new_bounty and the actual email.

@thelostone-mc
Copy link
Member

@sebastiantf moving this onto a draft PR -> just hit the ready for review button when it's ready

@thelostone-mc thelostone-mc marked this pull request as draft April 28, 2020 06:33
@owocki
Copy link
Contributor

owocki commented Apr 28, 2020

@sebastiantf what all are you reworking? whats your ETA?

@sebastiantf
Copy link
Contributor Author

sebastiantf commented Apr 28, 2020

@owocki I figured most of the issues were due to the lack of support from email clients, like not supporting external stylesheets and negative margins. So I have been fixing those and the work is almost complete. Please see the current status:

Gmail Web:

[image moved to original comment]

Gmail for Mobile:

[image moved to original comment]

The only issue left is that the font awesome icons are not being loaded. I shall fix that soon.

I believe I could submit the final PR within 3-4 days. I have to look into the daily email PR too since this PR is based on that and I think most of its issues must be solved by this rework.

Sorry for the delay. I haven't been able to work full time for the last couple of days due to some other engagements.

@owocki
Copy link
Contributor

owocki commented Apr 28, 2020 via email

@sebastiantf sebastiantf force-pushed the redesign-daily-email branch from a7c535d to d0bdd37 Compare May 26, 2020 21:21
@sebastiantf
Copy link
Contributor Author

@sebastiantf the email still doesnt match the design in the original post. please make the background color and the header logo match the figma in the OP

https://bits.owocki.com/E0ubkqXR

Done in d0bdd37

@danlipert danlipert merged commit 7942a83 into gitcoinco:master May 27, 2020
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.

Update daily email build
8 participants