Merge lp:~akretion-team/server-env-tools/web-context-tunnel into lp:~server-env-tools-core-editors/server-env-tools/7.0

[Raphaël Valyi - http://www.akretion.com]

please see the module description in the __openerp__.py file.

I would like to see if OCA could not use that to avoid changing on_change signatures in incompatible ways for the OCA modules.

Eager to look after your comments.

[lp:~akretion-team/server-env-tools/web-context-tunnel]

53. By Raphaël Valyi - http://www.akretion.com on 2013-12-11

[web_context_tunnel] more explicit usage description

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

To help for the review, it's probably helpful to mention that the original build_context method is defined this way:
https://github.com/akretion/openerp-web/blob/ocb-7.0/addons/web/static/src/js/view_form.js#L1866

Also, I'm redefining build_context on the prototype directly (instead of using .extend), because we want to alter the build_context of all widget built upon the instance.web.form.FormWidget prototype.

Notice that my module totally preserves the original behavior if a widget simply has a context attribute with no extra context* extensions.

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

could you please:

* wrap the lines in __openerp__.py
* line 28-29: fix the newline
* line 39: s/Ad/And/

review: Needs Fixing (code review, no test)

[lp:~akretion-team/server-env-tools/web-context-tunnel]

54. By Raphaël Valyi - http://www.akretion.com on 2013-12-11

[web_context_tunnel] wrapped description and fixed typo

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

@Alexandre Fayolle

Done. How is it now? Thanks.

[Alexis de Lattre (alexis-via)]

This is a great workaround for a very very annoying problem of OpenERP : 2 modules inherit the same on_change to add arguments and it makes the 2 modules incompatible. Almost all experienced OpenERP developers were already confronted to this headache. This seems a clever and lightweight solution.

I am not a JS developer so I can't give my opinion on the code itself, but it approve this from an OpenERP module developer point of view.

review: Approve

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

It could deserve a test.

You should remove the console.log()

What about the yaml tests? They use the views to apply automatically the onchanges, so the tests will fail because they won't take the extra attributes in account.

review: Needs Fixing

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

Hello Guewen,

sorry about the console.log, removing it.
You made an interesting point about the on_change.
In fact only tests testing modules with the extra fields may fail, not the existing base tests.

Are there so many of such tests? (but yeah lack of test is not a terribly good reason anyway).
Eventually, in OCB, we could modularize the on_change test player so this module have a chance to alter it properly. Then our module will keep being compatible with the commercial branch but at least tests will be properly executed in the OCB branch? Not sure, just thinking...

[lp:~akretion-team/server-env-tools/web-context-tunnel]

55. By Raphaël Valyi - http://www.akretion.com on 2013-12-26

removing trash console.log

56. By Raphaël Valyi - http://www.akretion.com on 2013-12-26

adding unit test and explanation about how to run the unit test and also about how to run YAML tests of module using web_context_tunnel

[Raphaël Valyi - http://www.akretion.com (rvalyi)]

@Guewen,

as you can see with my last commits:

1) I removed the console.log statement
2) I added a web unit tests + instruction on how to run it
3) I added a section explaining how to run standard YAML tests for a module using web_context_tunnel and gave a real example of module conversion for the Brazilian localization (including YAML tests) https://github.com/openerpbrasil/l10n_br_core/compare/develop...feature%2Fsale-web-context-tunnel

So what do you think now?

I think a place we could use it (among many others would be the fiscal-rules) modules, so we could leave the original on_change signatures untouched and retain compatibility with the other modules.

[lp:~akretion-team/server-env-tools/web-context-tunnel]

57. By Raphaël Valyi - http://www.akretion.com on 2013-12-26

fixed tipo in test logs

[Mario Arias (the-clone-master)]

A great way to avoid signature changes!

When approved, please promote its usage among known community modules with these signature problems... ;-)

review: Approve (code review, no test)

[Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c)]

review: Approve (code review, no test)

[Guewen Baconnier @ Camptocamp (gbaconnier-c2c)]

> @Guewen,
>
> as you can see with my last commits:
>
> 1) I removed the console.log statement
> 2) I added a web unit tests + instruction on how to run it
> 3) I added a section explaining how to run standard YAML tests for a module
> using web_context_tunnel and gave a real example of module conversion for the
> Brazilian localization (including YAML tests)
> https://github.com/openerpbrasil/l10n_br_core/compare/develop...feature
> %2Fsale-web-context-tunnel
>
> So what do you think now?
>
> I think a place we could use it (among many others would be the fiscal-rules)
> modules, so we could leave the original on_change signatures untouched and
> retain compatibility with the other modules.

Neat! Thanks

review: Approve (code review)

