[IMP] mail: star is renamed as favorite (propagated changes through module); added views for this model in technical feature, under emails. Cleaned message_read and message_read_get_expandable because the code was quite obfuscated.

bzr revid: tde@openerp.com-20121018152322-cakaas3a77h1pi7m

7 files changed, 115 insertions, 147 deletions

diff --git a/addons/mail/__init__.py b/addons/mail/__init__.py
index da002d13d9d..aea7ee62cbc 100644
--- a/addons/mail/__init__.py
+++ b/addons/mail/__init__.py
@@ -27,7 +27,7 @@ import mail_mail
 import mail_thread
 import mail_group
 import mail_vote
-import mail_star
+import mail_favorite
 import res_partner
 import res_users
 import report
@@ -37,4 +37,3 @@ import mail_group_menu
 import update
 
 # vim:expandtab:smartindent:tabstop=4:softtabstop=4:shiftwidth=4:
-
diff --git a/addons/mail/__openerp__.py b/addons/mail/__openerp__.py
index 39a3f1bee13..27565bc4102 100644
--- a/addons/mail/__openerp__.py
+++ b/addons/mail/__openerp__.py
@@ -28,7 +28,7 @@
     'description': """
 Business oriented Social Networking
 ===================================
-The Social Networking module provides a unified social network abstraction layer allowing applications to display a complete 
+The Social Networking module provides a unified social network abstraction layer allowing applications to display a complete
 communication history on documents with a fully-integrated email and message management system.
 
 It enables the users to read and send messages as well as emails. It also provides a feeds page combined to a subscription mechanism that allows to follow documents and to be constantly updated about recent news.
@@ -54,6 +54,7 @@ Main Features
         'mail_message_view.xml',
         'mail_mail_view.xml',
         'mail_followers_view.xml',
+        'mail_favorite_view.xml',
         'mail_thread_view.xml',
         'mail_group_view.xml',
         'res_partner_view.xml',
diff --git a/addons/mail/mail_star.py b/addons/mail/mail_favorite.py
index 00a10c32dc6..4fe5fbdb557 100644
--- a/addons/mail/mail_star.py
+++ b/addons/mail/mail_favorite.py
@@ -22,13 +22,13 @@
 from osv import osv, fields
 
 
-class mail_star(osv.Model):
-    ''' Mail vote feature allow users to select messages and display.
-        This allows for example see in one time all important document
-        for the user. '''
+class mail_favorite(osv.Model):
+    ''' Favorite model: relationship table between messages and users. A favorite
+        message is a message the user wants to see in a specific 'Favorite'
+        mailbox, like a starred mechanism. '''
 
-    _name = 'mail.star'
-    _description = 'Mail Star'
+    _name = 'mail.favorite'
+    _description = 'Favorite messages'
     _columns = {
             'message_id': fields.many2one('mail.message', 'Message', select=1,
                 ondelete='cascade', required=True),
diff --git a/addons/mail/mail_message.py b/addons/mail/mail_message.py
index 6f28014c7de..31fec861b3a 100644
--- a/addons/mail/mail_message.py
+++ b/addons/mail/mail_message.py
@@ -20,12 +20,10 @@
 ##############################################################################
 
 import logging
-import openerp
 import tools
 
 from email.header import decode_header
 from openerp import SUPERUSER_ID
-from operator import itemgetter
 from osv import osv, orm, fields
 from tools.translate import _
 
@@ -48,7 +46,10 @@ class mail_message(osv.Model):
     _order = 'id desc'
 
     _message_read_limit = 10
+    _message_read_fields = ['id', 'parent_id', 'model', 'res_id', 'body', 'subject', 'date', 'to_read',
+        'type', 'vote_user_ids', 'attachment_ids', 'author_id', 'partner_ids', 'record_name', 'favorite_user_ids']
     _message_record_name_length = 18
+    _message_read_more_limit = 1024
 
     def _shorten_name(self, name):
         if len(name) <= (self._message_record_name_length + 3):
@@ -56,7 +57,9 @@ class mail_message(osv.Model):
         return name[:self._message_record_name_length] + '...'
 
     def _get_record_name(self, cr, uid, ids, name, arg, context=None):
-        """ Return the related document name, using get_name. """
+        """ Return the related document name, using name_get. It is included in
+            a try/except statement, because if uid cannot read the related
+            document, he should see a void string instead of crashing. """
         result = dict.fromkeys(ids, False)
         for message in self.read(cr, uid, ids, ['model', 'res_id'], context=context):
             if not message['model'] or not message['res_id']:
@@ -132,10 +135,12 @@ class mail_message(osv.Model):
             type='boolean', string='To read',
             help='Functional field to search for messages the current user has to read'),
         'subtype_id': fields.many2one('mail.message.subtype', 'Subtype'),
-        'vote_user_ids': fields.many2many('res.users', 'mail_vote', 'message_id', 'user_id', string='Votes',
+        'vote_user_ids': fields.many2many('res.users', 'mail_vote',
+            'message_id', 'user_id', string='Votes',
             help='Users that voted for this message'),
-        'star_user_ids': fields.many2many('res.users', 'mail_star', 'message_id', 'user_id', string='Stared',
-            help='Users that stared this message'),
+        'favorite_user_ids': fields.many2many('res.users', 'mail_favorite',
+            'message_id', 'user_id', string='Favorite',
+            help='Users that set this message in their favorites'),
     }
 
     def _needaction_domain_get(self, cr, uid, context=None):
@@ -144,8 +149,7 @@ class mail_message(osv.Model):
         return []
 
     def _get_default_author(self, cr, uid, context=None):
-        # remove context to avoid possible hack in browse with superadmin using context keys that could trigger a specific behavior
-        return self.pool.get('res.users').browse(cr, SUPERUSER_ID, uid, context=None).partner_id.id
+        return self.pool.get('res.users').read(cr, uid, uid, ['partner_id'], context=context)['partner_id'][0]
 
     _defaults = {
         'type': 'email',
@@ -158,56 +162,47 @@ class mail_message(osv.Model):
     # Vote/Like
     #------------------------------------------------------
 
-    def vote_toggle(self, cr, uid, ids, user_ids=None, context=None):
-        ''' Toggles voting. Done as SUPERUSER_ID because of write access on
-            mail.message not always granted. '''
-        if not user_ids:
-            user_ids = [uid]
-        for message in self.read(cr, SUPERUSER_ID, ids, ['vote_user_ids'], context=context):
-            for user_id in user_ids:
-                has_voted = user_id in message.get('vote_user_ids')
-                if not has_voted:
-                    self.write(cr, SUPERUSER_ID, message.get('id'), {'vote_user_ids': [(4, user_id)]}, context=context)
-                else:
-                    self.write(cr, SUPERUSER_ID, message.get('id'), {'vote_user_ids': [(3, user_id)]}, context=context)
-        return not(has_voted) or False
+    def vote_toggle(self, cr, uid, ids, context=None):
+        ''' Toggles vote. Performed using read to avoid access rights issues. '''
+        for message in self.read(cr, uid, ids, ['vote_user_ids'], context=context):
+            new_has_voted = not (uid in message.get('vote_user_ids'))
+            if new_has_voted:
+                self.write(cr, uid, message.get('id'), {'vote_user_ids': [(4, uid)]}, context=context)
+            else:
+                self.write(cr, uid, message.get('id'), {'vote_user_ids': [(3, uid)]}, context=context)
+        return new_has_voted or False
 
     #------------------------------------------------------
-    # Stared/unstared
+    # Favorite
     #------------------------------------------------------
 
-    def star_toggle(self, cr, uid, ids, user_ids=None, context=None):
-        ''' Toggles voting. Done as SUPERUSER_ID because of write access on
-            mail.message not always granted. '''
-        if not user_ids:
-            user_ids = [uid]
-        for message in self.read(cr, SUPERUSER_ID, ids, ['star_user_ids'], context=context):
-            for user_id in user_ids:
-                has_stared = user_id in message.get('star_user_ids')
-                if not has_stared:
-                    self.write(cr, SUPERUSER_ID, message.get('id'), {'star_user_ids': [(4, user_id)]}, context=context)
-                else:
-                    self.write(cr, SUPERUSER_ID, message.get('id'), {'star_user_ids': [(3, user_id)]}, context=context)
-        return not(has_stared) or False
+    def favorite_toggle(self, cr, uid, ids, context=None):
+        ''' Toggles favorite. Performed using read to avoid access rights issues. '''
+        for message in self.read(cr, uid, ids, ['favorite_user_ids'], context=context):
+            new_is_favorite = not (uid in message.get('favorite_user_ids'))
+            if new_is_favorite:
+                self.write(cr, uid, message.get('id'), {'favorite_user_ids': [(4, uid)]}, context=context)
+            else:
+                self.write(cr, uid, message.get('id'), {'favorite_user_ids': [(3, uid)]}, context=context)
+        return new_is_favorite or False
 
     #------------------------------------------------------
     # Message loading for web interface
     #------------------------------------------------------
 
     def _message_get_dict(self, cr, uid, message, context=None):
-        """ Return a dict representation of the message.
+        """ Return a dict representation of the message. This representation is
+            used in the JS client code, to display the messages.
 
             :param dict message: read result of a mail.message
         """
