From 1fdfe31b1ef073f507dfba3ebfcee66942652a3d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicolas=20=C3=89vrard?= <nicoe@b2ck.com>
Date: Sun, 29 Jan 2023 01:12:24 +0100
Subject: [PATCH] Get directly the string value of Selection fields in non
 editable views

Closes #10331
---
 sao/CHANGELOG                                 |  1 +
 sao/src/model.js                              | 16 +++-
 sao/src/view/tree.js                          | 81 +++++++++++--------
 tryton/CHANGELOG                              |  1 +
 tryton/tryton/common/selection.py             |  2 +
 .../gui/window/view_form/model/record.py      | 20 +++--
 .../window/view_form/view/list_gtk/widget.py  | 23 ++++--
 trytond/CHANGELOG                             |  1 +
 trytond/doc/ref/fields.rst                    | 26 ++++++
 trytond/doc/ref/models.rst                    |  4 +
 trytond/trytond/model/fields/selection.py     | 43 ++++++----
 trytond/trytond/model/modelsql.py             | 20 ++++-
 .../tests/test_field_multiselection.py        | 39 +++++++++
 trytond/trytond/tests/test_field_selection.py | 39 +++++++++
 14 files changed, 250 insertions(+), 66 deletions(-)

diff --git a/sao/CHANGELOG b/sao/CHANGELOG
index 09d2019b4d..c8487c996a 100644
--- a/sao/CHANGELOG
+++ b/sao/CHANGELOG
@@ -1,3 +1,4 @@
+* Eagerly read string value of selection fields
 * Do not select first record by default
 * Use ``&`` and ``|`` as boolean operator in search widget
 * Use toggle button and label on translate dialog
diff --git a/sao/src/model.js b/sao/src/model.js
index 8a9c4b832f..593108aa52 100644
--- a/sao/src/model.js
+++ b/sao/src/model.js
@@ -671,6 +671,10 @@
                 var fdescription = this.model.fields[fname].description;
                 if (~rec_named_fields.indexOf(fdescription.type))
                     fnames_to_fetch.push(fname + '.rec_name');
+                else if ((fdescription.type == 'selection') &&
+                        ((fdescription.loading || 'lazy') == 'eager')) {
+                    fnames_to_fetch.push(fname + ':string');
+                }
             }
             if (!~fnames.indexOf('rec_name')) {
                 fnames_to_fetch.push('rec_name');
@@ -830,10 +834,16 @@
                 if (this.model.fields[name] instanceof Sao.field.One2Many) {
                     later[name] = value;
                 }
-                if ((this.model.fields[name] instanceof Sao.field.Many2One) ||
-                        (this.model.fields[name] instanceof Sao.field.Reference)) {
-                    var related = name + '.';
+                const field = this.model.fields[name];
+                var related;
+                if ((field instanceof Sao.field.Many2One) ||
+                        (field instanceof Sao.field.Reference)) {
+                    related = name + '.';
                     this._values[related] = values[related] || {};
+                } else if ((field instanceof Sao.field.Selection) &&
+                        (name + ':string' in values)){
+                    related = name + ':string';
+                    this._values[related] = values[related];
                 }
                 this.model.fields[name].set(this, value);
                 this._loaded[name] = true;
diff --git a/sao/src/view/tree.js b/sao/src/view/tree.js
index 47ba61261f..74d86c9a5d 100644
--- a/sao/src/view/tree.js
+++ b/sao/src/view/tree.js
@@ -2529,38 +2529,49 @@
             Sao.View.Tree.SelectionColumn._super.init.call(this, model,
                 attributes);
             Sao.common.selection_mixin.init.call(this);
-            this.init_selection();
+            if (!this.tree || this.tree.editable) {
+                this.init_selection();
+            }
         },
         init_selection: function(key) {
             Sao.common.selection_mixin.init_selection.call(this, key);
         },
         update_selection: function(record, callback) {
+            if (!this.selection) {
+                this.init_selection();
+            }
             Sao.common.selection_mixin.update_selection.call(this, record,
                 this.field, callback);
         },
         update_text: function(cell, record) {
-            this.update_selection(record, () => {
-                var value = this.field.get(record);
-                var prm, text, found = false;
-                for (const option of this.selection) {
-                    if (option[0] === value) {
-                        found = true;
-                        text = option[1];
-                        break;
+            if (!this.tree.editable &&
+                    (this.field.name + ':string' in record._values)) {
+                var text_value = record._values[this.field.name + ':string'];
+                cell.text(text_value).attr('title', text_value);
+            } else {
+                this.update_selection(record, () => {
+                    var value = this.field.get(record);
+                    var prm, text, found = false;
+                    for (const option of this.selection) {
+                        if (option[0] === value) {
+                            found = true;
+                            text = option[1];
+                            break;
+                        }
                     }
-                }
-                if (!found) {
-                    prm = Sao.common.selection_mixin.get_inactive_selection
-                        .call(this, value).then(function(inactive) {
-                            return inactive[1];
-                        });
-                } else {
-                    prm = jQuery.when(text);
-                }
-                prm.done(text_value => {
-                    cell.text(text_value).attr('title', text_value);
+                    if (!found) {
+                        prm = Sao.common.selection_mixin.get_inactive_selection
+                            .call(this, value).then(function(inactive) {
+                                return inactive[1];
+                            });
+                    } else {
+                        prm = jQuery.when(text);
+                    }
+                    prm.done(text_value => {
+                        cell.text(text_value).attr('title', text_value);
+                    });
                 });
-            });
+            }
         }
     });
 
@@ -2580,18 +2591,24 @@
                 this.field, callback);
         },
         update_text: function(cell, record) {
-            this.update_selection(record, () => {
-                var values = this.field.get_eval(record).map(value => {
-                    for (const option of this.selection) {
-                        if (option[0] === value) {
-                            return option[1];
-                        }
-                    }
-                    return '';
-                });
-                var text_value = values.join(';');
+            if (!this.tree.editable &&
+                    (this.field_name + ':string' in record._value)) {
+                var text_value = record._values[this.field_name + ':string'];
                 cell.text(text_value).attr('title', text_value);
-            });
+            } else {
+                this.update_selection(record, () => {
+                    var values = this.field.get_eval(record).map(value => {
+                        for (const option of this.selection) {
+                            if (option[0] === value) {
+                                return option[1];
+                            }
+                        }
+                        return '';
+                    });
+                    var text_value = values.join(';');
+                    cell.text(text_value).attr('title', text_value);
+                });
+            }
         },
     });
 
