[16.0][MIG] mail_tracking: Migration to 16.0 #1216

payen000 · 2023-09-05T20:49:03Z

Redo of PR #1191; this PR is opened due to inactivity of #1091 (from which the MessagingInitializer._init fix was taken).

payen000 · 2023-09-05T21:20:33Z

Hello @luisg123v, could you review this, please?

chienandalu

Some serious issue when trying to test it functionally:

Whenever a enter a record I get an error when it tries to load the failed messages tracker:

TypeError: Cannot read properties of undefined (reading 'refreshMessagefailed') at model._onThreadIdOrThreadModelChanged (http://oca-social-16-0-pr1216-13154c4fcbef.runboat.odoo-community.org/web/assets/debug/web.assets_backend.js:176759:25) (/mail_tracking/static/src/js/models/chatter.esm.js:20) at model._onThreadIdOrThreadModelChanged (http://oca-social-16-0-pr1216-13154c4fcbef.runboat.odoo-community.org/web/assets/debug/web.assets_backend.js:130691:31) (/mail/static/src/model/model_core.js:371) at Listener.onChange (http://oca-social-16-0-pr1216-13154c4fcbef.runboat.odoo-community.org/web/assets/debug/web.assets_backend.js:133311:52) (/mail/static/src/model/model_manager.js:886) at ModelManager._executeCreatedRecordsOnChange (http://oca-social-16-0-pr1216-13154c4fcbef.runboat.odoo-community.org/web/assets/debug/web.assets_backend.js:133315:26) (/mail/static/src/model/model_manager.js:890) at ModelManager._flushUpdateCycle (http://oca-social-16-0-pr1216-13154c4fcbef.runboat.odoo-community.org/web/assets/debug/web.assets_backend.js:133352:14) (/mail/static/src/model/model_manager.js:927) at ModelManager._notifyListenersInUpdateCycle (http://oca-social-16-0-pr1216-13154c4fcbef.runboat.odoo-community.org/web/assets/debug/web.assets_backend.js:133522:18) (/mail/static/src/model/model_manager.js:1097) at ModelManager._flushUpdateCycle (http://oca-social-16-0-pr1216-13154c4fcbef.runboat.odoo-community.org/web/assets/debug/web.assets_backend.js:133349:14) (/mail/static/src/model/model_manager.js:924) at ModelManager._executeCreatedRecordsComputes (http://oca-social-16-0-pr1216-13154c4fcbef.runboat.odoo-community.org/web/assets/debug/web.assets_backend.js:133269:18) (/mail/static/src/model/model_manager.js:844) at ModelManager._flushUpdateCycle (http://oca-social-16-0-pr1216-13154c4fcbef.runboat.odoo-community.org/web/assets/debug/web.assets_backend.js:133348:14) (/mail/static/src/model/model_manager.js:923) at ModelManager.insert (http://oca-social-16-0-pr1216-13154c4fcbef.runboat.odoo-community.org/web/assets/debug/web.assets_backend.js:132678:14) (/mail/static/src/model/model_manager.js:253)

The mail message filter isn't injected into the filter menu:

chienandalu · 2023-09-06T08:18:21Z

Also, if you took changes from the other PR is good to respect the contributor attribution in a separate commit

payen000 · 2023-09-06T20:54:27Z

Hello @chienandalu, thank you for testing,

the issues you found should now be solved, could you check again, please?

As for this comment:

Also, if you took changes from the other PR is good to respect the contributor attribution in a separate commit

I added a new commit as you suggested, but including Co-authorship and referencing the original PR, please let me know if there is a better/more respectful way of doing this, as I was not able to find a similar case in the guidelines,

thanks again!

chienandalu

Thanks @payen000 :) That can be a way to attribute changes, yes :) (normally I try to cherry-pick and adapt, so I keep the original commit as similar as possible, but is ok)

Functionally I still see some issues:

Clicking on the tracking status icon doesn't show up the trackings dialog.
The failed messages review doesn't show up in the record thread either

payen000 · 2023-09-11T07:09:18Z

Hello again @chienandalu, thank you for the tips, I appreciate it!

I've updated the code, you should be able to see the failed messages and the tracking action now, could you check again, please?

payen000 · 2023-09-25T16:08:23Z

Hello @chienandalu, I've updated the code so that the MessagingInitializer changes from the first PR are present in their original commit (which also contains a good chunk of the first PR's migration) and added my changes on top as you suggested. Could you check, please?

chienandalu

Looks like your re-rendering the whole window after each action with the failed messages . Can't you use owl reactivity?