+        has_voted = False
         if uid in message['vote_user_ids']:
             has_voted = True
-        else:
-            has_voted = False
-        has_stared = False
-        if uid in message['star_user_ids']:
-            has_stared = True
-        else:
-            has_stared = False
+
+        is_favorite = False
+        if uid in message['favorite_user_ids']:
+            is_favorite = True
 
         try:
             attachment_ids = [{'id': attach[0], 'name': attach[1]} for attach in self.pool.get('ir.attachment').name_get(cr, uid, message['attachment_ids'], context=context)]
@@ -219,11 +214,6 @@ class mail_message(osv.Model):
         except (orm.except_orm, osv.except_osv):
             partner_ids = []
 
-        try:
-            record_name = msg.record_name
-        except (orm.except_orm, osv.except_osv):
-            record_name = False
-
         return {
             'id': message['id'],
             'type': message['type'],
@@ -241,40 +231,41 @@ class mail_message(osv.Model):
             'parent_id': message['parent_id'] and message['parent_id'][0] or False,
             # TDE note: see with CHM about votes, how they are displayed (only number, or name_get ?)
             # vote: should only use number of votes
-            # 'vote_user_ids': vote_ids,
+            'vote_nb': len(message['vote_user_ids']),
             'has_voted': has_voted,
             'is_private': message['model'] and message['res_id'],
-            'has_stared': has_stared,
+            'is_favorite': is_favorite,
             'to_read': message['to_read'],
         }
 
