Code review comment for lp:~openerp-dev/openobject-addons/7.0-opw-591889-msh

Revision history for this message
Mohammed Shekha(Open ERP) (msh-openerp) wrote :

Hello Thibault,

Thanks for the review.

>>In mail_thread.message_post(), we want to attach manually added attachments (having mail.compose.message model) to the message's document (having in our case a False model).As we could have 'free' attachments in the 7.0 modeling of the communication, being able to write False as res_model is required.

Would you please explain me why we should have model free attachment ?, if it should be then I have one scenario.
Install document module, now compose message from Compose New Message button, now go to Settings -> Email -> Messages.
See you last message you just send, see there are no attachments available for this document, so for this res_model and res_id are required which we will get from context which I passed here, we already passed context in other places, open full mail composer through corner button("open full mail composer"), here context is passed res_model and res_id set according to context in ir.attachment, also tyr without full mail composer post your message with attachment context comes in send_mail.

If this is the case that we should set res_model and res_id to False then why we have passed default_model and default_res_id from action context, see mail_thread_view.xml.
one more thing I didn't get here why default_res_model is res.users, and default_res_id is uid, according to me res_model should be mail.message and res_id should be active_id, is ther any other use of this default_model and default_res_id ?

According to me context should be set in action called from mail.js and in mail_thread_view.xml default_res_model and default_res_id should be mail.message and active_id respectively, so when you post any message you can attahcment from header in Settings -> Email -> Mesages.

No doubt fix of revision 4991 in server will hide traceback but according to me actual problem is due to context.

Correct me if I am wrong.

Hope this will help.

Thanks.

review: Needs Resubmitting

« Back to merge proposal