Merge lp://staging/~invitu/ocb-addons/6.1-fix-1193220 into lp://staging/ocb-addons/6.1

Proposed by invitu
Status: Work in progress
Proposed branch: lp://staging/~invitu/ocb-addons/6.1-fix-1193220
Merge into: lp://staging/ocb-addons/6.1
Diff against target: 76 lines (+31/-3)
2 files modified
account_followup/i18n/account_followup.pot (+6/-0)
account_followup/wizard/account_followup_print.py (+25/-3)
To merge this branch: bzr merge lp://staging/~invitu/ocb-addons/6.1-fix-1193220
Reviewer Review Type Date Requested Status
Stefan Rijnhart (Opener) Needs Fixing
Review via email: mp+170748@code.staging.launchpad.net

Description of the change

[FIX] attachment and sender email are missing in account followup emails (bug 1193220)

To post a comment you must log in.
Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Thanks! Can you please undo your monolithic commit, and cherrypick the OPW's distinguishing commits? Afterwards, commit your additional change in a separate commit. This will help us review your additional change and prevent conflicts should the OPW get merged in upstream.

Mention the OPW in your merge proposal and give credit to its author by committing the cherrypicking merge using --author [author's emailaddress].

Does this bug affect 7.0? Your bug report seems to indicate it. Please clarify. Also link the OPW branch to the bug report.

review: Needs Fixing
Revision history for this message
invitu (invitu) wrote :

> Does this bug affect 7.0? Your bug report seems to indicate it. Please
> clarify. Also link the OPW branch to the bug report.
My mistake, I removed 7.0 link

6754. By rgo-openerp

[FIX] account_followup: mail sent using followup wizard do not have pdf attached: (Maintenance Case : 578235) (6.1-opw-578235-rgo)

6755. By invitu

|FIX] account_followup : sender email was missing in followup emails (bug 1193220)

Revision history for this message
invitu (invitu) wrote :

> Thanks! Can you please undo your monolithic commit, and cherrypick the OPW's
> distinguishing commits? Afterwards, commit your additional change in a
> separate commit. This will help us review your additional change and prevent
> conflicts should the OPW get merged in upstream.
>
> Mention the OPW in your merge proposal and give credit to its author by
> committing the cherrypicking merge using --author [author's emailaddress].

Thanks Stefan, you learned me how to cherry pick
Could you check out my moves ?

Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Thanks, the merging went really well!

Unfortunately, the OPW code is not very nice.

l.39..43 this is begging for the use of setdefault
l.36 reuse of variable 'partners', now a list of browse records instead of ids
l.68..72 Looks very suspicious to me. Can this perhaps be replaced by

    filename = _('Followup-%s.%s') % (partner.name, frmt)
    attach = {filename: datax}

Your code has an error too, I suspect:
l.54 should be "tools.config.options.get", not "tools.config.get"

review: Needs Fixing
Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

Setting to work in progress after > 2 months without response

Unmerged revisions

6755. By invitu

|FIX] account_followup : sender email was missing in followup emails (bug 1193220)

6754. By rgo-openerp

[FIX] account_followup: mail sent using followup wizard do not have pdf attached: (Maintenance Case : 578235) (6.1-opw-578235-rgo)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches