Feature/exclude user from drip #171

jumcorredorro · 2022-08-04T19:54:29Z

Description

Add the feature for allowing the user to be excluded from drip emails.
The library user should config DRIP_UNSUBSCRIBE_USERS = True and add drip.urls to the project path.
This configurations will enable a context variable in Body html template field in form as the following

This will build a url hashing user and drip data that will be valid forever. Once you open the link it will show you a dumb HTML like this, if the hashes are valid:

If the hashes are invalid it will show you the following:

Once you click the button it will show you a success page.

The library user could add the drip.urls or extend the UnsubscribeDripView and change the template names for the ones that matches the style of the user page:

template_name = "unsubscribe_drip.html"
invalid_template_name = "unsubscribe_drip_invalid.html"
success_template_name = "unsubscribe_drip_success.html"

For building this feature there is a new ManyToManyField unsubscribed_users and new functions for builiding context for the message and excluding this new model in querysets, based on DRIP_UNSUBSCRIBE_USERS.

Changes include

Bugfix (non-breaking change that solves an issue, bump the third digit of the version)
New feature (non-breaking change that adds functionality, bump the second digit of the version)
Breaking change (change that is not backwards-compatible and/or changes current functionality, bump the first digit of the version)
Documentation update (Don't bump the version of the project)

Other comments

…ews!

brunomichetti · 2022-08-08T17:58:12Z

drip/tokens.py

+            user = User.objects.get(pk=uid)
+        except (TypeError, ValueError, OverflowError, User.DoesNotExist):
+            user = None
+        if user is None or not custom_token_generator.check_token(user, token):


you don't need (user is None) here because if it's None it will return the user at the end

brunomichetti

Great job!!!!
Approved with comments

brunomichetti · 2022-08-09T13:38:57Z

drip/tokens.py

+        Build hash value ignoring login_timestamp and password for keeping it valid over time
+        """
+        email_field = user.get_email_field_name()
+        email = getattr(user, email_field, "") or ""


I'm asking because I'm not sure: do you need the or ""?

I copy this code directly from Django source code but I remove the time check. I will leave it as it is right now.

brunomichetti · 2022-08-09T13:40:35Z

drip/tokens.py

+        email = getattr(user, email_field, "") or ""
+        return f"{user.pk}{timestamp}{email}"
+
+    def check_token(self, user: Optional[AbstractBaseUser], token: Optional[str]) -> bool:


just a suggestion: I would change the name by something like token_is_correct just to be more easy to read in the place you use. But is a minor suggestion you can ignore it if you want

I only override already existing class methods in PasswordResetTokenGenerator class.

brunomichetti · 2022-08-09T13:43:22Z

drip/urls.py

+if getattr(settings, "DRIP_UNSUBSCRIBE_USERS", False):
+    urlpatterns += [
+        re_path(
+            r"^drip/(?P<drip_uidb64>\w+)/(?P<uidb64>\w+)/(?P<token>[\w-]+)/$",


just a suggestion:
you can use slug here as in this example
ignore this comment if you want

brunomichetti · 2022-08-09T13:45:07Z

drip/utils.py

+    unsubscribe_link = None
+    try:
+        unsubscribe_link = reverse(path, kwargs=url_args)
+    except NoReverseMatch:
+        return unsubscribe_link
+    return unsubscribe_link


Suggested change

unsubscribe_link = None

try:

unsubscribe_link = reverse(path, kwargs=url_args)

except NoReverseMatch:

return unsubscribe_link

return unsubscribe_link

try:

unsubscribe_link = reverse(path, kwargs=url_args)

except NoReverseMatch:

return None

return unsubscribe_link

Ignore this if you want

I apply this suggestion. Thanks!

brunomichetti · 2022-08-09T13:48:26Z

drip/views.py

+    def _set_user_and_drip(self, **kwargs):
+        drip_uidb64 = kwargs.get("drip_uidb64", "")
+        uidb64 = kwargs.get("uidb64", "")
+        token = kwargs.get("token", "")
+        self.user = EmailToken.validate_user_uidb64_token(uidb64, token)
+        self.drip = EmailToken.validate_drip_uidb64(drip_uidb64)
+        self.post_sucess = False
+
+    def dispatch(self, *args, **kwargs):


You have to set Any type for the args and kwargs right?

I apply this suggestion. Thanks!

brunomichetti · 2022-08-09T13:48:43Z

drip/views.py

+    def _set_user_and_drip(self, **kwargs):
+        drip_uidb64 = kwargs.get("drip_uidb64", "")
+        uidb64 = kwargs.get("uidb64", "")
+        token = kwargs.get("token", "")


What happens if these values are ""?

It will go to invalid template, but a URL with /// no data between slashes will break, so I think it is impossible to reach this case.

jumcorredorro force-pushed the feature/exclude-user-from-drip branch from 1cabb45 to 1289ef4 Compare August 5, 2022 16:32

jumcorredorro added 2 commits August 5, 2022 11:33

Add unsubscribed_users field, and make exclude on running drip

e27d66e

Add views and context build for unsubscribe drips

b0dfac3

jumcorredorro force-pushed the feature/exclude-user-from-drip branch from 1289ef4 to 280e1d6 Compare August 5, 2022 16:34

jumcorredorro changed the base branch from develop to feature/resend-email August 5, 2022 16:35

Change version to 1.16.0, add ignore

923c048

jumcorredorro force-pushed the feature/exclude-user-from-drip branch from 280e1d6 to 923c048 Compare August 5, 2022 16:38

jumcorredorro changed the base branch from feature/resend-email to develop August 5, 2022 18:46

Add test for exclude, admin form, build context message and django vi…

dd90e49

…ews!

jumcorredorro marked this pull request as ready for review August 8, 2022 15:48

jumcorredorro requested a review from a team as a code owner August 8, 2022 15:48

jumcorredorro requested review from brunomichetti, MathiasLantean, JorgeMichelena and malexandroff1 and removed request for a team August 8, 2022 15:48

brunomichetti reviewed Aug 8, 2022

View reviewed changes

Add suggestion by Bruno in condition

958f397

jumcorredorro requested a review from brunomichetti August 9, 2022 12:43

brunomichetti approved these changes Aug 9, 2022

View reviewed changes

Add suggestions by Bruno

155511c

MathiasLantean approved these changes Aug 10, 2022

View reviewed changes

jumcorredorro merged commit 6372ffb into develop Aug 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/exclude user from drip #171

Feature/exclude user from drip #171

jumcorredorro commented Aug 4, 2022 •

edited

Loading

brunomichetti Aug 8, 2022

jumcorredorro Aug 9, 2022

brunomichetti left a comment

brunomichetti Aug 9, 2022

jumcorredorro Aug 9, 2022

brunomichetti Aug 9, 2022

jumcorredorro Aug 9, 2022

brunomichetti Aug 9, 2022

brunomichetti Aug 9, 2022

brunomichetti Aug 9, 2022

jumcorredorro Aug 10, 2022

brunomichetti Aug 9, 2022

jumcorredorro Aug 10, 2022

brunomichetti Aug 9, 2022

jumcorredorro Aug 9, 2022

Feature/exclude user from drip #171

Feature/exclude user from drip #171

Conversation

jumcorredorro commented Aug 4, 2022 • edited Loading

Description

Changes include

Other comments

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brunomichetti left a 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

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

jumcorredorro commented Aug 4, 2022 •

edited

Loading