chienandalu

Superb work! I made a succesful functional tour and a final code review

Just two final tweaks 🙂

chienandalu · 2023-09-27T08:40:23Z

mail_tracking/readme/CONTRIBUTORS.rst

Put yourself in the contributors list 😄 👍

chienandalu · 2023-09-27T08:49:42Z

mail_tracking/controllers/discuss.py

+
+class MailTrackingDiscussController(DiscussController):
+    @http.route()
+    def mail_init_messaging(self):


This method has a different signature (https://github.com/odoo/odoo/blob/913c8678cba0741e99610b1174ba6c42ef0c21a6/addons/mail/controllers/discuss.py#L194)

Suggested change

def mail_init_messaging(self):

def mail_init_messaging(self, **kwargs):

payen000 · 2023-09-27T16:43:50Z

Hello @chienandalu,

Thank you for reviewing and testing this far, and of course, for your words of encouragement!

I've added your suggested changes, could you check please?

Also, I converted the 'message ids list' inside store into a Set for a faster/more reliable filtering of reviewed messages, since the counter of failed messages on any chatter was being reduced by the total number of stored IDs from all chatters - but not anymore, it is now working as expected.

Thanks again!

chienandalu · 2023-09-29T08:19:00Z

Oh, after the last changes:

I can't see the failed messages in the discuss app
I only see one of the demo failed messages in the record

payen000 · 2023-10-01T04:23:46Z

Sorry @chienandalu, it seems I forgot to reset the runbot after my own tests, that's why most of the records were not visible as failed, after resetting the runbot the failed messages were visible again :)

I've done one last push to re-render the "Retry" and "Set as reviewed" options properly from the Inbox/History/Starred mailboxes, could you check please?

chienandalu

I think we're ready 😄 👍

Thanks!

payen000 · 2023-10-04T04:15:41Z

Thank you @chienandalu!

payen000 · 2023-10-06T23:05:49Z

Hello @luisg123v, could you review this, please?

mail_tracking/demo/demo.xml

mail_tracking/tests/test_mail_tracking.py

mail_tracking/security/mail_tracking_email_security.xml

mail_tracking/models/mail_tracking_email.py

The following changes were implemented: 1 - Added Failed Message component and related components to reuse the Message component when rendering failed messages. This allows us to dispose of the messagefailed JS model altogether, since failed messages are now just regular messages that were marked as failed. 2 - Added Owl reactivity to failed message actions so that browser does not have to be reloaded each time a message is marked as reviewed or resent. 3 - Fixed 'Retry' and 'Set as reviewed' flows for failed messages. 4 - Fixed `Failed sent messages` filter on models by overriding `get_view` instead of `_fields_view_get` 5 - Refactored folder structure to more closely resemble the `mail` module's folder structure. 6 - Refactored module to utilize `Command` as a means to create, write, etc. instead of `[0, ...]`, `[4, ...]`. 7 - Fixed and added unit tests. 8 - Removed dead/unused code from `v15`.

payen000 · 2023-10-09T15:03:49Z

Hello @luisg123v, I've applied your suggestions, could you check please?

luisg123v

LGTM 👍

pedrobaeza · 2023-10-10T06:14:23Z

/ocabot merge nobump

OCA-git-bot · 2023-10-10T06:15:12Z

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-1216-by-pedrobaeza-bump-nobump, awaiting test results.

OCA-git-bot · 2023-10-10T06:26:03Z

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

payen000 force-pushed the 16.0-mail_tracking-mig-payen branch 3 times, most recently from 5942941 to 13154c4 Compare September 5, 2023 21:09

payen000 marked this pull request as ready for review September 5, 2023 21:15

payen000 mentioned this pull request Sep 5, 2023

[16.0][MIG] mail_tracking: Migration to 16.0 #1091

Closed

pedrobaeza requested a review from chienandalu September 6, 2023 06:42

chienandalu suggested changes Sep 6, 2023

View reviewed changes

payen000 force-pushed the 16.0-mail_tracking-mig-payen branch 2 times, most recently from 9f7106f to d1a3359 Compare September 6, 2023 19:47

payen000 marked this pull request as draft September 6, 2023 20:31

payen000 marked this pull request as ready for review September 6, 2023 20:49

payen000 requested a review from chienandalu September 6, 2023 20:54

chienandalu suggested changes Sep 7, 2023

View reviewed changes

payen000 marked this pull request as draft September 7, 2023 20:27

payen000 force-pushed the 16.0-mail_tracking-mig-payen branch 9 times, most recently from 33c61ff to caf2c08 Compare September 10, 2023 01:20

payen000 marked this pull request as ready for review September 10, 2023 01:41

payen000 force-pushed the 16.0-mail_tracking-mig-payen branch 2 times, most recently from 6500e9c to 1d3b076 Compare September 11, 2023 07:00

payen000 requested a review from chienandalu September 25, 2023 16:08

chienandalu reviewed Sep 26, 2023

View reviewed changes

payen000 force-pushed the 16.0-mail_tracking-mig-payen branch from c2faec5 to 3915290 Compare September 27, 2023 01:01

chienandalu suggested changes Sep 27, 2023

View reviewed changes

payen000 force-pushed the 16.0-mail_tracking-mig-payen branch 2 times, most recently from 632dc0c to 504a304 Compare September 27, 2023 16:13

payen000 requested a review from chienandalu September 27, 2023 16:43

payen000 force-pushed the 16.0-mail_tracking-mig-payen branch from 504a304 to 1205086 Compare October 1, 2023 04:03

chienandalu approved these changes Oct 3, 2023

View reviewed changes

luisg123v suggested changes Oct 7, 2023

View reviewed changes

payen000 force-pushed the 16.0-mail_tracking-mig-payen branch from 1205086 to ef73e2d Compare October 9, 2023 15:01

payen000 requested a review from luisg123v October 9, 2023 15:03

luisg123v approved these changes Oct 10, 2023

View reviewed changes

OCA-git-bot added the approved label Oct 10, 2023

OCA-git-bot added the bot is merging ⏳ label Oct 10, 2023

OCA-git-bot merged commit b41a54a into OCA:16.0 Oct 10, 2023
6 of 7 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Oct 10, 2023

rafaelbn mentioned this pull request Oct 19, 2023

[16.0][MIG] mail_tracking_mass_mailing: Migration to 16.0 #1099

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[16.0][MIG] mail_tracking: Migration to 16.0 #1216

[16.0][MIG] mail_tracking: Migration to 16.0 #1216

payen000 commented Sep 5, 2023 •

edited

Loading

payen000 commented Sep 5, 2023

chienandalu left a comment •

edited

Loading

chienandalu commented Sep 6, 2023

payen000 commented Sep 6, 2023

chienandalu left a comment

payen000 commented Sep 11, 2023

payen000 commented Sep 25, 2023

chienandalu left a comment

chienandalu left a comment

chienandalu Sep 27, 2023

chienandalu Sep 27, 2023

payen000 commented Sep 27, 2023

chienandalu commented Sep 29, 2023

payen000 commented Oct 1, 2023

chienandalu left a comment

payen000 commented Oct 4, 2023

payen000 commented Oct 6, 2023

payen000 commented Oct 9, 2023

luisg123v left a comment

pedrobaeza commented Oct 10, 2023

OCA-git-bot commented Oct 10, 2023

OCA-git-bot commented Oct 10, 2023

	def mail_init_messaging(self):
	def mail_init_messaging(self, **kwargs):

[16.0][MIG] mail_tracking: Migration to 16.0 #1216

[16.0][MIG] mail_tracking: Migration to 16.0 #1216

Conversation

payen000 commented Sep 5, 2023 • edited Loading

payen000 commented Sep 5, 2023

chienandalu left a comment • edited Loading

Choose a reason for hiding this comment

chienandalu commented Sep 6, 2023

payen000 commented Sep 6, 2023

chienandalu left a comment

Choose a reason for hiding this comment

payen000 commented Sep 11, 2023

payen000 commented Sep 25, 2023

chienandalu left a comment

Choose a reason for hiding this comment

chienandalu left a comment

Choose a reason for hiding this comment

chienandalu Sep 27, 2023

Choose a reason for hiding this comment

chienandalu Sep 27, 2023

Choose a reason for hiding this comment

payen000 commented Sep 27, 2023

chienandalu commented Sep 29, 2023

payen000 commented Oct 1, 2023

chienandalu left a comment

Choose a reason for hiding this comment

payen000 commented Oct 4, 2023

payen000 commented Oct 6, 2023

payen000 commented Oct 9, 2023

luisg123v left a comment

Choose a reason for hiding this comment

pedrobaeza commented Oct 10, 2023

OCA-git-bot commented Oct 10, 2023

OCA-git-bot commented Oct 10, 2023

payen000 commented Sep 5, 2023 •

edited

Loading

chienandalu left a comment •

edited

Loading