cms_form: go ahead w/ test cov 100%

cms_form/__init__.py

@@ -1,2 +1,4 @@
-from . import models
-from . import controllers
+# not sure why these lines are counted as not test covered
+# since nothing would work in tests w/out them being loaded
+from . import models  # pragma: no cover
+from . import controllers  # pragma: no cover

cms_form/controllers/main.py

@@ -34,10 +34,6 @@ def get_render_values(self, form, **kw):
         You can override this to inject more values.
         """
         main_object = form.main_object
-        parent = None
-        if getattr(main_object, 'parent_id', None):
-            # get the parent if any
-            parent = main_object.parent_id
         # Cleanup render values and remove form fields' values.
         # When you submit a form and there's an error odoo will give you back
         # all submitted values into `kw` but:
@@ -52,7 +48,6 @@ def get_render_values(self, form, **kw):
         vals.update({
             'form': form,
             'main_object': main_object,
-            'parent': parent,
             'controller': self,
         })
         return vals

cms_form/models/cms_form.py

@@ -1,7 +1,6 @@
 # Copyright 2017-2018 Simone Orsi
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html).
 
-import werkzeug
 from psycopg2 import IntegrityError
 
 from odoo import models, exceptions, _
@@ -90,18 +89,10 @@ def form_cancel_url(self, main_object=None):
             return main_object.website_url
         return self.request.referrer or '/'
 
-    def form_check_empty_field(self, fname, field, value, **req_values):
+    def form_check_empty_value(self, fname, field, value, **req_values):
         """Return True if passed field value is really empty."""
-        if isinstance(value, werkzeug.datastructures.FileStorage):
-            has_value = bool(value.filename)
-            if not has_value and req_values.get(fname + '_keepcheck') == 'yes':
-                # no value, but we want to preserve existing one
-                return False
-            # file field w/ no content
-            # TODO: this is not working sometime...
-            # return not bool(value.content_length)
-            return not has_value
-        return value in (False, '')
+        # delegate to each specific widget
+        return field['widget'].w_check_empty_value(value, **req_values)
 
     def form_validate(self, request_values=None):
         """Validate submitted values."""
@@ -113,9 +104,8 @@ def form_validate(self, request_values=None):
         for fname, field in self.form_fields().items():
             value = request_values.get(fname)
             error = False
-            if field['required'] \
-                    and self.form_check_empty_field(
-                        fname, field, value, **request_values):
+            if field['required'] and self.form_check_empty_value(
+                    fname, field, value, **request_values):
                 errors[fname] = 'missing'
                 missing = True
             validator = self.form_get_validator(fname, field)

cms_form/models/cms_form_mixin.py

@@ -520,8 +520,7 @@ def form_process(self, **kw):
         render_values.update(kw)
         render_values['form_data'] = self.form_load_defaults()
         handler = getattr(self, 'form_process_' + self.request.method.upper())
-        render_values.update(handler(render_values))
-        self.form_render_values = render_values
+        self.form_render_values = dict(render_values, **handler(render_values))
 
     def form_process_GET(self, render_values):
         """Process GET requests."""

cms_form/models/cms_form_wizard.py

@@ -110,7 +110,7 @@ def wiz_get_step_info(self, step):
         try:
             return self.wiz_configure_steps()[step]
         except KeyError:
-            raise ValueError('Step `%d` does not exists.' % step)
+            raise ValueError('Step `%s` does not exists.' % str(step))
 
     def wiz_current_step(self):
         return self.wiz_storage_get().get('current') or 1
@@ -142,6 +142,7 @@ def wiz_save_step(self, values, step=None):
         step = step or self.wiz_current_step()
         storage = self.wiz_storage_get()
         if step not in storage['steps']:
+            # safely re-init step
             storage['steps'][step] = {}
         storage['steps'][step].update(values)
 

cms_form/models/cms_search_form.py

@@ -27,8 +27,12 @@ class CMSFormSearch(models.AbstractModel):
     _form_search_fields_multi = ()
     # declare custom domain computation rules
     _form_search_domain_rules = {
-        # field name: (leaf field name, operator, format value),
-        # 'product_id': ('product_id.name', 'ilike', '{}'),
+        # opt 1: `field name: (leaf field name, operator, format value)`
+        # `format_value` is a formatting compatible string
+        # 'product_id': ('product_id.name', 'ilike', '{}')
+
+        # opt 2: function that give back `(fname, operator, value)``
+        # 'foo': lambda field, value, search_values: ('foo', 'not like', value)
     }
 
     def form_check_permission(self):
@@ -81,15 +85,7 @@ def form_search(self, render_values):
         domain = self.form_search_domain(search_values)
         count = self.form_model.search_count(domain)
         page = render_values.get('extra_args', {}).get('page', 0)
-        url = render_values.get('extra_args', {}).get('pager_url', '')
-        if self._form_model:
-            url = getattr(self.form_model, 'cms_search_url', url)
-        if not url:
-            # default to current path w/out paging
-            path = self.request.path
-            if not isinstance(path, pycompat.text_type):
-                path = path.decode('utf-8')
-            url = path.split('/page')[0]
+        url = self._form_get_url_for_pager(render_values)
         pager = self._form_results_pager(count=count, page=page, url=url)
         order = self._form_results_orderby or None
         results = self.form_model.search(
@@ -105,6 +101,17 @@ def form_search(self, render_values):
         }
         return self.form_search_results
 
+    def _form_get_url_for_pager(self, render_values):
+        # default to current path w/out paging
+        path = pycompat.to_text(self.request.path)
+        url = path.split('/page')[0]
+        if self._form_model:
+            # rely on model's cms search url
+            url = getattr(self.form_model, 'cms_search_url', None) or url
+        # override via controller/request specific value
+        url = render_values.get('extra_args', {}).get('pager_url', url)
+        return url
+
     def pager(self, **kw):
         return self.env['website'].pager(**kw)
 
@@ -147,23 +154,18 @@ def form_search_domain(self, search_values):
                 if not value:
                     continue
                 operator = 'in'
-            elif field['type'] in ('many2one', ):
-                value = int(value) if value.isdigit() else 0
-                if not value or value < 1:
-                    # we need an existing ID here ( > 0)
-                    continue
             elif field['type'] in ('boolean', ):
                 value = value == 'on' and True
             elif field['type'] in ('date', 'datetime'):
                 if not value:
                     # searching for an empty string breaks search
                     continue
             if fname in self._form_search_domain_rules:
-                fname, operator, fmt_value = \
-                    self._form_search_domain_rules[fname]
-                if hasattr(fmt_value, '__call__'):
-                    value = fmt_value(field, value, search_values)
+                rule = self._form_search_domain_rules[fname]
+                if hasattr(rule, '__call__'):
+                    fname, operator, value = rule(field, value, search_values)
                 else:
+                    fname, operator, fmt_value = rule
                     value = fmt_value.format(value) if fmt_value else value
             leaf = (fname, operator, value)
             domain.append(leaf)

cms_form/models/widgets/widget_binary.py

@@ -73,6 +73,19 @@ def form_to_binary(self, value, **req_values):
                 _value = value.split(',')[-1]
         return _value
 
+    def w_check_empty_value(self, value, **req_values):
+        if isinstance(value, werkzeug.datastructures.FileStorage):
+            has_value = bool(value.filename)
+            keep_flag = req_values.get(self.w_fname + '_keepcheck')
+            if not has_value and keep_flag == 'yes':
+                # no value, but we want to preserve existing one
+                return False
+            # file field w/ no content
+            # TODO: this is not working sometime...
+            # return not bool(value.content_length)
+            return not has_value
+        return super().w_check_empty_value(value, **req_values)
+
 
 class ImageWidget(models.AbstractModel):
     _name = 'cms.form.widget.image'

cms_form/models/widgets/widget_mixin.py

@@ -43,7 +43,7 @@ def w_css_klass(self):
     def w_load(self, **req_values):
         """Load value for current field in current request."""
         value = self.w_field.get('_default')
-        # we could have form-only fields (like `custom` in test form below)
+        # we could have form-only fields
         if self.w_record and self.w_fname in self.w_record:
             value = self.w_record[self.w_fname] or value
         # maybe a POST request with new values: override item value
@@ -54,6 +54,10 @@ def w_extract(self, **req_values):
         """Extract value from form submit."""
         return req_values.get(self.w_fname)
 
+    def w_check_empty_value(self, value, **req_values):
+        # `None` values are meant to be ignored as not changed
+        return value in (False, '')
+
     @staticmethod
     def w_ids_from_input(value):
         """Convert list of ids from form input."""

cms_form/tests/common.py

@@ -1,7 +1,6 @@
 # Copyright 2017-2018 Simone Orsi
 # License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html).
 
-
 from lxml import html
 from odoo.tests.common import SavepointCase, HttpCase
 from .utils import (
@@ -114,3 +113,6 @@ def tearDown(self):
     def html_get(self, url):
         resp = self.url_open(url, timeout=30)
         return html.document_fromstring(resp.content)
+
+    def get_form(self, form_model, **kw):
+        return get_form(self.env, form_model, **kw)

cms_form/tests/fake_models.py

@@ -68,6 +68,16 @@ def form_widgets(self):
         return res
 
 
+class FakePartnerChannelForm(models.AbstractModel):
+    """A test model form."""
+
+    _name = 'cms.form.mail.channel.partner'
+    _inherit = 'cms.form'
+    # This model has `_rec_name = 'partner_id'` and allows us
+    # to test a specific case for form_title computation
+    _form_model = 'mail.channel.partner'
+
+
 class FakeFieldsForm(models.AbstractModel):
     """A test model form."""
 
@@ -193,16 +203,33 @@ class FakeWizStep3Partner(models.AbstractModel):
 
 
 # `AbstractModel` or `TransientModel` needed to make ACL check happy`
-class FakePublishModel(models.TransientModel):
+class FakePubModel(models.TransientModel):
     _name = 'fake.publishable'
     _inherit = [
         'website.published.mixin',
     ]
     name = fields.Char()
 
+    def _compute_website_url(self):
+        for item in self:
+            item.website_url = '/publishable/%d' % item.id
+
 
-class FakePublishModelForm(models.AbstractModel):
+class FakePubModelForm(models.AbstractModel):
     _name = 'cms.form.fake.publishable'
     _inherit = 'cms.form'
     _form_model = 'fake.publishable'
     _form_model_fields = ('name', )
+
+
+# `AbstractModel` or `TransientModel` needed to make ACL check happy`
+class FakeNonPubModel(models.TransientModel):
+    _name = 'fake.non.publishable'
+    name = fields.Char()
+
+
+class FakeNonPubModelForm(models.AbstractModel):
+    _name = 'cms.form.fake.non.publishable'
+    _inherit = 'cms.form'
+    _form_model = 'fake.non.publishable'
+    _form_model_fields = ('name', )

cms_form/tests/test_controllers.py

@@ -30,24 +30,87 @@ def setUp(self):
         self.authenticate('admin', 'admin')
 
     @contextmanager
-    def mock_assets(self):
+    def mock_assets(self, req=None):
         """Mocks some stuff like request."""
         with mock.patch('%s.request' % IMPORT) as request:
-            faked = fake_request()
+            faked = req or fake_request()
             request.session = self.session
             request.env = self.env
             request.httprequest = faked.httprequest
             yield {
                 'request': request,
             }
 
+    def test_get_template(self):
+        with self.mock_assets():
+            form = self.form_controller.get_form('res.partner')
+            # default
+            self.assertEqual(
+                self.form_controller.get_template(form),
+                'cms_form.form_wrapper'
+            )
+            # custom on form
+            form.form_wrapper_template = 'foo.baz'
+            self.assertEqual(
+                self.form_controller.get_template(form),
+                'foo.baz'
+            )
+            self.form_controller.template = None
+            form.form_wrapper_template = None
+            with self.assertRaises(NotImplementedError):
+                self.form_controller.get_template(form)
+
+    def test_get_render_values(self):
+        with self.mock_assets():
+            form = self.form_controller.get_form('res.partner')
+            # default, no main object
+            self.assertEqual(
+                self.form_controller.get_render_values(form),
+                {
+                    'form': form,
+                    'main_object': self.env['res.partner'],
+                    'controller': self.form_controller,
+                }
+            )
+            # get a main obj
+            partner = self.env.ref('base.res_partner_12')
+            form = self.form_controller.get_form(
+                'res.partner', model_id=partner.id)
+            self.assertEqual(
+                self.form_controller.get_render_values(form),
+                {
+                    'form': form,
+                    'main_object': partner,
+                    'controller': self.form_controller,
+                }
+            )
+            # strip out form fields values (they are held by the form itself)
+            self.assertEqual(
+                self.form_controller.get_render_values(
+                    form, name='John', custom='foo', not_a_form_field=1),
+                {
+                    'form': form,
+                    'main_object': partner,
+                    'controller': self.form_controller,
+                    'not_a_form_field': 1
+                }
+            )
+
     def test_get_no_form(self):
         with self.mock_assets():
             # we do not have a specific form for res.groups
             # and cms form is not enabled on partner model
             with self.assertRaises(NotImplementedError):
                 self.form_controller.get_form('res.groups')
 
+    def test_get_form_no_model_no_main_object(self):
+        with self.mock_assets():
+            form = self.form_controller.get_form(
+                None, form_model_key=FakePartnerForm._name)
+            self.assertEqual(
+                form.main_object, self.env[FakePartnerForm._name]
+            )
+
     def test_get_default_form(self):
         with self.mock_assets():
             # we have one for res.partner
@@ -80,6 +143,22 @@ def test_get_wizard_form(self):
             self.assertEqual(form._form_model, 'res.partner')
             self.assertEqual(form.form_mode, 'create')
 
+    def test_redirect_after_success(self):
+        req = fake_request(
+            form_data={'name': 'John'},
+            method='POST',
+        )
+        with self.mock_assets(req=req):
+            partner = self.env.ref('base.res_partner_12')
+            response = self.form_controller.make_response(
+                'res.partner', model_id=partner.id)
+            self.assertEqual(response.status_code, 303)
+            if 'website_url' in partner:
+                # website_partner installed
+                self.assertEqual(response.location, partner.website_url)
+            else:
+                self.assertEqual(response.location, '/')
+
     def _check_rendering(self, dom, form_model, model, mode, extra_klass=''):
         """Check default markup for form and form wrapper."""
         # test wrapper klass