[19.0][MIG] privacy_consent: Migration to 19.0 - #97
Open
FrancescoBellingeri wants to merge 41 commits into
Open
Conversation
Update privacy_consent.pot README.rst README.rst
Currently translated at 100.0% (74 of 74 strings) Translation: data-protection-10.0/data-protection-10.0-privacy_consent Translate-URL: https://translation.odoo-community.org/projects/data-protection-10-0/data-protection-10-0-privacy_consent/pt/
The purpose of this `@post_install` no longer exists, and by removing these decorators, we avoid test failures in case some custom addon customizes the welcome message.
Before this patch, all privacy consent emails were forced to be sent under a context with `mark_consent_sent=True` to track the state change and trigger the attached server action. When using the automatic mode, if a separate cron job started running the mail queue, it could happen that mails were being sent from that another worker, losing the context key and, as such, being sent without token and without being marked as sent nor executing the attached server action (if any). To avoid this problem, now the context dependency is removed. After all, the `mail.mail` object is only created when sending in `mass_mail` model, so other kind of notifications or messages are not affected. When a mail of type `mass_mail` is sent, we can assume that it is asking for consent and we can move draft consent requests to sent. Update privacy_consent.pot Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: data-protection-10.0/data-protection-10.0-privacy_consent Translate-URL: https://translation.odoo-community.org/projects/data-protection-10-0/data-protection-10-0-privacy_consent/
It could happen that, while Odoo is still sending emails, a subject receives it and clicks on accept/reject links. In such case, he'd get a 404 error because the record wouldn't exist yet in the database. That's because the DB commit was made only after processing all the sent emails. We need to commit in advance to make sure that doesn't happen.
Currently translated at 79.5% (58 of 73 strings) Translation: data-protection-10.0/data-protection-10.0-privacy_consent Translate-URL: https://translation.odoo-community.org/projects/data-protection-10-0/data-protection-10-0-privacy_consent/de/ Update privacy_consent.pot Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: data-protection-10.0/data-protection-10.0-privacy_consent Translate-URL: https://translation.odoo-community.org/projects/data-protection-10-0/data-protection-10-0-privacy_consent/
Before OCA#29 there was a race condition where an email could be sent while the same transaction that created the `privacy.consent` record still wasn't committed, producing a 404 error if the user clicked on "Accept" or "Reject" before all mails were sent. To avoid that, a raw `cr.commit()` was issued, but this produced another situation where the user had to wait until the full email queue is cleared to get his page loaded. It wasn't an error, but a long queue meant several minutes waiting, and it's ulikely that an average human is so patient. So, here's the final fix (I hope!). The main problem was that I was looking in the wrong place to send the email. It turns out that the `self.post_message_with_template()` method is absolutely helpless in the case at hand, where these criteria must be met: * E-mail must be enqueued, no matter if there are less or more than 50 consents to send. * The template must be processed per record. * In an ideal world, a `cr.commit()` must be issued after each sent mail. The metod that was being used: * Didn't allow to use `auto_commit` mode. * Only allowed to render the template per record if called with `composition_mode="mass_mail"`. * Only allowed to enqueue emails if called with `composition_mode="mass_post"`. Obviously, I cannot set 2 different values for `composition_mode`, so a different strategy had to be used. I discovered that the `mail.template` model has a helpful method called `send_mail()` that, by default: * Renders the template per record * Enqueues the email * The email queue is cleared in `auto_commit=True` mode. So, from now on, problems are gone: * The user click, or the cron run, will just generate the missing `privacy.consent` records and enqueue mails for them. * The mail queue manager will send them later, in `auto_commit` mode. * After sending the e-mail, this module will set the `privacy.consent` record as `sent`. * Thanks to *not* sending the email, the process the user faces when he hits the "generate" button is faster. * Instructions in the README and text in the "generate" button are updated to reflect this new behavior. * Thanks to the `auto_commit` feature, if Odoo is rebooted in the middle of a mail queue clearance, the records that were sent remain properly marked as sent, and the missing mails will be sent after the next boot. * No hardcoded commits. * No locked transactions. * BTW I discovered that 2 different emails were created when creating a new consent. I started using `mail_create_nolog=True` to avoid that problem and only log a single creation message. Note to self: never use again `post_message_with_template()`. Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: data-protection-10.0/data-protection-10.0-privacy_consent Translate-URL: https://translation.odoo-community.org/projects/data-protection-10-0/data-protection-10-0-privacy_consent/ Update privacy_consent.pot Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: data-protection-10.0/data-protection-10.0-privacy_consent Translate-URL: https://translation.odoo-community.org/projects/data-protection-10-0/data-protection-10-0-privacy_consent/
[UPD] Update privacy_consent.pot [UPD] README.rst
- Partner's `opt_out` no longer exists. Using `mail.blacklist` now. - Tests updated to support that change. - Test workarounds removed. - Duplicated-field-name-in-model warning removed. - Use create multi where possible.
Due to a bug present in the `_postprocess_sent_message` code, if a consent wasn't successfully sent, still the consent got marked as sent. It should stay as draft. Modify tests to test this new behavior. @Tecnativa TT24457 privacy_consent 12.0.1.0.1 Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: data-protection-12.0/data-protection-12.0-privacy_consent Translate-URL: https://translation.odoo-community.org/projects/data-protection-12-0/data-protection-12-0-privacy_consent/
Some little nasty details were overlooked in the v12 migration, which rendered the module basically useless: - The wizard used to ask for consent in manual activities did not fill the placeholders, resulting in an awkward UX. - The sent mails **did not have the token** to authenticate and actually allow accepting or rejecting. These buttons returned a 500 error. - Once the token is added, the form was ugly. - Sadly, some tests were testing the how and not the what. It is understandable due to the complexity of testing the what, even more without the `Form` utility, new on v12. These are fixed now too. @Tecnativa TT25868 privacy_consent 12.0.1.1.0
As the controller is `auth="none"`, sometimes there's no user. This wasn't a problem on v12 where `sudo()` applied `SUPERUSER_ID` by default (below, line 35). In v13 we need to add it manually to avoid some situations where `self.env.uid == False`. @Tecnativa privacy_consent 13.0.1.0.1
privacy_consent 13.0.1.0.2
Before this fix, this module would work in a multidatabase instance. However, it would introduce random conflicts when used together with `website_sale` and its dependencies. According to [docs][1]: > For servers hosting multiple databases in production, especially if website is used, dbfilter must be set, otherwise a number of features will not work correctly. If we consider this module to fall into that category, we remove some of its workarounds that were only needed for unsupported scenarios. Less errors with less code. [1] https://www.odoo.com/documentation/13.0/setup/deploy.html#dbfilter privacy_consent 13.0.1.0.3 [UPD] Update privacy_consent.pot Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: data-protection-13.0/data-protection-13.0-privacy_consent Translate-URL: https://translation.odoo-community.org/projects/data-protection-13-0/data-protection-13-0-privacy_consent/
…ave access to privacy. TT35907
Containing
* assets mechanism now in manifest;
* groups not supported on view definition itself but in arch;
* templates should use QWeb syntax since 15.0;
* update privacy consent form view so that description html field correctly
takes the whole available place, issue due to small rendering changes in
views coming with new owl2 engine;
* use update_context on request instead of updating context directly;
* t-raw is deprecated, use t-out instead;
* update methods used in tests: cache management, composer naming;
* update doc links;
…ing it As we now use t-out in frontend templates we cannot inject directly html content. This was possible with t-raw that is deprecated. In this commit we fix that issue by using Markup in order to create valid html content based on partner html-ified name and use it in template. Other solution would be to have display_name of partner being automatically updated depending on context keys, but as it is a stored field it cannot be done.
In this commit we add tests for invalid or no token access in order to test corner cases. Those tests are done in a new class to better reflect test purpose. Some ACLs tests are also added.
Using consteq lessen potential attacks using timing.
Currently translated at 100.0% (92 of 92 strings) Translation: data-protection-16.0/data-protection-16.0-privacy_consent Translate-URL: https://translation.odoo-community.org/projects/data-protection-16-0/data-protection-16-0-privacy_consent/es/
Currently translated at 100.0% (91 of 91 strings) Translation: data-protection-16.0/data-protection-16.0-privacy_consent Translate-URL: https://translation.odoo-community.org/projects/data-protection-16-0/data-protection-16-0-privacy_consent/it/
Currently translated at 40.4% (36 of 89 strings) Translation: data-protection-18.0/data-protection-18.0-privacy_consent Translate-URL: https://translation.odoo-community.org/projects/data-protection-18-0/data-protection-18-0-privacy_consent/sl/
Currently translated at 100.0% (89 of 89 strings) Translation: data-protection-18.0/data-protection-18.0-privacy_consent Translate-URL: https://translation.odoo-community.org/projects/data-protection-18-0/data-protection-18-0-privacy_consent/sl/
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migration of
privacy_consentfrom 18.0 to 19.0, following the OCA migration guide.Depends on
privacybeing migrated first: #96.git format-patch/git am._sql_constraintsreplaced bymodels.Constraint.read_groupreplaced by_read_group(2 call sites).mail.mail._postprocess_sent_messageoverride updated for the new requiredsuccess_emailsparameter._message_get_suggested_recipients/_message_add_suggested_recipientoverride rewritten for the new batch-oriented_message_add_suggested_recipientsAPI.string="Group By"from the search view groupby<group>.IrMailServerrenamed toIrMail_Serverin core._disable_send()guard at the top ofmail.mail._send(), so simply patchingbuild_emailis no longer enough to exercise the real send path in a test; the test now also fakessmtplib.SMTP/SMTP_SSLand forces_disable_send()toFalse, mirroringodoo.addons.base.tests.common.MockSmtplibCase.privacy.consent.state(a tracked field) picks up an extra automatic "State Changed" tracking message on transition tosent; assertions were adjusted to expect it only when the recipient's email is actually valid.Test plan
pre-commit run -aclean