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.

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

[11][MIG] email_template_qweb: Migration to 11.0 #250

Merged
merged 6 commits into from
May 15, 2018

Conversation

celm1990
Copy link
Sponsor Contributor

PR replace #249
Sorry for create and close various PR. Is my first contribution

@pedrobaeza
Copy link
Member

@celm1990 next time, for avoiding opening more PRs, you just need to use git push -f for overwriting the branch with the new contents. Please try next time or ask me and I'll teach you.

@pedrobaeza pedrobaeza mentioned this pull request Mar 25, 2018
26 tasks

.. image:: https://odoo-community.org/website/image/ir.attachment/5784_f2813bd/datas
:alt: Try me on Runbot
:target: https://runbot.odoo-community.org/runbot/205/10.0
Copy link
Contributor

Choose a reason for hiding this comment

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

/11.0

Demo data contains an example on how to separate corporate identity from a
template's content.

For further information, please visit:
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -0,0 +1,4 @@
# -*- coding: utf-8 -*-
Copy link
Contributor

Choose a reason for hiding this comment

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

remove coding headers

@@ -0,0 +1,4 @@
# -*- coding: utf-8 -*-
# © 2016 Therp BV <http://therp.nl>
Copy link
Contributor

Choose a reason for hiding this comment

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

replace (c) symbol w/ "Copyright"

"author": "Therp BV,Odoo Community Association (OCA)",
"license": "AGPL-3",
"category": "Marketing",
"summary": "Use the QWeb templating mechanism for emails",
Copy link
Contributor

Choose a reason for hiding this comment

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

add 'website': 'https://github.com/OCA/social',

default='jinja2', required=True)
body_view_id = fields.Many2one(
'ir.ui.view', 'Body view', domain=[('type', '=', 'qweb')])
body_view_arch = fields.Text(related=['body_view_id', 'arch'])
Copy link
Contributor

Choose a reason for hiding this comment

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

related='body_view_id.arch'?

result = super(MailTemplate, self).generate_email(
res_ids, fields=fields
)
for record_id, this in self.get_email_template(res_ids).items():
Copy link
Contributor

Choose a reason for hiding this comment

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

we are not in JS here: change this w/ record?

'object': record,
'email_template': this,
})
# Some wizards, like when sending a sales order, need this
Copy link
Contributor

Choose a reason for hiding this comment

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

still the case w/ Odoo 11 and py3?

self.assertTrue(
# this comes from the called template if everything worked
'<footer>' in mail_values[self.env.user.id]['body_html'],
'Did not rcv rendered template in response. Got: \n%s\n' % (
Copy link
Contributor

Choose a reason for hiding this comment

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

s/rcv/??? "receive"?

@@ -33,7 +33,7 @@ def generate_email(self, res_ids, fields=None):
})
# Some wizards, like when sending a sales order, need this
# fix to display accents correctly
if not isinstance(body_html, unicode):
if not isinstance(body_html, str):
Copy link
Member

Choose a reason for hiding this comment

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

I think we'll be better off with using https://github.com/OCA/OCB/blob/11.0/odoo/loglevels.py#L43 here

Copy link

@tschanzt tschanzt left a comment

Choose a reason for hiding this comment

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

Only some minor nitpicking.

@@ -0,0 +1,43 @@

Choose a reason for hiding this comment

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

can your remove the empty line here?

@@ -0,0 +1,4 @@

Choose a reason for hiding this comment

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

can your remove the empty line here?

Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

@celm1990 LGTM but module version. @tschanzt can you update your review?
@hbrunn can you do yours? :)

# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
{
"name": "QWeb for email templates",
"version": "10.0.1.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

11.0.1.0.0

@simahawk
Copy link
Contributor

also, whe you are done, squash your commits pls :)

Copy link

@tschanzt tschanzt left a comment

Choose a reason for hiding this comment

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

Apart from the things @simahawk mentioned it looks good to me.

Copy link
Member

@hbrunn hbrunn left a comment

Choose a reason for hiding this comment

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

same here

@pedrobaeza
Copy link
Member

@celm1990 please make the little remaining things to merge this.

@celm1990 celm1990 force-pushed the 11.0-mig-email_template_qweb branch 2 times, most recently from 9dd3892 to 0aed142 Compare April 27, 2018 19:38
@rafaelbn rafaelbn added this to the 11.0 milestone Apr 28, 2018
@celm1990
Copy link
Sponsor Contributor Author

@pedrobaeza is ok or something is missing???

@simahawk
Copy link
Contributor

@celm1990 this comment seems unattended #250 (comment)
compare "bug tracker" and "contributor" sections w/ the template I linked ;)

Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Approving to not get back, but check README again ;)

@celm1990 celm1990 force-pushed the 11.0-mig-email_template_qweb branch from bd21ca6 to 4c48870 Compare May 10, 2018 14:30
@celm1990
Copy link
Sponsor Contributor Author

@simahawk Done, update readme.rst.

#. Select `QWeb` in the field `Body templating engine`
#. Select a QWeb view to be used to render the body field
#. Apart from QWeb's standard variables, you also have access to ``object`` and ``email_template``, which are browse records of the current object and the email template in use, respectively.
#. Go to ...
Copy link
Member

Choose a reason for hiding this comment

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

You can delete this line

@celm1990 celm1990 force-pushed the 11.0-mig-email_template_qweb branch 3 times, most recently from 58f1a51 to 2de67e8 Compare May 11, 2018 02:09
@pedrobaeza pedrobaeza force-pushed the 11.0-mig-email_template_qweb branch from 2de67e8 to 80436ac Compare May 15, 2018 14:33
@pedrobaeza pedrobaeza merged commit 057aa7b into OCA:11.0 May 15, 2018
@celm1990 celm1990 deleted the 11.0-mig-email_template_qweb branch May 15, 2018 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants