Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.0] ADD module 'hr_timesheet_validators'. #19

Closed
wants to merge 8 commits into from

Conversation

Projects
None yet
6 participants
@jbeficent
Copy link
Member

commented Jan 14, 2015

In the following post you can see a description of the functions performed by the module.
http://www.eficent.com/blog_en/timesheet-validators-in-odoo/

Regards,
Jordi.

@jbeficent jbeficent changed the title 7.0 [7.0] ADD module 'hr_timesheet_validators'. Jan 14, 2015

from openerp import netsvc


class hr_timesheet_sheet(osv.osv):

This comment has been minimized.

Copy link
@tafaRU

tafaRU Jan 15, 2015

Member

Please use orm.Model instead of osv.osv

# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
##############################################################################
import model

This comment has been minimized.

Copy link
@tafaRU

tafaRU Jan 15, 2015

Member

Please use relative import

# along with this program. If not, see <http://www.gnu.org/licenses/>.
#
##############################################################################
import hr_timesheet_sheet

This comment has been minimized.

Copy link
@tafaRU

tafaRU Jan 15, 2015

Member

Same as above

for timesheet in self.browse(cr, uid, ids):
res[timesheet.id] = []
users[timesheet.id] = []
if timesheet.employee_id \

This comment has been minimized.

Copy link
@tafaRU

tafaRU Jan 15, 2015

Member

You should consider to use brackets instead of \
I mean something like this:

if  (
    timesheet.employee_id
    and timesheet.employee_id.parent_id
    and timesheet.employee_id.parent_id.user_id
):
- The head of the department that the employee belongs to
In case that the employee is head of the department, it will attempt to add the head of the parent department instead.

This comment has been minimized.

Copy link
@tafaRU

tafaRU Jan 15, 2015

Member

E501 line too long (118 > 79 characters)

@jbeficent

This comment has been minimized.

Copy link
Member Author

commented Jan 15, 2015

Thanks for your feedback Alex. I have now incorporated your recommended changes and submitted a new commit.

@tafaRU

This comment has been minimized.

Copy link
Member

commented Jan 15, 2015

Thanks to you for your contribution, @jbeficent 😉

@ehdem

This comment has been minimized.

Copy link

commented Jan 16, 2015

Can you add a help option when you define a new field; It is useful.

@jgrandguillaume

This comment has been minimized.

Copy link
Member

commented Apr 28, 2015

Hi @jbeficent ,

Thanks for this contrib.

  • Usually, we always ask for one PR per module, easier for reviewers. That would be nice that you'll split it.
  • Would also be nice if you use the OCA template for the description (not forgetting OCA as author otherwise it won't appear in Apps correctly)

Functionally speaking, it works well, once those points are ficed, you get my 👍

Regards,

@pedrobaeza

This comment has been minimized.

Copy link
Member

commented Apr 28, 2015

Please put module name in singular: hr_timesheet_validator

@jbeficent

This comment has been minimized.

Copy link
Member Author

commented Apr 30, 2015

Hello,

I just submitted a new PR for the module 'hr_timesheet_validator' only: #31

I am closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.