=== added directory 'web_context_tunnel'
=== added file 'web_context_tunnel/__init__.py'
=== added file 'web_context_tunnel/__openerp__.py'
--- web_context_tunnel/__openerp__.py	1970-01-01 00:00:00 +0000
+++ web_context_tunnel/__openerp__.py	2013-12-26 16:44:05 +0000
@@ -0,0 +1,92 @@
+{
+    'name': 'Web Context Tunnel',
+    'category': 'Hidden',
+    'author': 'Akretion',
+    'license': 'AGPL-3',
+    'description':"""
+Web Context Tunnel.
+===================
+
+The problem with OpenERP on_changes
+-----------------------------------
+
+OpenERP uses to pass on_change Ajax events arguments using positional
+arguments. This is annoying as modules often need to pass extra arguments
+that are not present in the base on_change signatures. As soon as two modules
+try to alter this signature to add their extra arguments, they are incompatible
+between them unless some extra glue module make them compatible again by
+taking all extra arguments into account. But this leads to a combinatorial 
+explosion to make modules compatible again.
+
+The solution
+------------
+
+This module provides a simple work around that will work in most of the cases.
+In fact it works if the base on_change is designed to pass the context
+argument. Else it won't work and you should go the old way. But in any case
+it's a bad practice if an on_change doesn't pass the context argument and you
+can certainly rant about these bad on_changes to the the context added in the
+arguments.
+
+So for an on_change passing the context, how does this module works?
+
+Well OpenERP already has an elegant solution for an extension module to alter
+an XML attributes: put an extension point in the view using
+position="attributes" and then redefine the attribute. That is already used at
+several places to replace the "context" attribute that the client will send to
+the server.
+
+The idea here is to wrap the extra arguments needed by your on_change inside
+that context dictionary just as it were a regular Python kwargs. That context
+should then be automatically propagated accross the on_change call chain,
+no matter of the module order and without any need to hack any on_change
+signature.
+
+The issue with just position="attributes" and redefining the context, is that
+again, if two independent modules redefine the context, they are incompatible
+unless a third module accounts for both of them.
+
+But with this module, an extension point can now use position="attributes" and
+instead of redefining the "context" attribute, you will now just define a new
+"context_foo" attribute this way:
+<attribute name="context_foo">{'my_extra_field': my_extra_field}</attribute>.
+
+This module modifies the web client in such a way that before sending the Ajax
+on_change event request to the server, all the node attributes starting with
+"context" are merged into a single context dictionnary, keeping the keys and
+values from all extensions. In the rare case a module really wants to override
+the value in context, then it needs to still override the original context
+attribute (or the other original attribute).
+
+And of course, if you should call your on_change by API or webservice instead
+of using the web client, simply ensure you are wrapping the required extra
+arguments in the context dictionary.
+
+Tests
+-----
+
+This module comes with a simple test in static/test/context_tunnel.js.
+To run it, open the page /web/tests?mod=web_context_tunnel in your browser
+as explained here https://doc.openerp.com/trunk/web/testing
+It should also by picked by the Python testing when testing with PhantomJS.
+
+As for testing modules using web_context_tunnel with YAML, yes it's possible.
+In fact you need to manually mimic the new web-client behavior by manually
+ensuring you add the extra context keys you will need later in your on_change.
+For instance, before the on_change is called, you can alter the context with
+a !python statement like context.update({'my_extra_field': my_extra_field}).
+
+You can see an example of module conversion to use web_context_tunnel here
+for instance:
+https://github.com/openerpbrasil/l10n_br_core/compare/develop...feature%2Fsale-web-context-tunnel
+""",
+    'version': '2.0',
+    'depends': ['web'],
+    'js': ['static/src/js/context_tunnel.js'],
+    'test': [
+        'static/test/context_tunnel.js',
+    ],
+    'css': [],
+    'auto_install': False,
+    'web_preload': False,
+}

=== added directory 'web_context_tunnel/i18n'
=== added directory 'web_context_tunnel/static'
=== added directory 'web_context_tunnel/static/src'
=== added directory 'web_context_tunnel/static/src/js'
=== added file 'web_context_tunnel/static/src/js/context_tunnel.js'
--- web_context_tunnel/static/src/js/context_tunnel.js	1970-01-01 00:00:00 +0000
+++ web_context_tunnel/static/src/js/context_tunnel.js	2013-12-26 16:44:05 +0000
@@ -0,0 +1,26 @@
+openerp.web_context_tunnel = function(instance) {
+
+    instance.web.form.FormWidget.prototype.build_context = function() {
+        var v_context = false;
+        var fields_values = false;
+        // instead of using just the attr context, we merge any attr starting with context
+        for (var key in this.node.attrs) {
+            if (key.substring(0, 7) === "context") {
+                if (!v_context) {
+                    fields_values = this.field_manager.build_eval_context();
+                    v_context = new instance.web.CompoundContext(this.node.attrs[key]).set_eval_context(fields_values);
+                } else {
+                    v_context = new instance.web.CompoundContext(this.node.attrs[key], v_context).set_eval_context(fields_values);
+                }
+
+            }
+        }
+        if (!v_context) {
+            v_context = (this.field || {}).context || {};
+        }
+        return v_context;
+    };
+};
+
+
+// vim:et fdc=0 fdl=0:

=== added directory 'web_context_tunnel/static/test'
=== added file 'web_context_tunnel/static/test/context_tunnel.js'
--- web_context_tunnel/static/test/context_tunnel.js	1970-01-01 00:00:00 +0000
+++ web_context_tunnel/static/test/context_tunnel.js	2013-12-26 16:44:05 +0000
@@ -0,0 +1,14 @@
+openerp.testing.section('context_tunnel', {
+}, function (test) {
+    test.dependencies = window['oe_all_dependencies'];
+    test("context composition", function (instance) {
+        var field_manager = new instance.web.form.DefaultFieldManager();
+        var node = {'attrs': {'context': {'key1': 'value1', 'key2': 'value2'}, 'context_2': {'key3': 'value3'}, 'context_3': {'key4': 'value4'}}}
+        var w = new instance.web.form.FormWidget(field_manager, node);
+        var context = w.build_context().eval();
+        ok(context['key1'] === 'value1', 'right value for key1 in context');
+        ok(context['key2'] === 'value2', 'right value for key2 in context');
+        ok(context['key3'] === 'value3', 'right value for key3 in context');
+        ok(context['key4'] === 'value4', 'right value for key4 in context');
+    });
+});