Review: Needs Fixing
Hello,
Thanks for the merge proposal update. Here are some comments !
**Views**
Remove "<separator string="Temporary Need Action" colspan="4"/>
<field name="need_action_user_id"/>
<newline/>" everywhere it appears
I know it is writen in hr_holidays, but this is only for debug purpose (I will
get it out of the module). Maybe it will appear in some objects, but by default
remove it everywhere. Keep only the thread widget.
In Leads and Opportunities forms, the "Send new email" button goes to the next
line, on the left of the screen. This is not very user friendly, it should be
under other action buttons.
**Lead**
Lead is converted to an opportunity. -> Lead has been converted ...
**Opportunity**
Scheduling a meeting: messages are not very easy to understand: "Meeting has
been scheduled on 2012-03-08 07:00:00 for opportunity. ", should be more like:
- for opportunity: "The meeting Meeting_name has been scheduled on Date."
- for meeting: "Meeting linked to the opportunity Opportunity_name has been
created and scheduled on Date."
Scheduling a phone call: on opportunity, I get only " log a call" message, that
is not very clear, and not very nice. In the phonecall, I have state changes,
but no messages stating it has been created from the opportunity. it should be
something like the meeting creation.
**crm.py**
- Why is "def _case_phonecall_notification(self, case, action, context=None)"
defined in crm_base ?
- Why do you use a leading underscore for each case_XXX_notification method ?
Underscore generally stands for private methods [1]. As those methods are part
of the public API of crm in order to be overwriten, they should not be private
in my opinion.
- By the way, methods usually use verbs while variables use nouns. I think
methods like case_open_send_note(self, cr, uid, ...) or create_send_note
instead of case_open_notification and create_notification (which seems to be
the method to create a notification) are more easy to understand. I think we
should use the send_note suffix for our methods, that is coherent with our
message_append_note API method.
Double-check your sentences; ex, in _case_partner_notification in crm.py:
message = _("Partner has <b>created</b> for this %s") % ('Opportunity' if
lead.type == 'opportunity' else 'Lead')
--> partner has **been** created
In _convert_opportunity_notification: change success_message variable name to
message.
**crm_lead**
In write: change sentence from "Changed to..." to "Stage changed to..." .
Hope everything is clear !
Best regards,
Thibault.
[1]: http://docs.python.org/tutorial/classes.html#private-variables
--
https://code.launchpad.net/~openerp-dev/openobject-addons/trunk-social-crm-bth/+merge/96754
Your team OpenERP R&D Team is subscribed to branch
lp:~openerp-dev/openobject-addons/trunk-social-tde.
_______________________________________________
Mailing list: https://launchpad.net/~openerp-dev-gtk
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openerp-dev-gtk
More help : https://help.launchpad.net/ListHelp
