12.0 mig mass_mailing_custom_unsubscribe #402

ernestotejeda · 2019-06-25T13:28:42Z

From Odoo 11 to Odoo 12 have changed several things regarding the subscription/unsubscription of a mass mailing contact, so I had to make several adaptations to the module. Some adaptations are described here:

Now in v12 this module does not need to depend on website_mass_mailing or website, just mass_mailing and uses its own layout for the UI
Now the contacts don't have the field opt_out, the class that has this field is a new one that is responsible for managing subscribtions/unsubscribtions from a contact to a list. So now each contact, is related to one or more mass mailing lists and also he/she can be unsubscribed to some of them and at the same time be subscribed to others of them.
For the reason explained above for this version of the module I have eliminated what had been done in v11 of the module to force users to choose only one mailing list per contact.
I have also deleted the not_cross_unsubscriptable field from the mailing lists because already in v12 there is a field called is_public for the same purpose.
In v11, when a user access the unsubscribe link in the email, odoo first shows you the status of your subscriptions and then the user chooses which list he/she wants to subscribe/unsubscribe to, now in v12 ordoo first unsubscribe the user from the list and then it shows the status of subscriptions/unsubscriptions. Because of that the code of the controller was adapted so that a new page is displayed where reasons for unsubscription are requested before that normal unsubscription is executed.
Now in v12, if the email that is going to be de-subscribed is one that does not correspond to a mass mailing contact, for example a partner, it goes to the blacklist. Because of that the module was adapted so that a new page is displayed where reasons for blacklisting are requested before going to the blacklist and the blacklistings are registered as well as the unsubscriptions are registered.

The unit tests using the phantom_js are not included here because I have not been able to make them work, so I hope that someone can help me with this.

Cc @Tecnativa

