[IMP] mass_mailing_custom_unsubscribe: GDPR compliance #278
Conversation
chienandalu
changed the title
* [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.
chienandalu
force-pushed
the
9.0-mass_mailing_custom_unsubscribe-gdpr
branch
from
2267575
to
dd2bb17
Compare
| for one in self: | ||
| if one.action == "unsubscription" and not one.reason_id: | ||
| raise exceptions.ReasonRequiredError( | ||
| _("Please indicate why are you unsubscribing.")) |
There was a problem hiding this comment.
The tour fails because of an error 500. This seems to be the cause.
Most likely, you need to alter the tour, so it indicates the unsubscription reason.
Or maybe the error pops up in a different form, since this shouldn't be printing an error 500 in the 1st place.
There was a problem hiding this comment.
Grrr, I can't reproduce in my local even with the whole repo CI 😕
|
@yajo please help in this. |
|
You still get an error raised to the tour, resulting in a 500 error page 😕 |
|
@yajo Seems that the context with the reason_id is lost from here: https://github.com/odoo/odoo/blob/9.0/addons/mass_mailing/controllers/main.py#L24 to here: https://github.com/Tecnativa/social/blob/091e7147f700e194078efce8f64983847bc61f64/mass_mailing_custom_unsubscribe/models/mail_mass_mailing.py#L56 |
There was a problem hiding this comment.
You nailed it!
Indeed, I was forgetting that v9 lacks the env autoreset when context changes.
.travis.yml
Outdated
| - TESTS="1" ODOO_REPO="odoo/odoo" EXCLUDE="mass_mailing_custom_unsubscribe" | ||
| - TESTS="1" ODOO_REPO="OCA/OCB" EXCLUDE="mass_mailing_custom_unsubscribe" | ||
| - TESTS="1" ODOO_REPO="odoo/odoo"INCLUDE="mass_mailing_custom_unsubscribe" | ||
| - TESTS="1" ODOO_REPO="OCA/OCB" INCLUDE="mass_mailing_custom_unsubscribe" |
There was a problem hiding this comment.
Remove this change. It wasn't the problem.
| request.context, | ||
| default_reason_id=reason_id, | ||
| default_details=post.get("details") or False, | ||
| ) |
There was a problem hiding this comment.
After changing request.context, do a del request.env.
|
@yajo Two problems I'm getting into:
|
|
To note also, that functional tests are working as far as I could try |
There was a problem hiding this comment.
I detected one failure, let's see if that's the ❌ cause.
Also, you can add timeout to tests if you need it.
| extra_context["default_reason_id"] = int(reason_id) | ||
| if details: | ||
| extra_context["default_details"] = details | ||
| request.context = dict(request.context, **extra_context) |
There was a problem hiding this comment.
After changing request.context, do a del request.env.
…ibe: GDPR compliance (OCA#267)
There was a problem hiding this comment.
Hi @chienandalu @yajo , Travis is still RED but functionally tested 👍 in my side after getting travis green we can merge safety
|
Yes there's still a problem with the tour I can't reproduce manually 😕 |
|
@yajo what can we do with travis 🔴 here? could you check it? Functionally this works! and could be merge! |
…ibe: GDPR compliance (OCA#267)
Implement #267 in
9.0cc @Tecnativa