[IMP] doc : expand the coding guidelines

- correct typos
- clarify some points, according to the community comments
- precise the xml notation : record tag vs act_window/menuitem/... tags
- add organization of report, wizard, data/demo, ... (directories and naming conventions)
- new conventions for 'symbols'
- ...

Thanks to @rim-odoo for its grammar extremist review, and @tde-banana-odoo for our long debates about wizard conventions.

1 files changed, 111 insertions, 38 deletions

diff --git a/doc/reference/guidelines.rst b/doc/reference/guidelines.rst
index 54a00097705..ad33f2ba56d 100644
--- a/doc/reference/guidelines.rst
+++ b/doc/reference/guidelines.rst
@@ -6,10 +6,9 @@
 Odoo Guidelines
 ===============
 
-This page introduce the new Odoo Coding Guidelines. These guidelines
-aim to improve the quality of the code (better readability of source,
-...) and  Odoo Apps. Indeed, proper code ought ease maintenance, aid
-debugging, lower complexity and promote reliability.
+This page introduce the new Odoo Coding Guidelines. These guidelines aim to improve the quality of the code (better readability of source, ...) and Odoo Apps. Indeed, proper code ought ease maintenance, aid debugging, lower complexity and promote reliability.
+
+These guidelines should be applied to every new module, and new developpment. These guidelines will be applied to old module **only** in case of code refactoring (migration to new API, big refactoring, ...).
 
 .. warning::
 
@@ -24,14 +23,22 @@ Module structure
 
 Directories
 -----------
