pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[REF] mail: cleanup 'mail.tracking.value' model code

Browse Source 
PURPOSE

Simplify 'mail.tracking.value' model and code. Remove unnecessary fields and
computation. Make code easier to handle and more batch-enabled.

SPECIFICATIONS

SPEC 1: prepare all tracking values at same code place

Delegate all computation of tracking values into the 'create_tracking_values'
method. Curently part of it (currency field) is done in the caller. Better
split code per feature.

Method is also made private, as it is not required to expose it.

SPEC 2: cleanup tracking value formatting methods and calls

Code used to display tracking value can be simplified: remove unnecessary
wrappers, make code easier to read, avoid composition of field name but
use a mapping instead (easier to grep 'old_value_char' when it is effectively
used).

Ensure code always calls '_tracking_value_format' to ease future improvements
and have all formatting code being batch-enabled.

SPEC 3: simplify Chatter formatted value structure

'fieldType' is currently added in old and new values. As it is a field
property it can be moved higher in the formatting result to be included
only once. Also add 'fieldName', the column name, to the formatted results
as it will soon help various tool methods. Moreover it makes sense to have
the source of the tracking as the real column name, in addition to its
string and type.

Task-3345979 (Mail: Simplify tracking model)

Part-of: odoo/odoo#124182
This commit is contained in:
Thibault Delavallée
2023-10-06 06:13:54 +00:00
parent c40053bc28
commit 11289fd128
12 changed files with 150 additions and 99 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/l10n_in/models/mail_message.py
+11 -4
View File
@@ -23,14 +23,21 @@ class Message(models.Model):
elif not title and message.subtype_id and not message.subtype_id.internal:
title = message.subtype_id.display_name
audit_log_preview = Markup("<div>%s</div>") % (title)
for value in tracking_value_ids:
trackings = [
(
fmt_vals['changedField'],
fmt_vals['oldValue']['value'],
fmt_vals['newValue']['value'],
) for fmt_vals in tracking_value_ids._tracking_value_format()
]
for field_desc, old_value, new_value in trackings:
audit_log_preview += Markup(
"<li>%(old_value)s <i class='o_TrackingValue_separator fa fa-long-arrow-right mx-1 text-600' title='%(title)s' role='img' aria-label='%(title)s'></i>%(new_value)s (%(field)s)</li>"
) % {
'old_value': value._get_old_display_value()[0] or _("None"),
'new_value': value._get_new_display_value()[0] or _("None"),
'old_value': old_value,
'new_value': new_value,
'title': _("Changed"),
'field': value.field.field_description,
'field': field_desc,
}
message.l10n_in_audit_log_preview = audit_log_preview
addons/mail/__manifest__.py
+1 -1
View File
@@ -71,7 +71,7 @@ For more specific needs, you may also assign custom-defined actions
'wizard/mail_template_reset_views.xml',
'views/fetchmail_views.xml',
'views/mail_message_subtype_views.xml',
'views/mail_tracking_views.xml',
'views/mail_tracking_value_views.xml',
'views/mail_notification_views.xml',
'views/mail_message_views.xml',
'views/mail_message_schedule_views.xml',
addons/mail/models/mail_message.py
+2 -2
View File
@@ -989,14 +989,14 @@ class Message(models.Model):
{
'changedField': "Customer",
'id': 2965,
'fieldName': 'partner_id',
'fieldType': 'char',
'newValue': {
'currencyId': "",
'fieldType': 'char',
'value': "Axelor",
],
'oldValue': {
'currencyId': "",
'fieldType': 'char',
'value': "",
],
}
addons/mail/models/mail_thread.py
+15 -8
View File
@@ -3333,15 +3333,22 @@ class MailThread(models.AbstractModel):
)
record_name = msg_vals.get('record_name') if 'record_name' in msg_vals else message.record_name
# tracking
# tracking: in case of missing value, perform search (skip only if sure we don't have any)
check_tracking = msg_vals.get('tracking_value_ids', True) if msg_vals else bool(self)
tracking = []
if msg_vals.get('tracking_value_ids', True) if msg_vals else bool(self): # could be tracking
for tracking_value in self.env['mail.tracking.value'].sudo().search([('mail_message_id', '=', message.id)]):
groups = tracking_value.field_groups
if not groups or self.env.is_superuser() or self.user_has_groups(groups):
tracking.append((tracking_value.field_desc,
tracking_value._get_old_display_value()[0],
tracking_value._get_new_display_value()[0]))
if check_tracking:
tracking_values = self.env['mail.tracking.value'].sudo().search(
[('mail_message_id', '=', message.id)]
).filtered(
lambda track: not track.field_groups or self.env.is_superuser() or self.user_has_groups(track.field_groups)
)
tracking = [
(
fmt_vals['changedField'],
fmt_vals['oldValue']['value'],
fmt_vals['newValue']['value'],
) for fmt_vals in tracking_values._tracking_value_format()
]
subtype_id = msg_vals.get('subtype_id') if msg_vals and 'subtype_id' in msg_vals else message.subtype_id.id
is_discussion = subtype_id == self.env['ir.model.data']._xmlid_to_res_id('mail.mt_comment')
addons/mail/models/mail_tracking_value.py
+79 -45
View File
@@ -46,10 +46,26 @@ class MailTracking(models.Model):
tracking.field_groups = field.groups if field else 'base.group_system'
@api.model
def create_tracking_values(self, initial_value, new_value, col_name, col_info, tracking_sequence, model_name):
def _create_tracking_values(self, initial_value, new_value, col_name, col_info, tracking_sequence, record):
""" Prepare values to create a mail.tracking.value. It prepares old and
new value according to the field type.
:param initial_value: field value before the change, could be text, int,
date, datetime, ...;
:param new_value: field value after the change, could be text, int,
date, datetime, ...;
:param str col_name: technical field name, column name (e.g. 'user_id);
:param dict col_info: result of fields_get(col_name);
:param int tracking_sequence: sequence used for ordering tracking
value display;
:param <record> record: record on which tracking is performed, used for
related computation e.g. finding currency of monetary fields;
:return: a dict values valid for 'mail.tracking.value' creation;
"""
tracked = True
field = self.env['ir.model.fields']._get(model_name, col_name)
field = self.env['ir.model.fields']._get(record._name, col_name)
if not field:
return
@@ -57,9 +73,11 @@ class MailTracking(models.Model):
if col_info['type'] in ['integer', 'float', 'char', 'text', 'datetime', 'monetary']:
values.update({
'old_value_%s' % col_info['type']: initial_value,
'new_value_%s' % col_info['type']: new_value
f'old_value_{col_info["type"]}': initial_value,
f'new_value_{col_info["type"]}': new_value
})
if col_info['type'] == 'monetary':
values['currency_id'] = record[col_info['currency_field']].id
elif col_info['type'] == 'date':
values.update({
'old_value_datetime': initial_value and fields.Datetime.to_string(datetime.combine(fields.Date.from_string(initial_value), datetime.min.time())) or False,
@@ -95,50 +113,66 @@ class MailTracking(models.Model):
return {}
def _tracking_value_format(self):
tracking_values = [{
'changedField': tracking.field_desc,
'id': tracking.id,
'newValue': {
'currencyId': tracking.currency_id.id,
'fieldType': tracking.field_type,
'value': tracking._get_new_display_value()[0],
},
'oldValue': {
'currencyId': tracking.currency_id.id,
'fieldType': tracking.field_type,
'value': tracking._get_old_display_value()[0],
},
} for tracking in self]
return tracking_values
""" Return structure and formatted data structure to be used by chatter
to display tracking values.
:return list: for each tracking value in self, their formatted display
values given as a dict;
"""
formatted = []
for tracking in self:
formatted.append({
'changedField': tracking.field_desc,
'id': tracking.id,
'fieldName': tracking.field.name,
'fieldType': tracking.field_type,
'newValue': {
'currencyId': tracking.currency_id.id,
'value': tracking._format_display_value(new=True)[0],
},
'oldValue': {
'currencyId': tracking.currency_id.id,
'value': tracking._format_display_value(new=False)[0],
},
})
return formatted
def _format_display_value(self, new=True):
""" Format value of 'mail.tracking.value', according to the field type.
:param bool new: if True, display the 'new' value. Otherwise display
the 'old' one.
"""
field_mapping = {
'boolean': ('old_value_integer', 'new_value_integer'),
'date': ('old_value_datetime', 'new_value_datetime'),
'datetime': ('old_value_datetime', 'new_value_datetime'),
'char': ('old_value_char', 'new_value_char'),
'float': ('old_value_float', 'new_value_float'),
'integer': ('old_value_integer', 'new_value_integer'),
'monetary': ('old_value_monetary', 'new_value_monetary'),
'text': ('old_value_text', 'new_value_text'),
}
def _get_display_value(self, prefix):
assert prefix in ('new', 'old')
result = []
for record in self:
if record.field_type in ['integer', 'float', 'char', 'text', 'monetary']:
result.append(record[f'{prefix}_value_{record.field_type}'])
elif record.field_type == 'datetime':
if record[f'{prefix}_value_datetime']:
new_datetime = record[f'{prefix}_value_datetime']
result.append(f'{new_datetime}Z')
ftype = record.field_type
value_fname = field_mapping.get(
ftype, ('old_value_char', 'new_value_char')
)[bool(new)]
value = record[value_fname]
if ftype in {'integer', 'float', 'char', 'text', 'monetary'}:
result.append(value)
elif ftype in {'date', 'datetime'}:
if not record[value_fname]:
result.append(value)
elif ftype == 'date':
result.append(fields.Date.to_string(value))
else:
result.append(record[f'{prefix}_value_datetime'])
elif record.field_type == 'date':
if record[f'{prefix}_value_datetime']:
new_date = record[f'{prefix}_value_datetime']
result.append(fields.Date.to_string(new_date))
else:
result.append(record[f'{prefix}_value_datetime'])
elif record.field_type == 'boolean':
result.append(bool(record[f'{prefix}_value_integer']))
result.append(f'{value}Z')
elif ftype == 'boolean':
result.append(bool(value))
else:
result.append(record[f'{prefix}_value_char'])
result.append(value)
return result
def _get_old_display_value(self):
# grep : # old_value_integer | old_value_datetime | old_value_char
return self._get_display_value('old')
def _get_new_display_value(self):
# grep : # new_value_integer | new_value_datetime | new_value_char
return self._get_display_value('new')
addons/mail/models/models.py
+13 -9
View File
@@ -71,13 +71,14 @@ class BaseModel(models.AbstractModel):
# GENERIC MAIL FEATURES
# ------------------------------------------------------------
def _mail_track(self, tracked_fields, initial):
def _mail_track(self, tracked_fields, initial_values):
""" For a given record, fields to check (tuple column name, column info)
and initial values, return a valid command to create tracking values.
:param tracked_fields: fields_get of updated fields on which tracking
is checked and performed;
:param initial: dict of initial values for each updated fields;
:param dict tracked_fields: fields_get of updated fields on which
tracking is checked and performed;
:param dict initial_values: dict of initial values for each updated
fields;
:return: a tuple (changes, tracking_value_ids) where
changes: set of updated column names;
@@ -92,9 +93,9 @@ class BaseModel(models.AbstractModel):
# generate tracked_values data structure: {'col_name': {col_info, new_value, old_value}}
for col_name, col_info in tracked_fields.items():
if col_name not in initial:
if col_name not in initial_values:
continue
initial_value = initial[col_name]
initial_value = initial_values[col_name]
new_value = self[col_name]
if new_value != initial_value and (new_value or initial_value): # because browse null != False
@@ -102,10 +103,13 @@ class BaseModel(models.AbstractModel):
getattr(self._fields[col_name], 'track_sequence', 100)) # backward compatibility with old parameter name
if tracking_sequence is True:
tracking_sequence = 100
tracking = self.env['mail.tracking.value'].create_tracking_values(initial_value, new_value, col_name, col_info, tracking_sequence, self._name)
tracking = self.env['mail.tracking.value']._create_tracking_values(
initial_value, new_value,
col_name, col_info,
tracking_sequence,
self
)
if tracking:
if tracking['field_type'] == 'monetary':
tracking['currency_id'] = self[col_info['currency_field']].id
tracking_value_ids.append([0, 0, tracking])
changes.add(col_name)
addons/mail/static/src/core/web/message_patch.js
+4 -10
View File
@@ -54,14 +54,8 @@ patch(Message.prototype, {
/**
* @returns {string}
*/
formatTracking(trackingValue) {
/**
* Maps tracked field type to a JS formatter. Tracking values are
* not always stored in the same field type as their origin type.
* Field types that are not listed here are not supported by
* tracking in Python. Also see `create_tracking_values` in Python.
*/
switch (trackingValue.fieldType) {
formatTracking(trackingType, trackingValue) {
switch (trackingType) {
case "boolean":
return trackingValue.value ? _t("Yes") : _t("No");
/**
@@ -106,8 +100,8 @@ patch(Message.prototype, {
/**
* @returns {string}
*/
formatTrackingOrNone(trackingValue) {
const formattedValue = this.formatTracking(trackingValue);
formatTrackingOrNone(trackingType, trackingValue) {
const formattedValue = this.formatTracking(trackingType, trackingValue);
return formattedValue || _t("None");
},
});
addons/mail/static/src/core/web/message_patch.xml
+2 -2
View File
@@ -13,9 +13,9 @@
<ul class="mb-0 ps-4">
<t name="trackingValues" t-foreach="message.trackingValues" t-as="trackingValue" t-key="trackingValue.id">
<li class="o-mail-Message-tracking mb-1" role="group">
<span class="o-mail-Message-trackingOld me-1 px-1 text-muted fw-bold" t-esc="formatTrackingOrNone(trackingValue.oldValue)"/>
<span class="o-mail-Message-trackingOld me-1 px-1 text-muted fw-bold" t-esc="formatTrackingOrNone(trackingValue.fieldType, trackingValue.oldValue)"/>
<i class="o-mail-Message-trackingSeparator fa fa-long-arrow-right mx-1 text-600"/>
<span class="o-mail-Message-trackingNew me-1 fw-bold text-info" t-esc="formatTrackingOrNone(trackingValue.newValue)"/>
<span class="o-mail-Message-trackingNew me-1 fw-bold text-info" t-esc="formatTrackingOrNone(trackingValue.fieldType, trackingValue.newValue)"/>
<span class="o-mail-Message-trackingField ms-1 fst-italic text-muted">(<t t-esc="trackingValue.changedField"/>)</span>
</li>
</t>
addons/mail/static/tests/helpers/mock_server/models/mail_tracking_value.js
+19 -14
View File
@@ -31,7 +31,7 @@ patch(MockServer.prototype, {
return mockWriteResult;
},
/**
* Simulates `create_tracking_values` on `mail.tracking.value`
* Simulates `_create_tracking_values` on `mail.tracking.value`
*/
_mockMailTrackingValue_CreateTrackingValues(
initialValue,
@@ -100,24 +100,29 @@ patch(MockServer.prototype, {
* Simulates `_tracking_value_format` on `mail.tracking.value`
*/
_mockMailTrackingValue_TrackingValueFormat(tracking_value_ids) {
const trackingValues = tracking_value_ids.map((tracking) => ({
changedField: tracking.field_desc,
id: tracking.id,
newValue: {
const trackingValues = tracking_value_ids.map((tracking) => {
const irField = this.models["ir.model.fields"].records.find(
(field) => field.id === tracking.field
);
return {
changedField: tracking.field_desc,
id: tracking.id,
fieldName: irField.name,
fieldType: tracking.field_type,
value: this._mockMailTrackingValue_GetDisplayValue(tracking, "new"),
},
oldValue: {
fieldType: tracking.field_type,
value: this._mockMailTrackingValue_GetDisplayValue(tracking, "old"),
},
}));
newValue: {
value: this._mockMailTrackingValue_FormatDisplayValue(tracking, "new"),
},
oldValue: {
value: this._mockMailTrackingValue_FormatDisplayValue(tracking, "old"),
},
};
});
return trackingValues;
},
/**
* Simulates `_get_display_value` on `mail.tracking.value`
* Simulates `_format_display_value` on `mail.tracking.value`
*/
_mockMailTrackingValue_GetDisplayValue(record, type) {
_mockMailTrackingValue_FormatDisplayValue(record, type) {
switch (record.field_type) {
case "float":
case "integer":
addons/mail/tests/test_res_partner.py
+2 -2
View File
@@ -105,9 +105,9 @@ class TestPartner(MailCommon):
self.assertEqual(len(change_messages), 1)
tracking_values = change_messages.tracking_value_ids
self.assertIn(f'{self.env.company.name}, Some Street Name, Some City Name CA 94134, United States',
tracking_values._get_old_display_value())
tracking_values.old_value_char)
self.assertIn(f'{self.env.company.name}, Some Other Street Name, Some Other City Name CA 94134, United States',
tracking_values._get_new_display_value())
tracking_values.new_value_char)
# none of the address fields are logged at the same time
self.assertEqual(set(), set(partner._address_fields()) & set(tracking_values.sudo().field.mapped('name')))
addons/mail/views/mail_tracking_views.xml → addons/mail/views/mail_tracking_value_views.xml
View File
addons/test_mail/tests/test_message_track.py
+2 -2
View File
@@ -546,14 +546,14 @@ class TestTrackingInternals(MailCommon):
formattedTrackingValues = [{
'changedField': 'Email From',
'id': tracking_values[0]['id'],
'fieldName': 'email_from',
'fieldType': 'char',
'newValue': {
'currencyId': False,
'fieldType': 'char',
'value': 'X',
},
'oldValue': {
'currencyId': False,
'fieldType': 'char',
'value': False,
},
}]