Skip to content

Add sent, open & click tracking#40

Merged
codingjoe merged 19 commits into
mainfrom
tracking
Jul 20, 2023
Merged

Add sent, open & click tracking#40
codingjoe merged 19 commits into
mainfrom
tracking

Conversation

@codingjoe
Copy link
Copy Markdown
Collaborator

@codingjoe codingjoe commented Jun 25, 2023

Watching I See You GIF

@codingjoe codingjoe self-assigned this Jun 25, 2023
@codingjoe codingjoe requested a review from amureki June 25, 2023 13:22
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 25, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (8f2168d) 100.00% compared to head (6b1b61f) 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##              main       #40    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            4         9     +5     
  Lines          172       313   +141     
==========================================
+ Hits           172       313   +141     
Impacted Files Coverage Δ
emark/conf.py 100.00% <ø> (ø)
emark/backends.py 100.00% <100.00%> (ø)
emark/message.py 100.00% <100.00%> (ø)
emark/migrations/0001_initial.py 100.00% <100.00%> (ø)
emark/models.py 100.00% <100.00%> (ø)
emark/urls.py 100.00% <100.00%> (ø)
emark/utils.py 100.00% <100.00%> (ø)
emark/views.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Copy Markdown
Member

@amureki amureki left a comment

Choose a reason for hiding this comment

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

Awesome feature!

I have first comments here, but I still need to do the testing part.

Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread emark/models.py Outdated
Comment thread emark/message.py Outdated
Copy link
Copy Markdown
Member

@amureki amureki left a comment

Choose a reason for hiding this comment

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

Also, as you mentioned - we need to think about rate limiting.

codingjoe and others added 2 commits July 6, 2023 10:15
Co-authored-by: Rust Saiargaliev <fly.amureki@gmail.com>
@codingjoe codingjoe requested a review from amureki July 6, 2023 15:27
Co-authored-by: Rust Saiargaliev <fly.amureki@gmail.com>
Copy link
Copy Markdown
Member

@amureki amureki left a comment

Choose a reason for hiding this comment

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

Shall we do a beta release first?

Copy link
Copy Markdown
Member

@amureki amureki left a comment

Choose a reason for hiding this comment

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

I did some tests, found a couple of things. Also, tracking is questionable, we'd need to test how it works in comparison to the mailing systems like Brevo, as here are the results from my test:

  • checking an email via gmail (no ad-blockers) in browser does not count as opening
  • gmail on iOS also does not count at all
  • Microsoft Windows 10 Mail does not count at all
  • MacOS Mail.app open counts as 1 open (no matter how many times you will open it)

Comment thread emark/message.py
self.language = language
utm_params = utm_params or {}
self.utm_params = utm_params or {}
self.subject = subject or self.subject
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When called via to_user, like:

LoginEmail.to_user(user, context).send()

... the subject is being omitted completely, so I am getting None there instead of the one defined in the class.

Comment thread emark/backends.py Outdated
Comment thread emark/backends.py Outdated
codingjoe and others added 2 commits July 19, 2023 15:58
Co-authored-by: Rust Saiargaliev <fly.amureki@gmail.com>
@codingjoe codingjoe force-pushed the tracking branch 3 times, most recently from 7cce520 to 2c246f1 Compare July 19, 2023 16:01
Copy link
Copy Markdown
Member

@amureki amureki left a comment

Choose a reason for hiding this comment

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

flake8 complained about something

Comment thread emark/views.py Outdated
Comment thread emark/templates/emark/base.html Outdated
Comment on lines +11 to +15
{% if tracking_pixel_url %}
.main {
background-image: url("{{ tracking_pixel_url }}");
}
{% endif %}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Something is odd here with gmail desktop - tracking does not work there.

I wonder if premailer does not handle this style setting correctly.
CleanShot 2023-07-20 at 10 25 03@2x
Shall we try just adding a pixel to html directly?

Tracking in mail.app and gmail mobile works.

Comment thread emark/models.py Outdated
Copy link
Copy Markdown
Member

@amureki amureki left a comment

Choose a reason for hiding this comment

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

@codingjoe switching to <img> worked, opens are tracked in gmail desktop, as well as in the other clients.

So, from my side this is good to go now!

@codingjoe codingjoe enabled auto-merge (squash) July 20, 2023 14:31
@codingjoe codingjoe disabled auto-merge July 20, 2023 14:31
@codingjoe codingjoe merged commit 27cf7b0 into main Jul 20, 2023
@codingjoe codingjoe deleted the tracking branch July 20, 2023 14:31
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.

2 participants