-A module is organised in a few directories :
+A module is organised in some important directories. These directories aim to contain the business core of the module; having a look at them should make understand the purpose of the module.
 
 - *data/* : demo and data xml
 - *models/* : models definition
-- *controllers/* : contains controllers (http routes).
+- *controllers/* : contains controllers (HTTP routes).
 - *views/* : contains the views and templates
 - *static/* : contains the web assets, separated into *css/, js/, img/, lib/, ...*
 
+Other directories compose the module.
+
+- *data/* : contains the data (in XML form)
+- *wizard/* : regroup the transient models (formerly *osv_memory*) and their views.
+- *report/* : contains the reports (RML report **[deprecated]**, models based on SQL views (for reporting) and other complex reports). Python objects and XML views are included in this directory.
+- *tests/* : contains the Python/YML tests
+
+
 File naming
 -----------
 For *views* declarations, split backend views from (frontend)
@@ -52,16 +59,36 @@ For instance, *sale* module introduces ``sale_order`` and
 ``<main_model>`` files will be named :file:`models/sale_order.py` and
 :file:`views/sale_order_views.py`.
 
-
 For *data*, split them by purpose : demo or data. The filename will be
 the main_model name, suffixed by *_demo.xml* or *_data.xml*.
 
-For *controller*, the only file should be named *main.py*.
+For *controllers*, the only file should be named *main.py*. Otherwise, if you need to inherit an existing controller from another module, its name will be *<module_name>.py*. Unlike *models*, each controller should be contained in a separated file.
+
+For *static files*, since the resources can be used in different contexts (frontend, backend, both), they will be included in only one bundle. So, CSS/Less, JavaScript and XML files should be suffixed with the name of the bundle type. i.e.: *im_chat_common.css*, *im_chat_common.js* for 'assets_common' bundle, and *im_chat_backend.css*, *im_chat_backend.js* for 'assets_backend' bundle.
+For modules having only one file, the convention will be *<module_name>.ext* (i.e.: *project.js*).
+Don't link data (image, libraries) outside Odoo: don't use an
+URL to an image but copy it in our codebase instead.
+
+For *data*, split them by purpose: data or demo. The filename will be
+the *main_model* name, suffixed by *_data.xml* or *_demo.xml*.
+
+For *wizards*, the naming convention is :
+
+- :file:`{<main_transient>}.py`
+- :file:`{<main_transient>}_views.xml`
+
+Where *<main_transient>* is the name of the dominant transient model, just like for *models*. <main_transient>.py can contains the models 'model.action' and 'model.action.line'.
+
+For *statistics reports*, their names should look like :
+
+- :file:`{<report_name_A>}_report.py`
+- :file:`{<report_name_A>}_report_views.py` (often pivot and graph views)
+
+For *printable reports*, you should have :
+
+- :file:`{<print_report_name>}_reports.py` (report actions, paperformat definition, ...)
+- :file:`{<print_report_name>}_templates.xml` (xml report templates)
 
-For *static files*, the name pattern is *<module_name>.ext* (i.e. :
-static/js/im_chat.js, static/css/im_chat.css, static/xml/im_chat.xml,
-...). Don't link data (image, libraries) outside Odoo : don't use an
-url to an image but copy it in our codebase instead.
 
 The complete tree should look like
 
@@ -72,6 +99,7 @@ The complete tree should look like
     |-- __openerp__.py
     |-- controllers/
     |   |-- __init__.py
+    |   |-- <inherited_module_name>.py
     |   `-- main.py
     |-- data/
     |   |-- <main_model>_data.xml
@@ -80,6 +108,12 @@ The complete tree should look like
     |   |-- __init__.py
     |   |-- <main_model>.py
     |   `-- <inherited_main_model>.py
+    |-- report/
+    |   |-- __init__.py
+    |   |-- <main_stat_report_model>.py
+    |   |-- <main_stat_report_model>_views.xml
+    |   |-- <main_print_report>_reports.xml
+    |   `-- <main_print_report>_templates.xml
     |-- security/
     |   |-- ir.model.access.csv
     |   `-- <main_model>_security.xml
@@ -98,12 +132,16 @@ The complete tree should look like
     |       |   `-- <my_module_name>.less
     |       `-- xml/
     |           `-- <my_module_name>.xml
-    `-- views/
-        |-- <main_model>_templates.xml
-        |-- <main_model>_views.xml
-        |-- <inherited_main_model>_templates.xml
-        `-- <inherited_main_model>_views.xml
-
+    |-- views/
+    |   |-- <main_model>_templates.xml
+    |   |-- <main_model>_views.xml
+    |   |-- <inherited_main_model>_templates.xml
+    |   `-- <inherited_main_model>_views.xml
+    `-- wizard/
+        |-- <main_transient_A>.py
+        |-- <main_transient_A>_views.xml
+        |-- <main_transient_B>.py
+        `-- <main_transient_B>_views.xml
 
 .. note:: File names should only contain ``[a-z0-9_]`` (lowercase
           alphanumerics and ``_``)
@@ -115,7 +153,7 @@ XML files
 
 Format
 ------
-When declaring a record in XML,
+To declare a record in XML, the **record** notation (using *<record>*) is recommended:
 
 - Place ``id`` attribute before ``model``
 - For field declaration, ``name`` attribute is first. Then place the
@@ -124,7 +162,7 @@ When declaring a record in XML,
   ordered by importance.
 
 - Try to group the record by model. In case of dependencies between
-  action/menu/views, the convention may not be applicable.
+  action/menu/views, this convention may not be applicable.
 - Use naming convention defined at the next point
 - The tag *<data>* is only used to set not-updatable data with ``noupdate=1``
 
@@ -142,6 +180,16 @@ When declaring a record in XML,
         </field>
     </record>
 
+Some syntax equivalences exists, and can be used:
+
+- menuitem: use it as a shortcut to declare a ``ir.ui.menu``
+- workflow: the <workflow> tag sends a signal to an existing workflow.
+- template: use it to declare a QWeb View requiring only the ``arch`` section of the view.
+- report: use to declare a :ref:`report action <reference/actions/report>`
+- act_window: use it if the record notation can't do what you want
+
+The 4 first tags are prefered over the *record* notation.
+
 
 Naming xml_id
 -------------
@@ -165,14 +213,11 @@ Use the following pattern :
   *concerned_group* is the short name of the concerned group ('user'
   for the 'model_name_group_user', 'public' for public user, 'company'
   for multi-company rules, ...).
+* For a group : :samp:`{<model_name>}_group_{<group_name>}` where *group_name* is the name of the group, generally 'user', 'manager', ...
 
 .. code-block:: xml
 
     <!-- views and menus -->
-    <record id="model_name_menu" model="ir.ui.menu">
-        ...
-    </record>
-
     <record id="model_name_view_form" model="ir.ui.view">
         ...
     </record>
@@ -181,6 +226,19 @@ Use the following pattern :
         ...
     </record>
 
+    <menuitem
+        id="model_name_menu_root"
+        name="Main Menu"
+        sequence="5"
+    />
+    <menuitem
+        id="model_name_menu_action"
+        name="Sub Menu 1"
+        parent="module_name.module_name_menu_root"
+        action="model_name_action"
+        sequence="10"
+    />
+
     <!-- actions -->
     <record id="model_name_action" model="ir.actions.act_window">
         ...
@@ -191,7 +249,7 @@ Use the following pattern :
     </record>
 
     <!-- security -->
-    <record id="model_name_group_user" model="res.groups">
+    <record id="module_name_group_user" model="res.groups">
         ...
     </record>
 
@@ -261,6 +319,7 @@ Inside these 3 groups, the imported lines are alphabetically sorted.
     import base64
     import re
     import time
+    from datetime import datetime
     # 2 :  imports of openerp
     import openerp
     from openerp import api, fields, models # alphabetically ordered
@@ -274,22 +333,27 @@ Inside these 3 groups, the imported lines are alphabetically sorted.
 Idioms
 ------
 
-- Prefer ``%`` over ``.format()``, prefer ``%(varname)`` instead of position (This is better for translations)
-- Try to avoid generators and decorators
-- Always favor *Readability* over *conciseness* or using the language features or idioms.
+- Prefer ``%`` over ``.format()`` (when only one variable to replace in a string), prefer ``%(varname)`` instead of position (when multiple variables are to be replaced in a string). This makes the translation easier for the translators community.
+- Avoid to create generators and decorators: only use the ones provide by the Odoo API.
+- Always favor *readability* over *conciseness* or using the language features or idioms.
 - Use list comprehension, dict comprehension, and basic manipulation using ``map``, ``filter``, ``sum``, ... They make the code easier to read.
 - The same applies for recordset methods : use ``filtered``, ``mapped``, ``sorted``, ...
-- Each python file should have ``# coding: utf-8 `` as first line
-- Use the ``UserError`` defined in ``openerp.exceptions`` instead of overriding ``Warning``, or find a more appropriate exception in *exceptions.py*
+- Each python file should have ``# -*- coding: utf-8 -*-`` as first line.
 - Document your code (docstring on methods, simple comments for the tricky part of the code)
-- Use meaningful variable/class/method names
-
+- Use meaningful variable/class/method names.
+- Every method used to compute data for a 'stat button' should use a ``read_group`` or a SQL query. This aims to improve performance (by computing data in only on query).
 
 
 Symbols
 -------
 
-- Odoo Python Class : use camelcase for code in api v8, underscore lowercase notation for old api.
+- Model name (using the dot notation, prefix by the module name) :
+    - When defining an Odoo Model : use singular form of the name (*res.partner* and *sale.order* instead of *res.partnerS* and *saleS.orderS*)
+    - When defining an Odoo Transient (wizard) : use ``<related_base_model>.<action>`` where *related_base_model* is the base model (defined in *models/*) related to the transient, and *action* is the short name of what the transient do. For instance : ``account.invoice.make``, ``project.task.delegate.batch``, ...
+    - When defining *report* model (SQL views e.i.) : use ``<related_base_model>.report.<action>``, based on the Transient convention.
+
+- Odoo Python Class : use camelcase for code in api v8 (Object-oriented style), underscore lowercase notation for old api (SQL style).
+
 
 .. code-block:: python
 
@@ -349,7 +413,7 @@ Symbols
             store=True, readonly=True, compute='_compute_seats')
         price = fields.Integer(string='Price')
 
-        # compute and search fields, in the same order that fields declaration
+        # compute and search fields, in the same order of fields declaration
         @api.multi
         @api.depends('seats_max', 'registration_ids.state', 'registration_ids.nb_register')
         def _compute_seats(self):
@@ -364,8 +428,8 @@ Symbols
         def _onchange_date_begin(self):
             ...
 
-        # CRUD methods
-        def create(self):
+        # CRUD methods (and name_get, name_search, ...) overrides
+        def create(self, values):
             ...
 
         # Action methods
@@ -387,12 +451,20 @@ Javascript and CSS
 - Use a linter (jshint, ...)
 - Never add minified Javascript Libraries
 - Use camelcase for class declaration
+- Unless your code is supposed to run on every page, target specific pages using the ``if_dom_contains`` function of website module. Target an element which is specific to the pages your code needs to run on using JQuery.
+
+.. code-block:: javascript
+
+    openerp.website.if_dom_contains('.jquery_class_selector', function () {
+        /*your code here*/
+    });
+
 
 **For CSS :**
 
-- Prefix all your class with *o_<module_name>* where *module_name* is the technical name of the module ('sale', 'im_chat', ...) or the main route reserved by the module (for website module mainly, i.e. : 'o_forum' for website_forum module). The only exception for this rule is the webclient : it simply use *o_* prefix.
+- Prefix all your classes with *o_<module_name>* where *module_name* is the technical name of the module ('sale', 'im_chat', ...) or the main route reserved by the module (for website module mainly, i.e. : 'o_forum' for *website_forum* module). The only exception for this rule is the webclient: it simply uses *o_* prefix.
 - Avoid using id
-- Use bootstrap native class
+- Use Bootstrap native classes
 - Use underscore lowercase notation to name class
 
 Git
@@ -408,6 +480,7 @@ Prefix your commit with
 - **[REF]** for refactoring
 - **[ADD]** for adding new resources
 - **[REM]** for removing of resources
+- **[MOV]** for moving files (Do not change content of moved file, otherwise Git will loose track, and the history will be lost !), or simply moving code from a file to another one.
 - **[MERGE]** for merge commits (only for forward/back-port)
 - **[CLA]** for signing the Odoo Individual Contributor License