diff --git a/tryton/CHANGELOG b/tryton/CHANGELOG
index bcda2cd403..bb64077982 100644
--- a/tryton/CHANGELOG
+++ b/tryton/CHANGELOG
@@ -1,3 +1,4 @@
+* Eagerly read string value of selection fields
 * Do not select first record by default
 * Use ``&`` and ``|`` as boolean operator in search widget
 * Use toggle button and label on translate dialog
diff --git a/tryton/tryton/common/selection.py b/tryton/tryton/common/selection.py
index 8412b45077..b3ee916ff3 100644
--- a/tryton/tryton/common/selection.py
+++ b/tryton/tryton/common/selection.py
@@ -48,6 +48,8 @@ class SelectionMixin(object):
     def update_selection(self, record, field):
         if not field:
             return
+        if not self.selection:
+            self.init_selection()
 
         domain = field.domain_get(record)
         if 'relation' not in self.attrs:
diff --git a/tryton/tryton/gui/window/view_form/model/record.py b/tryton/tryton/gui/window/view_form/model/record.py
index 7eb9aaf3c1..a92cd0bcae 100644
--- a/tryton/tryton/gui/window/view_form/model/record.py
+++ b/tryton/tryton/gui/window/view_form/model/record.py
@@ -74,9 +74,13 @@ class Record:
             fnames = [fname for fname, field in fields
                 if fname not in self._loaded
                 and (not views or (views & field.views))]
-            fnames.extend(('%s.rec_name' % fname for fname in fnames[:]
-                    if self.group.fields[fname].attrs['type']
-                    in ('many2one', 'one2one', 'reference')))
+            for fname in list(fnames):
+                f_attrs = self.group.fields[fname].attrs
+                if f_attrs['type'] in {'many2one', 'one2one', 'reference'}:
+                    fnames.append('%s.rec_name' % fname)
+                elif (f_attrs['type'] == 'selection'
+                        and f_attrs.get('loading', 'lazy') == 'eager'):
+                    fnames.append('%s:string' % fname)
             if 'rec_name' not in fnames:
                 fnames.append('rec_name')
             fnames.extend(['_timestamp', '_write', '_delete'])
@@ -464,13 +468,17 @@ class Record:
                 if fieldname == 'rec_name':
                     self.value['rec_name'] = value
                 continue
-            if isinstance(self.group.fields[fieldname], fields.O2MField):
+            field = self.group.fields[fieldname]
+            if isinstance(field, fields.O2MField):
                 later[fieldname] = value
                 continue