-    def _message_read_expandable(self, cr, uid, tree, result, dict_tree, message_loaded_ids, domain, context, parent_id, limit):
+    def _message_read_expandable(self, cr, uid, message_list, read_messages,
+            message_loaded_ids=[], domain=[], context=None, parent_id=False, limit=None):
         """ Create the expandable message for all parent message read
             this function is used by message_read
-            TDE note: add default values for args, add some comments
-            tree: id tree
-            dict_tree: [id]: message
-            :param dict tree: tree of message ids
+
+            :param list message_list: list of messages given by message_read to
+                which we have to add expandables
+            :param dict read_messages: dict [id]: read result of the messages to
+                easily have access to their values, given their ID
         """
         # sort for group items / TDE: move to message_read
-        tree = sorted(tree, key=lambda k: k['id'])
+        # result = sorted(result, key=lambda k: k['id'])
         tree_not = []
         # expandable for not show message
-        for msg_id in tree:
-            message = dict_tree[msg_id]
+        for message_id, message in read_messages.iteritems():
             # get all childs
             # SHOULD NOT BE SUPERUSED_ID -> check search is correctly implemented in mail.message
             not_loaded_ids = self.search(cr, uid, [
                 ('parent_id', '=', message['id']),
                 ('id', 'not in', message_loaded_ids),
-                ], context=context, limit=1000)
+                ], context=context, limit=self._message_read_more_limit)
             # group childs not read
             id_min = None
             id_max = None
             nb = 0
 
             for not_loaded_id in not_loaded_ids:
