Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[NEW][mail_footer_notified_partners] #37

Merged
merged 1 commit into from
Jan 28, 2016
Merged

[NEW][mail_footer_notified_partners] #37

merged 1 commit into from
Jan 28, 2016

Conversation

JonathanNEMRY
Copy link

This module adds into the footer the partner's name notified by this email

This module was proposed into OCA/server-tools#147 but it is a better candidate for this repository

@sbidoul sbidoul mentioned this pull request Jan 20, 2016
Closed
@sbidoul
Copy link
Member

sbidoul commented Jan 20, 2016

As on the original PR, 👍

elicoidal
elicoidal reviewed Jan 21, 2016
View reviewed changes
mail_footer_notified_partners/README.rst

.. image:: http://odoo-community.org/logo.png
:alt: Odoo Community Association
:target: http://odoo-community.org
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https (x2)

@elicoidal
Copy link

Thanks for the contribution!

@JonathanNEMRY
Copy link
Author

Hi @elicoidal
Thanks for your review! Maybe a 👍 and it will be ready to merge

@max3903
Copy link
Member

max3903 commented Jan 26, 2016

👍

elicoidal
elicoidal reviewed Jan 26, 2016
View reviewed changes
mail_footer_notified_partners/README.rst

OCA, or the Odoo Community Association, is a nonprofit organization whose mission is to support the collaborative development of Odoo features and promote its widespread use.

To contribute to this module, please visit http://odoo-community.org.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

htpps

@JonathanNEMRY JonathanNEMRY force-pushed the mail_footer_notified_partners branch 3 times, most recently from 81c1d6b to 6fee02b Compare January 27, 2016 10:29
@JonathanNEMRY
Copy link
Author

@elicoidal
Concerning the additional checkbox on partner, I propose to not integrate it into this PR.
I still outsourced the computation of the partners_name into an other method in order to easily override it and add filter with other criterias.

I also changed my code in order to be more consistency with Odoo: even if the partner has checked Receive Inbox Notifications by Email: never it will be added into the footer too as Odoo send a notification into its inbox message too.

Hope this is OK for you now. Thanks again for your review

Jonathan

@elicoidal
Copy link

Ok. Maybe you can add a note in roadmap. 

Eric Caudal from my mobile device

[NEW][mail_footer_notified_partners]
38dbb7f 
This module adds into the footer the partner's name notified by this email
@JonathanNEMRY
Copy link
Author

@elicoidal
I've updated the .rst is is ok for you now?

@elicoidal
Copy link

👍

elicoidal added a commit that referenced this pull request Jan 28, 2016
@elicoidal
Merge pull request #37 from acsone/mail_footer_notified_partners
3a14bbc 
[NEW][mail_footer_notified_partners]
@elicoidal elicoidal merged commit 3a14bbc into OCA:8.0 Jan 28, 2016
@JonathanNEMRY
Copy link
Author

Thanks!

hbrunn
hbrunn reviewed Jan 28, 2016
View reviewed changes
mail_footer_notified_partners/tests/test_mail_notification.py
# © 2016 ACSONE SA/NV <https://acsone.eu>
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).

from anybox.testing.openerp import SharedSetupTransactionCase
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is going to break travis if you don't adapt the .travis.yml file

@hbrunn
Copy link
Member

hbrunn commented Jan 28, 2016

@elicoidal what happened to only merging PRs which are green on CI? https://travis-ci.org/OCA/social/jobs/105446506#L239

@JonathanNEMRY please repair this

@hbrunn
Copy link
Member

hbrunn commented Jan 28, 2016

@elicoidal @JonathanNEMRY weird, this branches runbot is green, but still the error is there: https://travis-ci.org/OCA/social/jobs/105188282#L229. Problem is that now all other new commits will be red until this is fixed. What does anybox' TransactionCase do that you need here?

@JonathanNEMRY
Copy link
Author

Hope this will repair https://github.com/OCA/social/pull/39/files

@sbidoul sbidoul deleted the mail_footer_notified_partners branch January 28, 2016 17:17
@elicoidal
Copy link

@hbrunn Sorry! I went too quick and saw it afterwards.
Will be more careful in the future

SimoRubi pushed a commit to SimoRubi/social that referenced this pull request Oct 10, 2019
@sbidoul
Merge OCA#37 into master
dfd8538
@rafaelbn rafaelbn mentioned this pull request Jun 3, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JonathanNEMRY @sbidoul @elicoidal @max3903 @hbrunn