-            if isinstance(self.group.fields[fieldname], (fields.M2OField,
-                        fields.ReferenceField)):
+            if isinstance(field, (fields.M2OField, fields.ReferenceField)):
                 related = fieldname + '.'
                 self.value[related] = val.get(related) or {}
+            elif (isinstance(field, fields.SelectionField)
+                    and fieldname + ':string' in val):
+                related = fieldname + ':string'
+                self.value[related] = val[related]
             self.group.fields[fieldname].set(self, value)
             self._loaded.add(fieldname)
             fieldnames.append(fieldname)
diff --git a/tryton/tryton/gui/window/view_form/view/list_gtk/widget.py b/tryton/tryton/gui/window/view_form/view/list_gtk/widget.py
index e3b2d48c7d..fe151f5c24 100644
--- a/tryton/tryton/gui/window/view_form/view/list_gtk/widget.py
+++ b/tryton/tryton/gui/window/view_form/view/list_gtk/widget.py
@@ -1044,16 +1044,22 @@ class Selection(GenericText, SelectionMixin, PopdownMixin):
         if 'renderer' not in kwargs:
             kwargs['renderer'] = CellRendererCombo
         super(Selection, self).__init__(*args, **kwargs)
-        self.init_selection()
-        # Use a variable let Python holding reference when calling set_property
-        model = self.get_popdown_model(self.selection)[0]
-        self.renderer.set_property('model', model)
-        self.renderer.set_property('text-column', 0)
+        if self.view and self.view.editable:
+            self.init_selection()
+            # Use a variable let Python holding reference when calling
+            # set_property
+            model = self.get_popdown_model(self.selection)[0]
+            self.renderer.set_property('model', model)
+            self.renderer.set_property('text-column', 0)
 
     def get_value(self, record, field):
         return field.get(record)
 
     def get_textual_value(self, record):
+        related = self.attrs['name'] + ':string'
+        if not self.view.editable and related in record.value:
+            return record.value[related]
+
         field = record[self.attrs['name']]
         self.update_selection(record, field)
         value = self.get_value(record, field)
@@ -1118,7 +1124,8 @@ class MultiSelection(GenericText, SelectionMixin):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        self.init_selection()
+        if self.view and self.view.editable:
+            self.init_selection()
 
     @realized
     @CellCache.cache
@@ -1131,6 +1138,10 @@ class MultiSelection(GenericText, SelectionMixin):
             callback()
 
     def get_textual_value(self, record):
+        related = self.attrs['name'] + ':string'
+        if not self.view.editable and related in record.value:
+            return ";".join(record.value[related])
+
         field = record[self.attrs['name']]
         self.update_selection(record, field)
         selection = dict(self.selection)
diff --git a/trytond/CHANGELOG b/trytond/CHANGELOG
index 251c90c6e9..a69264fb71 100644
--- a/trytond/CHANGELOG
+++ b/trytond/CHANGELOG
@@ -1,3 +1,4 @@
+* Support reading string value of selection fields
 * Add ButtonActionException and RPCReturnException
 * Rename ModelStorage.count into ModelStorage.estimated_count
 * Add border type to images
diff --git a/trytond/doc/ref/fields.rst b/trytond/doc/ref/fields.rst
index a9d0e4673c..01a9d64d27 100644
--- a/trytond/doc/ref/fields.rst
+++ b/trytond/doc/ref/fields.rst
@@ -589,6 +589,22 @@ Selection
 
    A dictionary mapping the selection value with its help string.
 
+Class methods:
+
+.. classmethod:: Selection.get_selection(model, name, inst)
+
+   Returns a :py:class`dictionary <dict>` mapping the selection value to its
+   human-readable value.
+
+.. classmethod:: Selection.get_selection_string(selection, value)
+
+   Returns the human-readable form of ``value`` in regard to ``selection``.
+   ``selection`` is acquired thanks to :meth:`Selection.get_selection`.
+   ..note::
+   This method should be used instead of relying on dictionary access
+   because this method take into account the internal representation of the
+   ``Selection`` value.
+
 Instance methods:
 
 .. method:: Selection.translated([name])
@@ -624,6 +640,16 @@ MultiSelection
 
    Same as :attr:`Selection.help_selection`.
 
+Class methods:
+
+.. classmethod:: MultiSelection.get_selection(model, name, inst)
+
+   Same as :meth:`Selection.get_selection`
+
+.. classmethod:: MultiSelection.get_selection_string(selection, value)
+
+   Same as :meth:`Selection.get_selection_string`
+
 Instance methods:
 
 .. method:: MultiSelection.translated([name])