-                if not_loaded_id not in tree:
+                if not read_messages.get(not_loaded_id):
                     nb += 1
                     if id_min == None or id_min > not_loaded_id:
                         id_min = not_loaded_id
@@ -283,11 +274,11 @@ class mail_message(osv.Model):
                     tree_not.append(not_loaded_id)
                 else:
                     if nb > 0:
-                        result.append({
-                            'domain': [('id', '>=', id_min), ('id', '<=', id_max), ('parent_id', '=', msg_id)],
+                        message_list.append({
+                            'domain': [('id', '>=', id_min), ('id', '<=', id_max), ('parent_id', '=', message_id)],
                             'nb_messages': nb,
                             'type': 'expandable',
-                            'parent_id': msg_id,
+                            'parent_id': message_id,
                             'id':  id_min,
                             'model':  message['model']
                         })
@@ -295,22 +286,22 @@ class mail_message(osv.Model):
                     id_max = None
                     nb = 0
             if nb > 0:
-                result.append({
-                    'domain': [('id', '>=', id_min), ('id', '<=', id_max), ('parent_id', '=', msg_id)],
+                message_list.append({
+                    'domain': [('id', '>=', id_min), ('id', '<=', id_max), ('parent_id', '=', message_id)],
                     'nb_messages': nb,
                     'type': 'expandable',
-                    'parent_id': msg_id,
+                    'parent_id': message_id,
                     'id':  id_min,
                     'model':  message['model'],
                 })
 
-        for msg_id in tree + tree_not:
+        for msg_id in read_messages.keys() + tree_not:
             message_loaded_ids.append(msg_id)
 
         # expandable for limit max
         ids = self.search(cr, uid, domain + [('id', 'not in', message_loaded_ids)], context=context, limit=1)
         if len(ids) > 0:
-            result.append({
+            message_list.append({
                 'domain': domain,
                 'nb_messages': 0,
                 'type': 'expandable',
@@ -319,10 +310,7 @@ class mail_message(osv.Model):
                 'max_limit': True
             })
 
-        return result
-
-    _message_read_fields = ['id', 'parent_id', 'model', 'res_id', 'body', 'subject', 'date', 'to_read',
-        'type', 'vote_user_ids', 'attachment_ids', 'author_id', 'partner_ids', 'record_name', 'star_user_ids']
+        return message_list
 
     def _get_parent(self, cr, uid, message, context=None):
         """ Tools method that tries to get the parent of a mail.message. If
@@ -354,68 +342,57 @@ class mail_message(osv.Model):
                 further parents
             :return list: list of trees of messages
         """
+        print '>>> executing message_read'
         if message_loaded_ids:
             domain += [('id', 'not in', message_loaded_ids)]
         limit = limit or self._message_read_limit
-        tree = []
-        # TDE note: better name ?
-        dict_tree = {}
-        message_ids = []
-        # record = None
+        limit = 200
+        read_messages = {}
         message_list = []
 
-        # select ids
+        # specific IDs given: fetch those ids and return directly the message list
         if ids:
             for message in self.read(cr, uid, ids, self._message_read_fields, context=context):
                 message_list.append(self._message_get_dict(cr, uid, message, context=context))
+            message_list = sorted(message_list, key=lambda k: k['id'])
+            return message_list
 
-        # key: ID, value: tree
+        # TDE FIXME: check access rights on search are implemented for mail.message
+        # fetch messages according to the domain, add their parents if uid has access to
         if not ids:
-            # TDE: check access rights on search are implemented for mail.message
             ids = self.search(cr, uid, domain, context=context, limit=limit)
             for message in self.read(cr, uid, ids, self._message_read_fields, context=context):
                 # if not in tree and not in message_loded list
-                if message['id'] not in message_ids and message['id'] not in tree and message['id'] not in message_loaded_ids:
-                    message_ids.append(message['id'])
-                    tree.append(message['id'])
-                    dict_tree[message['id']] = message
+                if not read_messages.get(message.get('id')) and message.get('id') not in message_loaded_ids:
+                    read_messages[message.get('id')] = message
                     message_list.append(self._message_get_dict(cr, uid, message, context=context))
 
+                    # get all parented message if the user have the access
                     parent = self._get_parent(cr, uid, message, context=context)
+                    while parent and parent.get('id') != parent_id:
+                        if not read_messages.get(parent.get('id')) and parent.get('id') not in message_loaded_ids:
+                            read_messages[parent.get('id')] = parent
+                            message_list.append(self._message_get_dict(cr, uid, parent, context=context))
+                        parent = self._get_parent(cr, uid, parent, context=context)
 
-                    # get all parented message if the user have the access
-                    while parent and parent['id'] != parent_id:
-                        if parent['id'] not in tree and parent['id'] not in message_loaded_ids:
-                            tree.append(parent['id'])
-                            dict_tree[message['id']] = message
-                            message_list.append(self._message_get_dict(cr, uid, message, context=context))
-                            # if not in tree and not in message_loded list
-                            if parent['id'] not in message_ids and parent['id'] not in message_loaded_ids:
-                                message_ids.append(parent['id'])
-                            parent = self._get_parent(cr, uid, parent, context=context)
-                        else:
-                            parent = False
-
-            # record the dic of message
-            # for msg in self.browse(cr, uid, message_ids, context=context):
-                # record = self._message_dict_get(cr, uid, msg, context=context)
-                # result.append( record )
-
-            # get the child expandable messages for the tree
-            message_list = self._message_read_expandable(cr, uid, tree, message_list, dict_tree, message_loaded_ids, domain, context, parent_id, limit)
-
-        message_list = sorted(message_list, key=lambda k: k['id'])
+                # print read_messages
+                # print message_list
+
+                # get the child expandable messages for the tree
+                message_list = sorted(message_list, key=lambda k: k['id'])
+                message_list = self._message_read_expandable(cr, uid, message_list, read_messages,
+                    message_loaded_ids=message_loaded_ids, domain=domain, context=context, parent_id=parent_id, limit=limit)
+
+        # message_list = sorted(message_list, key=lambda k: k['id'])
         return message_list
 
     # TDE Note: do we need this ?
     def user_free_attachment(self, cr, uid, context=None):
-        attachment_list = []
-
         attachment = self.pool.get('ir.attachment')
-        attachment_ids = attachment.search(cr, uid, [('res_model','=',''),('create_uid','=',uid)])
+        attachment_list = []
+        attachment_ids = attachment.search(cr, uid, [('res_model', '=', 'mail.message'), ('create_uid', '=', uid)])
         if len(attachment_ids):
             attachment_list = [{'id': attach.id, 'name': attach.name, 'date': attach.create_date} for attach in attachment.browse(cr, uid, attachment_ids, context=context)]
-
         return attachment_list
 
     #------------------------------------------------------
diff --git a/addons/mail/static/src/js/mail.js b/addons/mail/static/src/js/mail.js
index 0827e2a39fe..4367808836c 100644
--- a/addons/mail/static/src/js/mail.js
+++ b/addons/mail/static/src/js/mail.js
@@ -12,41 +12,31 @@ openerp.mail = function(session) {
      * ------------------------------------------------------------
      * 
      * Override of formview do_action method, to catch all return action about
-     * mail.compose.message. The purpose is to bind 'Send by e-mail' buttons
-     * and redirect them to the Chatter.
+     * mail.compose.message. The purpose is to bind 'Send by e-mail' buttons.
      */
 
     session.web.FormView = session.web.FormView.extend({
         do_action: function(action) {
             if (action.res_model == 'mail.compose.message') {
-
                 /* hack for stop context propagation of wrong value
                  * delete this hack when a global method to clean context is create
-                */
-                for(var key in action.context){
-                    if( key!='default_template_id' &&
-                        key!='default_composition_mode' &&
-                        key!='default_use_template' &&
-                        key!='default_partner_ids' &&
-                        key!='default_model' &&
-                        key!='default_res_id' &&
-                        key!='default_subtype' &&
-                        key!='active_id' &&
-                        key!='lang' &&
-                        key!='bin_raw' &&
-                        key!='tz' &&
-                        key!='active_model' &&
-                        key!='edi_web_url_view' &&
-                        key!='active_ids')
-                    action.context[key]=null;
-                };
+                 */
+                var context_keys = ['default_template_id', 'default_composition_mode', 
+                    'default_use_template', 'default_partner_ids', 'default_model',
+                    'default_res_id', 'default_subtype', 'active_id', 'lang',
+                    'bin_raw', 'tz', 'active_model', 'edi_web_url_view', 'active_ids']
+                for (var key in action.context) {
+                    if (_.indexOf(context_keys, key) == -1) {
+                        action.context[key] = null;
+                    }
+                }
                 /* end hack */
-
             }
             return this._super.apply(this, arguments);
         },
     });
 
+
     /**
      * ------------------------------------------------------------
      * ChatterUtils
@@ -90,6 +80,7 @@ openerp.mail = function(session) {
 
         /* replace textarea text into html text
          * (add <p>, <a>)
+         * TDE note : should not be here, but server-side I think ...
         */
         get_text2html: function(text){
             return text
@@ -501,7 +492,7 @@ openerp.mail = function(session) {
                 'body' : false,
                 'vote_user_ids' :[],
                 'has_voted' : false,
-                'has_stared' : false,
+                'is_favorite' : false,
                 'thread_level' : 0,
                 'to_read' : true,
                 'author_id' : [],
@@ -761,9 +752,9 @@ openerp.mail = function(session) {
             event.stopPropagation();
             var self=this;
             var button = self.$('button.oe_mail_starbox:first');
-            return this.ds_message.call('star_toggle', [[self.datasets.id]]).pipe(function(star){
-                self.datasets.has_stared=star;
-                if(self.datasets.has_stared){
+            return this.ds_message.call('favorite_toggle', [[self.datasets.id]]).pipe(function(star){
+                self.datasets.is_favorite=star;
+                if(self.datasets.is_favorite){
                     button.addClass('oe_stared');
                 } else {
                     button.removeClass('oe_stared');
diff --git a/addons/mail/static/src/xml/mail.xml b/addons/mail/static/src/xml/mail.xml
index 8388723f762..fbb64cc6772 100644
--- a/addons/mail/static/src/xml/mail.xml
+++ b/addons/mail/static/src/xml/mail.xml
@@ -287,7 +287,7 @@
     -->
     <span t-name="mail.thread.message.star">
         <span class="oe_left">
-            <button t-attf-class="oe_mail_starbox oe_tag #{widget.datasets.has_stared?'oe_stared':''}">*</button>
+            <button t-attf-class="oe_mail_starbox oe_tag #{widget.datasets.is_favorite?'oe_stared':''}">*</button>
         </span>
     </span>
 
diff --git a/addons/mail/tests/test_mail.py b/addons/mail/tests/test_mail.py
index 271d4d3ea38..5e0d0e1d39c 100644
--- a/addons/mail/tests/test_mail.py
+++ b/addons/mail/tests/test_mail.py
@@ -613,7 +613,7 @@ class test_mail(TestMailMockups):
         # Test: msg1 has Admin as voter
         self.assertEqual(set(msg1.vote_user_ids), set([user_admin]), 'after voting, Admin is not the voter')
         # Do: Bert vote for msg1
-        self.mail_message.vote_toggle(cr, uid, [msg1.id], [user_bert_id])
+        self.mail_message.vote_toggle(cr, user_bert_id, [msg1.id])
         msg1.refresh()
         # Test: msg1 has Admin and Bert as voters
         self.assertEqual(set(msg1.vote_user_ids), set([user_admin, user_bert]), 'after voting, Admin and Bert are not the voters')