Merge lp:~vauxoo/openerp-web/7.0-missing-menu-attr-nhomar into lp:openerp-web/7.0

[Nhomar - Vauxoo]

[FIX] Added some attributes necessary to implement missing features from V6.1 or even new ones for V7.0

The web_icon and web_icon_hover and string was totally ignored in V7.0 with this, this variables are available with a simple.

Module overwrite the feature and try special things.

Why not an extra module to achieve this?

Because the assignment of fields is inside the load method and this method is too important in OpenERP and has +20 lines, easy algorithm but to add some fields We think is not necessary overwrite this method it is better just make an MP.

One example of use this attributes can be found on

lp:~vauxoo/web-addons/7.0-web_hideleftmenu

Module: web_fontawesome

and the attributes available in ir.ui.menu can be verified here...

http://bazaar.launchpad.net/~openerp/openobject-server/7.0/view/head:/openerp/import_xml.rng#L174

Thanks in advance.

PS: If it is approved, I veerified is really easy forward port to trunk, if you think is necesary other MP I can do it.

[Olivier Dony (Odoo) (odo-openerp)]

Hi,

This patch is unfortunately similar to some "hooks" that are often requested, like adding an empty method called "_hook_xxx" that does nothing, just to be able to override it.

We have to decline these requests because:
- it creates artificial legacy/unused cruft in the code, that is very likely to be removed in a future cleanup - breaking anything that depends on it
- it creates more confusion for the reader: What was the purpose of reading those values if they're unused? Should someone now report a bug now because the "web_icon" is not used in the web client?

Similarly, reusing a deprecated field (web_icon, etc.) for a new purpose is likely to get broken in the future when we cleanup, even if I can see why it is tempting for you to do it - as <menuitem> supports this attribute directly.

If you still want to do it and are aware of the consequences, you could perhaps make a patch to add a MENU_FIELDS_TO_LOAD constant that you could easily monkey-patch in your module, and would be less likely to break in the near future. At least this code would not scream "please clean me up" ;-)

I hope you understand my point..

Thanks!

PS: some of the attributes in the RNG are deprecated, we should consider removing them in trunk and cleaning up (what would `string` do, for example?)

review: Disapprove

[Nhomar - Vauxoo (nhomar)]

Hello friend.

I understand your point, not agreed about your "Concept" of clean up, but I understood.

My concept of clean up my friend "Never ever" can mean "take off features" with this specific case, instead "Extend" the basic usage of menus you cut off the scope, but yes we understand.

REgadrs

Unmerged revisions

4031. By Nhomar - Vauxoo on 2013-09-23

[FIX] Added some attributes necesaries to implement missing features from V6.1 or even new ones for V7.0
The web_icon and web_icon_hover and string was totally ignored in V7.0 with this, this variables are availables with a simple.
Module overwrite the feature and try special things.

Why not an extra module to achieve this?

Because the asignment of fields is inside the load method and this method is to important in OpenERP and has +20 lines, easy
algorithm but to add some fields We think is not necesary overwrite this methid it is better just make an MP.

One example of use this attributes can be found on lp:~vauxoo/web-addons/7.0-web_hideleftmenu and the attributes availabes in menu can be verified here... http://bazaar.launchpad.net/~openerp/openobject-server/7.0/view/head:/openerp/import_xml.rng#L174

=== modified file 'addons/web/controllers/main.py'
--- addons/web/controllers/main.py	2013-07-16 13:15:48 +0000
+++ addons/web/controllers/main.py	2013-09-23 05:26:23 +0000
@@ -988,7 +988,7 @@
         """
         Menus = req.session.model('ir.ui.menu')
 
-        fields = ['name', 'sequence', 'parent_id', 'action']
+        fields = ['name', 'icon', 'string', 'web_icon', 'web_icon_hover', 'sequence', 'parent_id', 'action']
         menu_root_ids = self.get_user_roots(req)
         menu_roots = Menus.read(menu_root_ids, fields, req.context) if menu_root_ids else []
         menu_root = {