Skip to content

[MIG] document_page_approval#122

Merged
max3903 merged 2 commits into
OCA:10.0from
ursais:10.0-document_page_approval
Mar 23, 2017
Merged

[MIG] document_page_approval#122
max3903 merged 2 commits into
OCA:10.0from
ursais:10.0-document_page_approval

Conversation

@max3903

@max3903 max3903 commented Jan 19, 2017

Copy link
Copy Markdown
Member

@max3903 max3903 self-assigned this Jan 19, 2017
@pedrobaeza pedrobaeza mentioned this pull request Jan 19, 2017
12 tasks
@max3903 max3903 added this to the 10.0 milestone Jan 19, 2017
@max3903 max3903 force-pushed the 10.0-document_page_approval branch from d7392de to c95ce9e Compare February 2, 2017 14:51

@dreispt dreispt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's a clean port.

@max3903

max3903 commented Mar 21, 2017

Copy link
Copy Markdown
Member Author

@eugen-don or @LeartS any of you can review this pull request?

Thanks!

@LeartS LeartS left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Quick code review, looks good 👍

<field name="email_to">${object.get_approvers_email}</field>
<field name="model_id" ref="model_document_page_history"/>
<field name="auto_delete" eval="True"/>
<field name="lang">${object.create_uid.partner_id.lang}</field>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be in the approver language, not the page author's one?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@LeartS There could be many approvers with different languages. If they need to approve the content, I assumed they can at least read in the language of the creator.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, the creator language is definitely ok too, I was just wondering if there was a way to send each mail in the recipient language (isn't there a way to get the recipient in the evaluation context / variables?).

Also note that the author language is not necessarily the document language, imagine for example a global company where each employee sets their own language in their preferences, but they all contribute to documentation in english.

Anyway, I'm nitpicking. The author language is ok.

@max3903 max3903 merged commit 5269b2c into OCA:10.0 Mar 23, 2017
@max3903 max3903 deleted the 10.0-document_page_approval branch March 23, 2017 15:56
@eugen-don

Copy link
Copy Markdown
Member

seems youre all done with this. sorry for the late notice...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants