From bf4ada34b4e6bc9793f4870786d579d9dce4c799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 27 Jun 2013 16:46:47 +0200 Subject: [PATCH] [REF] mail_thread: message_track now uses browse records instead of read results. Updated addons accordingly. Updated tests to test corner cases using False / null browse records. bzr revid: tde@openerp.com-20130627144647-swbj77i84vo9ii0v --- addons/account/account_invoice.py | 4 +-- addons/analytic/analytic.py | 6 ++--- addons/crm/crm_lead.py | 8 +++--- addons/hr_expense/hr_expense.py | 6 ++--- addons/hr_holidays/hr_holidays.py | 6 ++--- addons/hr_recruitment/hr_recruitment.py | 6 ++--- addons/mail/mail_thread.py | 35 ++++++++++++++----------- addons/mail/tests/test_mail_features.py | 33 ++++++++++++++++++----- addons/project/project.py | 10 +++---- addons/project_issue/project_issue.py | 12 ++++----- addons/purchase/purchase.py | 6 ++--- addons/sale/sale.py | 4 +-- 12 files changed, 80 insertions(+), 56 deletions(-) diff --git a/addons/account/account_invoice.py b/addons/account/account_invoice.py index 0fb6af05e58..87e6a66740c 100644 --- a/addons/account/account_invoice.py +++ b/addons/account/account_invoice.py @@ -221,8 +221,8 @@ class account_invoice(osv.osv): 'type': { }, 'state': { - 'account.mt_invoice_paid': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'paid' and obj['type'] in ('out_invoice', 'out_refund'), - 'account.mt_invoice_validated': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'open' and obj['type'] in ('out_invoice', 'out_refund'), + 'account.mt_invoice_paid': lambda self, cr, uid, obj, ctx=None: obj.state == 'paid' and obj.type in ('out_invoice', 'out_refund'), + 'account.mt_invoice_validated': lambda self, cr, uid, obj, ctx=None: obj.state == 'open' and obj.type in ('out_invoice', 'out_refund'), }, } _columns = { diff --git a/addons/analytic/analytic.py b/addons/analytic/analytic.py index ce79c4d5ffb..df852181709 100644 --- a/addons/analytic/analytic.py +++ b/addons/analytic/analytic.py @@ -33,9 +33,9 @@ class account_analytic_account(osv.osv): _description = 'Analytic Account' _track = { 'state': { - 'analytic.mt_account_pending': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'pending', - 'analytic.mt_account_closed': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'close', - 'analytic.mt_account_opened': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'open', + 'analytic.mt_account_pending': lambda self, cr, uid, obj, ctx=None: obj.state == 'pending', + 'analytic.mt_account_closed': lambda self, cr, uid, obj, ctx=None: obj.state == 'close', + 'analytic.mt_account_opened': lambda self, cr, uid, obj, ctx=None: obj.state == 'open', }, } diff --git a/addons/crm/crm_lead.py b/addons/crm/crm_lead.py index 44ada2ffcc0..1070af22b3c 100644 --- a/addons/crm/crm_lead.py +++ b/addons/crm/crm_lead.py @@ -77,12 +77,12 @@ class crm_lead(base_stage, format_address, osv.osv): _track = { 'state': { - 'crm.mt_lead_create': lambda self, cr, uid, obj, ctx=None: obj['state'] in ['new', 'draft'], - 'crm.mt_lead_won': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'done', - 'crm.mt_lead_lost': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'cancel', + 'crm.mt_lead_create': lambda self, cr, uid, obj, ctx=None: obj.state in ['new', 'draft'], + 'crm.mt_lead_won': lambda self, cr, uid, obj, ctx=None: obj.state == 'done', + 'crm.mt_lead_lost': lambda self, cr, uid, obj, ctx=None: obj.state == 'cancel', }, 'stage_id': { - 'crm.mt_lead_stage': lambda self, cr, uid, obj, ctx=None: obj['state'] not in ['new', 'draft', 'cancel', 'done'], + 'crm.mt_lead_stage': lambda self, cr, uid, obj, ctx=None: obj.state not in ['new', 'draft', 'cancel', 'done'], }, } diff --git a/addons/hr_expense/hr_expense.py b/addons/hr_expense/hr_expense.py index 8d707baa50e..5d5be0eec97 100644 --- a/addons/hr_expense/hr_expense.py +++ b/addons/hr_expense/hr_expense.py @@ -65,9 +65,9 @@ class hr_expense_expense(osv.osv): _order = "id desc" _track = { 'state': { - 'hr_expense.mt_expense_approved': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'accepted', - 'hr_expense.mt_expense_refused': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'cancelled', - 'hr_expense.mt_expense_confirmed': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'confirm', + 'hr_expense.mt_expense_approved': lambda self, cr, uid, obj, ctx=None: obj.state == 'accepted', + 'hr_expense.mt_expense_refused': lambda self, cr, uid, obj, ctx=None: obj.state == 'cancelled', + 'hr_expense.mt_expense_confirmed': lambda self, cr, uid, obj, ctx=None: obj.state == 'confirm', }, } diff --git a/addons/hr_holidays/hr_holidays.py b/addons/hr_holidays/hr_holidays.py index 0eb3c0cbf16..f2be4844d27 100644 --- a/addons/hr_holidays/hr_holidays.py +++ b/addons/hr_holidays/hr_holidays.py @@ -110,9 +110,9 @@ class hr_holidays(osv.osv): _inherit = ['mail.thread', 'ir.needaction_mixin'] _track = { 'state': { - 'hr_holidays.mt_holidays_approved': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'validate', - 'hr_holidays.mt_holidays_refused': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'refuse', - 'hr_holidays.mt_holidays_confirmed': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'confirm', + 'hr_holidays.mt_holidays_approved': lambda self, cr, uid, obj, ctx=None: obj.state == 'validate', + 'hr_holidays.mt_holidays_refused': lambda self, cr, uid, obj, ctx=None: obj.state == 'refuse', + 'hr_holidays.mt_holidays_confirmed': lambda self, cr, uid, obj, ctx=None: obj.state == 'confirm', }, } diff --git a/addons/hr_recruitment/hr_recruitment.py b/addons/hr_recruitment/hr_recruitment.py index c25583cba77..d8c00c40a31 100644 --- a/addons/hr_recruitment/hr_recruitment.py +++ b/addons/hr_recruitment/hr_recruitment.py @@ -94,11 +94,11 @@ class hr_applicant(base_stage, osv.Model): _inherit = ['mail.thread', 'ir.needaction_mixin'] _track = { 'state': { - 'hr_recruitment.mt_applicant_hired': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'done', - 'hr_recruitment.mt_applicant_refused': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'cancel', + 'hr_recruitment.mt_applicant_hired': lambda self, cr, uid, obj, ctx=None: obj.state == 'done', + 'hr_recruitment.mt_applicant_refused': lambda self, cr, uid, obj, ctx=None: obj.state == 'cancel', }, 'stage_id': { - 'hr_recruitment.mt_stage_changed': lambda self, cr, uid, obj, ctx=None: obj['state'] not in ['done', 'cancel'], + 'hr_recruitment.mt_stage_changed': lambda self, cr, uid, obj, ctx=None: obj.state not in ['done', 'cancel'], }, } diff --git a/addons/mail/mail_thread.py b/addons/mail/mail_thread.py index 403cfab7d61..23196925b63 100644 --- a/addons/mail/mail_thread.py +++ b/addons/mail/mail_thread.py @@ -328,8 +328,8 @@ class mail_thread(osv.AbstractModel): # Track initial values of tracked fields tracked_fields = self._get_tracked_fields(cr, uid, values.keys(), context=context) if tracked_fields: - initial = self.read(cr, uid, ids, tracked_fields.keys(), context=context) - initial_values = dict((item['id'], item) for item in initial) + records = self.browse(cr, uid, ids, context=context) + initial_values = dict((this.id, dict((key, getattr(this, key)) for key in tracked_fields.keys())) for this in records) # Perform write, update followers result = super(mail_thread, self).write(cr, uid, ids, values, context=context) @@ -388,7 +388,7 @@ class mail_thread(osv.AbstractModel): if not value: return '' if col_info['type'] == 'many2one': - return value[1] + return value.name_get()[0][1] if col_info['type'] == 'selection': return dict(col_info['selection'])[value] return value @@ -407,23 +407,28 @@ class mail_thread(osv.AbstractModel): if not tracked_fields: return True - for record in self.read(cr, uid, ids, tracked_fields.keys(), context=context): - initial = initial_values[record['id']] - changes = [] + for browse_record in self.browse(cr, uid, ids, context=context): + initial = initial_values[browse_record.id] + changes = set() tracked_values = {} # generate tracked_values data structure: {'col_name': {col_info, new_value, old_value}} for col_name, col_info in tracked_fields.items(): - if record[col_name] == initial[col_name] and getattr(self._all_columns[col_name].column, 'track_visibility', None) == 'always': + initial_value = initial[col_name] + record_value = getattr(browse_record, col_name) + + if record_value == initial_value and getattr(self._all_columns[col_name].column, 'track_visibility', None) == 'always': tracked_values[col_name] = dict(col_info=col_info['string'], - new_value=convert_for_display(record[col_name], col_info)) - elif record[col_name] != initial[col_name]: + new_value=convert_for_display(record_value, col_info)) + elif record_value != initial_value: + if col_info['type'] == 'many2one' and not (record_value or initial_value): + continue if getattr(self._all_columns[col_name].column, 'track_visibility', None) in ['always', 'onchange']: tracked_values[col_name] = dict(col_info=col_info['string'], - old_value=convert_for_display(initial[col_name], col_info), - new_value=convert_for_display(record[col_name], col_info)) + old_value=convert_for_display(initial_value, col_info), + new_value=convert_for_display(record_value, col_info)) if col_name in tracked_fields: - changes.append(col_name) + changes.add(col_name) if not changes: continue @@ -433,7 +438,7 @@ class mail_thread(osv.AbstractModel): if field not in changes: continue for subtype, method in track_info.items(): - if method(self, cr, uid, record, context): + if method(self, cr, uid, browse_record, context): subtypes.append(subtype) posted = False @@ -444,11 +449,11 @@ class mail_thread(osv.AbstractModel): _logger.debug('subtype %s not found, giving error "%s"' % (subtype, e)) continue message = format_message(subtype_rec.description if subtype_rec.description else subtype_rec.name, tracked_values) - self.message_post(cr, uid, record['id'], body=message, subtype=subtype, context=context) + self.message_post(cr, uid, browse_record.id, body=message, subtype=subtype, context=context) posted = True if not posted: message = format_message('', tracked_values) - self.message_post(cr, uid, record['id'], body=message, context=context) + self.message_post(cr, uid, browse_record.id, body=message, context=context) return True #------------------------------------------------------ diff --git a/addons/mail/tests/test_mail_features.py b/addons/mail/tests/test_mail_features.py index 91ede8b8449..148b5faba87 100644 --- a/addons/mail/tests/test_mail_features.py +++ b/addons/mail/tests/test_mail_features.py @@ -739,18 +739,21 @@ class test_mail(TestMailBase): self.ir_model_data.create(cr, uid, {'name': 'mt_private', 'model': 'mail.message.subtype', 'module': 'mail', 'res_id': mt_private_id}) mt_name_supername_id = self.mail_message_subtype.create(cr, uid, {'name': 'name_supername', 'description': 'Supername name'}) self.ir_model_data.create(cr, uid, {'name': 'mt_name_supername', 'model': 'mail.message.subtype', 'module': 'mail', 'res_id': mt_name_supername_id}) + mt_group_public_set_id = self.mail_message_subtype.create(cr, uid, {'name': 'group_public_set', 'description': 'Group set'}) + self.ir_model_data.create(cr, uid, {'name': 'mt_group_public_set', 'model': 'mail.message.subtype', 'module': 'mail', 'res_id': mt_group_public_set_id}) mt_group_public_id = self.mail_message_subtype.create(cr, uid, {'name': 'group_public', 'description': 'Group changed'}) self.ir_model_data.create(cr, uid, {'name': 'mt_group_public', 'model': 'mail.message.subtype', 'module': 'mail', 'res_id': mt_group_public_id}) # Data: alter mail_group model for testing purposes (test on classic, selection and many2one fields) self.mail_group._track = { 'public': { - 'mail.mt_private': lambda self, cr, uid, obj, ctx=None: obj['public'] == 'private', + 'mail.mt_private': lambda self, cr, uid, obj, ctx=None: obj.public == 'private', }, 'name': { - 'mail.mt_name_supername': lambda self, cr, uid, obj, ctx=None: obj['name'] == 'supername', + 'mail.mt_name_supername': lambda self, cr, uid, obj, ctx=None: obj.name == 'supername', }, 'group_public_id': { + 'mail.mt_group_public_set': lambda self, cr, uid, obj, ctx=None: obj.group_public_id, 'mail.mt_group_public': lambda self, cr, uid, obj, ctx=None: True, }, } @@ -787,21 +790,37 @@ class test_mail(TestMailBase): self.assertIn(u'Public\u2192Private', _strip_string_spaces(last_msg.body), 'tracked: message body incorrect') self.assertIn(u'Pigs\u2192supername', _strip_string_spaces(last_msg.body), 'tracked feature: message body does not hold always tracked field') - # Test: change public as public, group_public_id -> 1 subtype, name always tracked + # Test: change public as public, group_public_id -> 2 subtypes, name always tracked self.mail_group.write(cr, self.user_raoul_id, [self.group_pigs_id], {'public': 'public', 'group_public_id': group_system_id}) self.group_pigs.refresh() - self.assertEqual(len(self.group_pigs.message_ids), 4, 'tracked: one message should have been produced') - # Test: first produced message: mt_group_public_id, with name always tracked, public tracked on change + self.assertEqual(len(self.group_pigs.message_ids), 5, 'tracked: one message should have been produced') + # Test: first produced message: mt_group_public_set_id, with name always tracked, public tracked on change last_msg = self.group_pigs.message_ids[-4] - self.assertEqual(last_msg.subtype_id.id, mt_group_public_id, 'tracked: message should not be linked to any subtype') + self.assertEqual(last_msg.subtype_id.id, mt_group_public_set_id, 'tracked: message should be linked to mt_group_public_set_id') + self.assertIn('Group set', last_msg.body, 'tracked: message body does not hold the subtype description') + self.assertIn(u'Private\u2192Public', _strip_string_spaces(last_msg.body), 'tracked: message body does not hold changed tracked field') + self.assertIn(u'HumanResources/Employee\u2192Administration/Settings', _strip_string_spaces(last_msg.body), 'tracked: message body does not hold always tracked field') + # Test: second produced message: mt_group_public_id, with name always tracked, public tracked on change + last_msg = self.group_pigs.message_ids[-5] + self.assertEqual(last_msg.subtype_id.id, mt_group_public_id, 'tracked: message should be linked to mt_group_public_id') self.assertIn('Group changed', last_msg.body, 'tracked: message body does not hold the subtype description') self.assertIn(u'Private\u2192Public', _strip_string_spaces(last_msg.body), 'tracked: message body does not hold changed tracked field') self.assertIn(u'HumanResources/Employee\u2192Administration/Settings', _strip_string_spaces(last_msg.body), 'tracked: message body does not hold always tracked field') + # Test: change group_public_id to False -> 1 subtype, name always tracked + self.mail_group.write(cr, self.user_raoul_id, [self.group_pigs_id], {'group_public_id': False}) + self.group_pigs.refresh() + self.assertEqual(len(self.group_pigs.message_ids), 6, 'tracked: one message should have been produced') + # Test: first produced message: mt_group_public_set_id, with name always tracked, public tracked on change + last_msg = self.group_pigs.message_ids[-6] + self.assertEqual(last_msg.subtype_id.id, mt_group_public_id, 'tracked: message should be linked to mt_group_public_id') + self.assertIn('Group changed', last_msg.body, 'tracked: message body does not hold the subtype description') + self.assertIn(u'Administration/Settings\u2192', _strip_string_spaces(last_msg.body), 'tracked: message body does not hold always tracked field') + # Test: change not tracked field, no tracking message self.mail_group.write(cr, self.user_raoul_id, [self.group_pigs_id], {'description': 'Dummy'}) self.group_pigs.refresh() - self.assertEqual(len(self.group_pigs.message_ids), 4, 'tracked: No message should have been produced') + self.assertEqual(len(self.group_pigs.message_ids), 6, 'tracked: No message should have been produced') # Data: removed changes public_col.track_visibility = None diff --git a/addons/project/project.py b/addons/project/project.py index cadc4d8afb8..587d60ff86f 100644 --- a/addons/project/project.py +++ b/addons/project/project.py @@ -574,15 +574,15 @@ class task(base_stage, osv.osv): _track = { 'state': { - 'project.mt_task_new': lambda self, cr, uid, obj, ctx=None: obj['state'] in ['new', 'draft'], - 'project.mt_task_started': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'open', - 'project.mt_task_closed': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'done', + 'project.mt_task_new': lambda self, cr, uid, obj, ctx=None: obj.state in ['new', 'draft'], + 'project.mt_task_started': lambda self, cr, uid, obj, ctx=None: obj.state == 'open', + 'project.mt_task_closed': lambda self, cr, uid, obj, ctx=None: obj.state == 'done', }, 'stage_id': { - 'project.mt_task_stage': lambda self, cr, uid, obj, ctx=None: obj['state'] not in ['new', 'draft', 'done', 'open'], + 'project.mt_task_stage': lambda self, cr, uid, obj, ctx=None: obj.state not in ['new', 'draft', 'done', 'open'], }, 'kanban_state': { # kanban state: tracked, but only block subtype - 'project.mt_task_blocked': lambda self, cr, uid, obj, ctx=None: obj['kanban_state'] == 'blocked', + 'project.mt_task_blocked': lambda self, cr, uid, obj, ctx=None: obj.kanban_state == 'blocked', }, } diff --git a/addons/project_issue/project_issue.py b/addons/project_issue/project_issue.py index 33e4fcb409c..a69d9356002 100644 --- a/addons/project_issue/project_issue.py +++ b/addons/project_issue/project_issue.py @@ -50,15 +50,15 @@ class project_issue(base_stage, osv.osv): _track = { 'state': { - 'project_issue.mt_issue_new': lambda self, cr, uid, obj, ctx=None: obj['state'] in ['new', 'draft'], - 'project_issue.mt_issue_closed': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'done', - 'project_issue.mt_issue_started': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'open', + 'project_issue.mt_issue_new': lambda self, cr, uid, obj, ctx=None: obj.state in ['new', 'draft'], + 'project_issue.mt_issue_closed': lambda self, cr, uid, obj, ctx=None: obj.state == 'done', + 'project_issue.mt_issue_started': lambda self, cr, uid, obj, ctx=None: obj.state == 'open', }, 'stage_id': { - 'project_issue.mt_issue_stage': lambda self, cr, uid, obj, ctx=None: obj['state'] not in ['new', 'draft', 'done', 'open'], + 'project_issue.mt_issue_stage': lambda self, cr, uid, obj, ctx=None: obj.state not in ['new', 'draft', 'done', 'open'], }, 'kanban_state': { - 'project_issue.mt_issue_blocked': lambda self, cr, uid, obj, ctx=None: obj['kanban_state'] == 'blocked', + 'project_issue.mt_issue_blocked': lambda self, cr, uid, obj, ctx=None: obj.kanban_state == 'blocked', }, } @@ -578,7 +578,7 @@ class project_issue(base_stage, osv.osv): if context is None: context = {} res = super(project_issue, self).message_post(cr, uid, thread_id, body=body, subject=subject, type=type, subtype=subtype, parent_id=parent_id, attachments=attachments, context=context, content_subtype=content_subtype, **kwargs) - if thread_id: + if thread_id and subtype: self.write(cr, SUPERUSER_ID, thread_id, {'date_action_last': time.strftime(tools.DEFAULT_SERVER_DATETIME_FORMAT)}, context=context) return res diff --git a/addons/purchase/purchase.py b/addons/purchase/purchase.py index 707012fc063..77151026765 100644 --- a/addons/purchase/purchase.py +++ b/addons/purchase/purchase.py @@ -162,9 +162,9 @@ class purchase_order(osv.osv): ] _track = { 'state': { - 'purchase.mt_rfq_confirmed': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'confirmed', - 'purchase.mt_rfq_approved': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'approved', - 'purchase.mt_rfq_done': lambda self, cr, uid, obj, ctx=None: obj['state'] == 'done', + 'purchase.mt_rfq_confirmed': lambda self, cr, uid, obj, ctx=None: obj.state == 'confirmed', + 'purchase.mt_rfq_approved': lambda self, cr, uid, obj, ctx=None: obj.state == 'approved', + 'purchase.mt_rfq_done': lambda self, cr, uid, obj, ctx=None: obj.state == 'done', }, } _columns = { diff --git a/addons/sale/sale.py b/addons/sale/sale.py index 67c6f82ced3..55bd34009bf 100644 --- a/addons/sale/sale.py +++ b/addons/sale/sale.py @@ -34,8 +34,8 @@ class sale_order(osv.osv): _description = "Sales Order" _track = { 'state': { - 'sale.mt_order_confirmed': lambda self, cr, uid, obj, ctx=None: obj['state'] in ['manual'], - 'sale.mt_order_sent': lambda self, cr, uid, obj, ctx=None: obj['state'] in ['sent'] + 'sale.mt_order_confirmed': lambda self, cr, uid, obj, ctx=None: obj.state in ['manual'], + 'sale.mt_order_sent': lambda self, cr, uid, obj, ctx=None: obj.state in ['sent'] }, }