…ption (OCA#58) * [8.0][IMP][mass_mailing_custom_unsubscribe] Get reasons for unsubscription.

- Imported last updates from v8. - Adapted to v9. - Added a saner default to `mass_mailing.salt` configuration parameter by reusing `database.secret` if available, hoping that some day odoo/odoo#12040 gets merged. - Updated README. - Increase security, drop backwards compatibility. Security got improved upstream, which would again break compatibility among current addon and future master upstream. I choose to break it now and keep it secured future-wise, so I drop the backwards compatibility features. - Includes tour tests. - Removes outdated tests. - Extends the mailing list management form when unsubscriber is a contact. - Adds a reason form even if he is not. - Avoids all methods that were not model-agnostic. [FIX][mass_mailing_custom_unsubscribe] Reasons noupdate After this fix, when you update the addon, you will not lose your customized reasons. [FIX] Compatibilize with mass_mailing_partner Current test code was based on the assumption that the `@api.model` decorator on `create()` ensured an empty recordset when running the method, but that's not true. This was causing an incompatibility betwee these tests and the `mass_mailing_partner` addon, which works assuming 0-1 recordsets. Now records are created from an empty recordset, and thus tests work everywhere. Update instructions If the user does not add the unsubscribe snippet, nothing will happen, so it's added to README to avoid confusion when testing/using the addon. [FIX] Use the right operator to preserve recordsets order Using `|=` sorts records at will each time (treating them as Python's `set`). Using `+=` always appends a record to the end of the set. Since we are using the record position in the set, this caused the test to work sometimes and fail other times. Now it works always.

* [IMP] mass_mailing_custom_unsubscribe: GDPR compliance - Record resubscriptions too. - Record action metadata. - Make ESLint happy. - Quick color-based action distinction in tree view. - Add useful quick groupings. - Display (un)subscription metadata. - Pivot & graph views.

This creates inconsistency issues when assembling them in the README.

Currently translated at 80.0% (44 of 55 strings) Translation: social-11.0/social-11.0-mass_mailing_custom_unsubscribe Translate-URL: https://translation.odoo-community.org/projects/social-11-0/social-11-0-mass_mailing_custom_unsubscribe/fr/

Fix assertIn error thrown in testing mode

Updated by Update PO files to match POT (msgmerge) hook in Weblate.

mass_mailing_custom_unsubscribe/models/mail_unsubscription.py

victormmtorres

I would like have the main changes on the history.rst file because this much more than a migration and will help reviewers

victormmtorres · 2019-06-27T11:58:01Z

mass_mailing_custom_unsubscribe/controllers/main.py

        else:
-            # Unsubscribe, saving reason and details by context


think you could keep a comment not sure by the content if is still valid

Yes, I added the comment again

chienandalu

Great work here, @ernestotejeda

I've been testing the functionality and I'm finding major changes from v11. This is how it worked in v11:

But this is how it works now in v12 (wich is less usable I think):

If only the main screen when the unsubscribe screen was the subscription lists update screen, it would fit the previous behavior.

ernestotejeda · 2019-07-05T14:44:39Z

Thanks for your comment, @chienandalu . I understand your point, but since this is the new unsubscription philosophy now in v12 and I don't think it's less usable, I don't know if it's better to go back to the previous version. Look at the following and tell me what you think ..
In general, the link that appears at the end of the newsletters is 'unsubscribe', but it is not 'Update subscriptions/unsubscription from mailing lists', therefore, I don't see so bad that first the user is unsubscribed after click on that link and then the status of the subscriptions is displayed.
At the beginning, when I started to make the migration, I thought about doing what you propose (keep the previous behavior), but I was getting complicated and also the main objectives of the module are:

Know why and when a contact has subscribed or canceled his subscription to a bulk email.
Provide evidence of why you are sending bulk emails to a specific contact, as required by the GDPR in Europe.

and that's not related to the way you subscribe or unsubscribe

yajo

There are a few stylistic suggestions, some questions and a few requirements to do.

In general terms, the next time you cannot get a tour to pass, just publish the PR with the failing tour, so we reviewers can help you make it pass again. Publishing the PR removing the tour feels like "tricking" somehow. So, please, restore the tours so we can see the failure and fix it. 😉

Thanks!

mass_mailing_custom_unsubscribe/controllers/main.py

mass_mailing_custom_unsubscribe/templates/mass_mailing_contact_reason.xml

mass_mailing_custom_unsubscribe/views/assets.xml

mass_mailing_custom_unsubscribe/models/mail_unsubscription.py

ernestotejeda · 2019-07-09T16:34:07Z

@yajo I have made all the changes except some to which I have responded with a comment

yajo

Just a couple of details to finish.

mass_mailing_custom_unsubscribe/static/src/js/unsubscribe.js

mass_mailing_custom_unsubscribe/views/assets.xml

pedrobaeza

Why the module has no tours now? Why there are removed features? If the scope of the module has changed as well, README should reflect these changes.

@yajo please support @ernestotejeda for a perfect migration, or tell me directly in a meeting the design decisions

mass_mailing_custom_unsubscribe/models/mail_mass_mailing_list.py

yajo

Good point, I forgot the tours thing.

You have to restore them. Please don't remove tours just because they fail; on migrations it's a common thing, and it's not expected that we hide that behind the carpet.

If you cannot make them pass, just push the code and we'll help you 😉

chienandalu · 2019-07-15T08:54:39Z

In general, the link that appears at the end of the newsletters is 'unsubscribe', but it is not 'Update subscriptions/unsubscription from mailing lists', therefore, I don't see so bad that first the user is unsubscribed after click on that link and then the status of the subscriptions is displayed.

Ok, @ernestotejeda Adapt tours to the new behavior and document it if necessary

mass_mailing_custom_unsubscribe/models/mail_mass_mailing.py

mass_mailing_custom_unsubscribe/static/src/js/unsubscribe.js

yajo · 2019-07-16T07:52:51Z

@Tardo, I think @ernestotejeda just copy-pasted upstream's file and tried to stay as close as possible to upstream source code.

In this case, upstream code isn't inheritable and we have to replace it completely (@ged-odoo could you help with that? 😇), so it's one of the cases where it's more maintainable to disable the linter than to obey it (so, when we migrate, we copy the new source code from odoo v13 and just apply our little diff).

victormmtorres

Code LGTM but I would like someone to show me funcional use

cc @chienandalu @rafaelbn

ernestotejeda · 2019-07-18T19:43:22Z

I have made two important changes:

I have added again the field not_cross_unsubscriptable to the mail.mass_mailing.list class and I have made the required adaptations so that this field has the same effect as in the previous version of the module. I had removed it because I thought it was no longer necessary with the new is_public field, but no, I was wrong.
I was able to make Odoo see the tours and I add the UI tests and I adapted its to the changes made in the module in this migration. to make Odoo see the tours I had to add <t t-call =" web.assets_frontend "/> to the of the custom template that odoo uses for the unsubscription.

ernestotejeda · 2019-07-18T20:12:23Z

@victormmtorres , Here you have a GIF with a use case so you can see how it works. The execution is a bit fast, but I hope it helps you:

yajo

@Tardo I took the freedom to mark all of your JS comments as resolved, as I saw your 👍 on #402 (comment). Maybe you want to update your review?

mass_mailing_custom_unsubscribe/models/mail_mass_mailing_list.py

mass_mailing_custom_unsubscribe/tests/test_ui.py

rafaelbn

This module has been lots of issues in the past so please @chienandalu @yajo review in deep.

yajo

Nice, now you have some linters to fix and you're done.

JS lints: https://travis-ci.org/OCA/social/jobs/561045770#L432-L644 (see also comment below)
Python lints: https://travis-ci.org/OCA/social/jobs/561045770#L322-L323

mass_mailing_custom_unsubscribe/static/src/js/unsubscribe.js

ernestotejeda · 2019-07-22T14:00:47Z

Changes done

pedrobaeza · 2019-07-22T17:00:57Z

/ocabot merge

OCA-git-bot · 2019-07-22T17:01:12Z

What a great day to merge this nice PR. Let's do it!
Rebased to 12.0-ocabot-merge-pr-402-by-pedrobaeza-bump-no, awaiting test results.

Signed-off-by pedrobaeza

OCA-git-bot · 2019-07-22T17:35:04Z

Congratulations, your PR was merged at e502eb1. Thanks a lot for contributing to OCA. ❤️

PS: Don't worry if GitHub says there are unmerged commits: it is due to a rebase before merge. All commits of this PR have been merged into 12.0.

antespi and others added 20 commits June 10, 2019 08:09

[ADD] mass_mailing_custom_unsubscribe addon

6cbc6a9

OCA Transbot updated translations from Transifex

2da2ab2

[MIG] Make modules uninstallable

75f2b13

[FIX] remove en.po that was erroneously created by transbot

c09b6e1

[MIG] Rename manifest files

11e3133

Add beautier pages for unsubscription process.

db7ea5f

[8.0][IMP][mass_mailing_custom_unsubscribe] Get reasons for unsubscri…

f24fda3

…ption (OCA#58) * [8.0][IMP][mass_mailing_custom_unsubscribe] Get reasons for unsubscription.

OCA Transbot updated translations from Transifex

d99961a

[MIG][mass_mailing_custom_unsubscribe] Migrate to v10

ad68bf6

OCA Transbot updated translations from Transifex

07ab8a7

[MIG] mass_mailing_custom_unsubscribe: Migration to 11.0

35df61a

[FIX] do not use section titles in readme fragments

68cb82e

This creates inconsistency issues when assembling them in the README.

[UPD] Update mass_mailing_custom_unsubscribe.pot

f57433a

Translated using Weblate (French)

232e279

Currently translated at 80.0% (44 of 55 strings) Translation: social-11.0/social-11.0-mass_mailing_custom_unsubscribe Translate-URL: https://translation.odoo-community.org/projects/social-11-0/social-11-0-mass_mailing_custom_unsubscribe/fr/

mass_mailing_custom_unsubscribe: Fix testing error

345f579

Fix assertIn error thrown in testing mode

[UPD] Update mass_mailing_custom_unsubscribe.pot

d51e00f

[UPD] README.rst

573ab59

Update translation files

907546c

Updated by Update PO files to match POT (msgmerge) hook in Weblate.

ernestotejeda changed the title ~~12.0 mig mass_mailing_custom unsubscribe~~ 12.0 mig mass_mailing_custom_unsubscribe Jun 25, 2019

Tardo reviewed Jun 25, 2019

View reviewed changes

mass_mailing_custom_unsubscribe/models/mail_unsubscription.py Show resolved Hide resolved

victormmtorres suggested changes Jun 27, 2019

View reviewed changes

ernestotejeda force-pushed the 12.0-mig-mass_mailing_custom_unsubscribe branch from fe4bc6b to 07687d3 Compare July 1, 2019 15:41

chienandalu suggested changes Jul 4, 2019

View reviewed changes

yajo requested changes Jul 8, 2019

View reviewed changes

ernestotejeda force-pushed the 12.0-mig-mass_mailing_custom_unsubscribe branch from 07687d3 to 853041a Compare July 9, 2019 16:27

yajo requested changes Jul 10, 2019

View reviewed changes

mass_mailing_custom_unsubscribe/static/src/js/unsubscribe.js Outdated Show resolved Hide resolved

mass_mailing_custom_unsubscribe/views/assets.xml Show resolved Hide resolved

yajo requested review from chienandalu, victormmtorres and Tardo July 11, 2019 07:38

yajo added this to the 12.0 milestone Jul 11, 2019

yajo added the needs review label Jul 11, 2019

pedrobaeza requested changes Jul 15, 2019

View reviewed changes

mass_mailing_custom_unsubscribe/models/mail_mass_mailing_list.py Outdated Show resolved Hide resolved

yajo requested changes Jul 15, 2019

View reviewed changes

Tardo suggested changes Jul 15, 2019

View reviewed changes

victormmtorres approved these changes Jul 18, 2019

View reviewed changes

ernestotejeda force-pushed the 12.0-mig-mass_mailing_custom_unsubscribe branch from 791dec8 to 5b6733a Compare July 18, 2019 19:02

yajo requested changes Jul 19, 2019

View reviewed changes

ernestotejeda force-pushed the 12.0-mig-mass_mailing_custom_unsubscribe branch from 5b6733a to 853ac5b Compare July 19, 2019 14:26

rafaelbn requested changes Jul 19, 2019

View reviewed changes

yajo requested changes Jul 22, 2019

View reviewed changes

mass_mailing_custom_unsubscribe/static/src/js/unsubscribe.js Outdated Show resolved Hide resolved

[MIG] mass_mailing_custom_unsubscribe: Migration to 12.0

fd6344b

ernestotejeda force-pushed the 12.0-mig-mass_mailing_custom_unsubscribe branch from 853ac5b to fd6344b Compare July 22, 2019 13:10

yajo approved these changes Jul 22, 2019

View reviewed changes

OCA-git-bot mentioned this pull request Jul 22, 2019

Migration to version 12.0 #306

Closed

24 tasks

OCA-git-bot added a commit that referenced this pull request Jul 22, 2019

Merge PR #402 into 12.0

e502eb1

Signed-off-by pedrobaeza

OCA-git-bot added the merged 🎉 label Jul 22, 2019

OCA-git-bot closed this Jul 22, 2019

yajo deleted the 12.0-mig-mass_mailing_custom_unsubscribe branch September 20, 2019 07:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

12.0 mig mass_mailing_custom_unsubscribe #402

12.0 mig mass_mailing_custom_unsubscribe #402

ernestotejeda commented Jun 25, 2019 •

edited

Loading

victormmtorres left a comment

victormmtorres Jun 27, 2019

ernestotejeda Jul 1, 2019

chienandalu left a comment

ernestotejeda commented Jul 5, 2019

yajo left a comment

ernestotejeda commented Jul 9, 2019

yajo left a comment

pedrobaeza left a comment

yajo left a comment

chienandalu commented Jul 15, 2019

yajo commented Jul 16, 2019

victormmtorres left a comment

ernestotejeda commented Jul 18, 2019

ernestotejeda commented Jul 18, 2019

yajo left a comment

rafaelbn left a comment

yajo left a comment

ernestotejeda commented Jul 22, 2019

pedrobaeza commented Jul 22, 2019

OCA-git-bot commented Jul 22, 2019

OCA-git-bot commented Jul 22, 2019

12.0 mig mass_mailing_custom_unsubscribe #402

12.0 mig mass_mailing_custom_unsubscribe #402

Conversation

ernestotejeda commented Jun 25, 2019 • edited Loading

victormmtorres left a comment

Choose a reason for hiding this comment

victormmtorres Jun 27, 2019

Choose a reason for hiding this comment

ernestotejeda Jul 1, 2019

Choose a reason for hiding this comment

chienandalu left a comment

Choose a reason for hiding this comment

ernestotejeda commented Jul 5, 2019

yajo left a comment

Choose a reason for hiding this comment

ernestotejeda commented Jul 9, 2019

yajo left a comment

Choose a reason for hiding this comment

pedrobaeza left a comment

Choose a reason for hiding this comment

yajo left a comment

Choose a reason for hiding this comment

chienandalu commented Jul 15, 2019

yajo commented Jul 16, 2019

victormmtorres left a comment

Choose a reason for hiding this comment

ernestotejeda commented Jul 18, 2019

ernestotejeda commented Jul 18, 2019

yajo left a comment

Choose a reason for hiding this comment

rafaelbn left a comment

Choose a reason for hiding this comment

yajo left a comment

Choose a reason for hiding this comment

ernestotejeda commented Jul 22, 2019

pedrobaeza commented Jul 22, 2019

OCA-git-bot commented Jul 22, 2019

OCA-git-bot commented Jul 22, 2019

ernestotejeda commented Jun 25, 2019 •

edited

Loading