-
-
Notifications
You must be signed in to change notification settings - Fork 587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MIG] mass_mailing_list_dynamic: Migration to 12.0 #399
[MIG] mass_mailing_list_dynamic: Migration to 12.0 #399
Conversation
82a82af
to
75f5647
Compare
@Tardo Changes done |
75f5647
to
e207140
Compare
I did pushed force again |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I click on the Sync now
button an error is thrown. Could you check that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @victormmtorres Some comments below:
mass_mailing_list_dynamic/views/mail_mass_mailing_list_view.xml
Outdated
Show resolved
Hide resolved
mass_mailing_list_dynamic/views/mail_mass_mailing_list_view.xml
Outdated
Show resolved
Hide resolved
e207140
to
bdd3b87
Compare
Changes done @ernestotejeda @chienandalu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have made some comments and I have made a PR suggesting some changes (Tecnativa#2). Please take a look
Hi @ernestotejeda could you push your commits directly to my branch? Here would be more efficient than compare and I extract what you expect |
@victormmtorres I pushed the Commit here on your branch and I have included what @pedrobaeza suggessted in this comment #399 (comment) |
8c7f547
to
d22702a
Compare
@yajo please review |
d22702a
to
0559e54
Compare
@yajo changes done |
@chienandalu Please review |
@victormmtorres you still have pending reviews from @Tardo and @ernestotejeda. Please attend their comments. Thanks! |
@yajo better that you take this as @victormmtorres is now with other things |
* [FIX+IMP] mass_mailing_list_dynamic: tests, icons, filters... * Brand new icon * Added feature of loading an existing filter as criteria * Tests as SavepointCase for optimizing times * Tests in post-install for avoiding errors on res.partner not null constraints when several modules added them. * Updated documentation. * Fix mock in test for not commiting test data. * [FIX] mass_mailing_list_dynamic: Wasn't able to create contacts in fully synced lists Syncing context was being set in the wrong object. Added to test too. * [FIX] mass_mailing_list_dynamic: Allow to write back vals from res.partner Module mass_mailing_partner writes back certain values from partner to mass_mailing_contact. Module should allow that write operation.
- Adds is_synced field to track whether a dynamic list has unsynced changes or not so the user is aware that the definitive number of contacts is yet to be determined. - It fixes an issue that made impossible deleting a res.partner filter when a list had use it to filter contacts. - It also shows only the filters available for the user (shared and belonging to self).
0559e54
to
2777d6f
Compare
@yajo Changes done |
2777d6f
to
b9eeff7
Compare
@Tardo changes done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
/ocabot merge |
What a great day to merge this nice PR. Let's do it! |
Congratulations, your PR was merged at 1da0313. 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 |
No description provided.