diff --git a/trytond/doc/ref/models.rst b/trytond/doc/ref/models.rst
index 5bdb44c5ce..d3c8d0253e 100644
--- a/trytond/doc/ref/models.rst
+++ b/trytond/doc/ref/models.rst
@@ -296,6 +296,10 @@ Class methods:
    ``.``.
    The number of *dots* in the name is not limited.
 
+   ``fields_names`` can also contain ``<field>:string`` for
+   :class:`~fields.Selection` or :class:`~fields.MultiSelection` fields.
+   Their human-readable value are returned.
+
    The virtual fields ``_write`` and ``_delete`` can be used the read the
    writeable and deleteable state of the records.
    Regarding the ``_timestamp`` virtual fields it contains a timestamp that is
diff --git a/trytond/trytond/model/fields/selection.py b/trytond/trytond/model/fields/selection.py
index 8230f77da8..db8b0cfce2 100644
--- a/trytond/trytond/model/fields/selection.py
+++ b/trytond/trytond/model/fields/selection.py
@@ -71,6 +71,27 @@ class SelectionMixin(Field):
                     (name, 'selection', language, source))
         return super().definition_translations(model, language) + selection
 
+    @classmethod
+    def get_selection(cls, model, name, inst):
+        selection = model.fields_get([name])[name]['selection']
+        if isinstance(selection, str):
+            sel_func = getattr(model, selection)
+            if not is_instance_method(model, selection):
+                selection = sel_func()
+            else:
+                selection = sel_func(inst)
+        return dict(selection)
+
+    @classmethod
+    def get_selection_string(cls, selection, value):
+        # None and '' are equivalent
+        if value is None or value == '':
+            if value not in selection:
+                switch_value = {None: '', '': None}[value]
+                if switch_value in selection:
+                    value = switch_value
+        return selection[value]
+
 
 class Selection(SelectionMixin, Field):
     '''
@@ -157,29 +178,17 @@ class TranslatedSelection(object):
         from ..model import Model
         if inst is None:
             return self
+        field = cls._fields[self.name]
         with Transaction().set_context(getattr(inst, '_context', {})):
-            selection = cls.fields_get([self.name])[self.name]['selection']
-            if not isinstance(selection, (tuple, list)):
-                sel_func = getattr(cls, selection)
-                if not is_instance_method(cls, selection):
-                    selection = sel_func()
-                else:
-                    selection = sel_func(inst)
-            selection = dict(selection)
+            selection = field.get_selection(cls, self.name, inst)
         value = getattr(inst, self.name)
-        # None and '' are equivalent
-        if value is None or value == '':
-            if value not in selection:
-                switch_value = {None: '', '': None}[value]
-                if switch_value in selection:
-                    value = switch_value
         # Use Model __name__ for Reference field
-        elif isinstance(value, Model):
+        if isinstance(value, Model):
             value = value.__name__
         if isinstance(value, (list, tuple)):
             values = []
             for item in value:
-                values.append(selection.get(item, item))
+                values.append(field.get_selection_string(selection, item))
             return values
         else:
-            return selection.get(value, value)
+            return field.get_selection_string(selection, value)
diff --git a/trytond/trytond/model/modelsql.py b/trytond/trytond/model/modelsql.py
index 21a3c5aec0..eb977043bd 100644
--- a/trytond/trytond/model/modelsql.py
+++ b/trytond/trytond/model/modelsql.py
@@ -899,6 +899,9 @@ class ModelSQL(ModelStorage):
             if '.' in field_name:
                 field_name, field_related = field_name.split('.', 1)
                 fields_related[field_name].add(field_related)
+            if field_name.endswith(':string'):
+                field_name = field_name[:-len(':string')]
+                fields_related[field_name]
             field = cls._fields[field_name]
             if hasattr(field, 'datetime_field') and field.datetime_field:
                 extra_fields.add(field.datetime_field)
@@ -1116,6 +1119,17 @@ class ModelSQL(ModelStorage):
                     for target in row[name]:
                         if target is not None:
                             values.append(targets[target])
+            elif field._type in {'selection', 'multiselection'}:
+                key = name + ':string'
+                for row, target in zip(rows, targets):
+                    selection = field.get_selection(cls, name, target)
+                    if field._type == 'selection':
+                        row[key] = field.get_selection_string(
+                            selection, row[name])
+                    else:
+                        row[key] = [
+                            field.get_selection_string(selection, v)
+                            for v in row[name]]
             else:
                 for row in rows:
                     value = row[name]
@@ -1147,7 +1161,9 @@ class ModelSQL(ModelStorage):
                     ctx.update(PYSONDecoder(row).decode(pyson_context))
                 if datetime_field:
                     ctx['_datetime'] = row.get(datetime_field)
-                if field._type == 'reference':
+                if field._type in {'selection', 'multiselection'}:
+                    Target = None
+                elif field._type == 'reference':
                     value = row[fname]
                     if not value:
                         Target = None
@@ -1171,7 +1187,7 @@ class ModelSQL(ModelStorage):
                             field, Target, rows, list(fields_related[fname]))
                         targets = {t['id']: t for t in targets}
                     else:
-                        targets = {}
+                        targets = cls.browse([r['id'] for r in rows])
                     add_related(field, rows, targets)
 
         for row, field in product(result, to_del):
diff --git a/trytond/trytond/tests/test_field_multiselection.py b/trytond/trytond/tests/test_field_multiselection.py
index 11b77997af..881d59ecda 100644
--- a/trytond/trytond/tests/test_field_multiselection.py
+++ b/trytond/trytond/tests/test_field_multiselection.py
@@ -319,6 +319,45 @@ class FieldMultiSelectionTestCase(unittest.TestCase):
         self.assertEqual(foo_baz, [])
         self.assertEqual(empty, [selection])
 
+    @with_transaction()
+    def test_read_string(self):
+        "Test reading value and string"
+        Selection = Pool().get('test.multi_selection')
+        foo, foo_bar, null = Selection.create([{
+                    'selects': ['foo'],
+                    }, {
+                    'selects': ['foo', 'bar'],
+                    }, {
+                    'selects': [],
+                    }])
+
+        foo_read, foo_bar_read, null_read = Selection.read(
+            [foo.id, foo_bar.id, null.id], ['selects:string'])
+
+        self.assertEqual(foo_read['selects:string'], ["Foo"])
+        self.assertEqual(foo_bar_read['selects:string'], ["Bar", "Foo"])
+        self.assertEqual(null_read['selects:string'], [])
+
+    @with_transaction()
+    def test_read_string_dynamic_selection(self):
+        "Test reading value and string of a dynamic selection"
+        Selection = Pool().get('test.multi_selection')
+        foo, bar, null = Selection.create([{
+                    'selects': ['foo'],
+                    'dyn_selects': ['foo', 'foobar'],
+                    }, {
+                    'dyn_selects': ['bar', 'baz'],
+                    }, {
+                    'dyn_selects': [],
+                    }])
+
+        foo_read, bar_read, null_read = Selection.read(
+            [foo.id, bar.id, null.id], ['dyn_selects:string'])
+
+        self.assertEqual(foo_read['dyn_selects:string'], ["Foo", "FooBar"])
+        self.assertEqual(bar_read['dyn_selects:string'], ["Bar", "Baz"])
+        self.assertEqual(null_read['dyn_selects:string'], [])
+
 
 @unittest.skipIf(
     backend.name != 'postgresql', 'jsonb only supported by postgresql')
diff --git a/trytond/trytond/tests/test_field_selection.py b/trytond/trytond/tests/test_field_selection.py
index 898d82f800..ab891cf6e0 100644
--- a/trytond/trytond/tests/test_field_selection.py
+++ b/trytond/trytond/tests/test_field_selection.py
@@ -242,3 +242,42 @@ class FieldSelectionTestCase(unittest.TestCase):
                     }])
 
         self.assertEqual(selection.dyn_select_static_string, '1')
+
+    @with_transaction()
+    def test_read_string(self):
+        "Test reading value and string"
+        Selection = Pool().get('test.selection')
+
+        arabic, null = Selection.create([
+                {
+                    'select': 'arabic',
+                    },
+                {
+                    'select': None,
+                    },
+                ])
+        arabic_read, null_read = Selection.read(
+            [arabic.id, null.id], ['select:string'])
+
+        self.assertEqual(arabic_read['select:string'], 'Arabic')
+        self.assertEqual(null_read['select:string'], '')
+
+    @with_transaction()
+    def test_read_string_dynamic_selection(self):
+        "Test reading value and string of dynamic selection"
+        Selection = Pool().get('test.selection')
+
+        one, null = Selection.create([
+                {
+                    'select': 'arabic',
+                    'dyn_select': '1'
+                    },
+                {
+                    'select': None,
+                    },
+                ])
+        one_read, null_read = Selection.read(
+            [one.id, null.id], ['dyn_select:string'])
+
+        self.assertEqual(one_read['dyn_select:string'], '1')
+        self.assertEqual(null_read['dyn_select:string'], '')