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

[14.0] [MIG] hr_applicant: Migrated the module in v14. #522

Conversation

Chandresh-SerpentCS
Copy link
Contributor

  • Module developed by Pragati Patel. (Assigned by Yogesh Mahera)

@Chandresh-SerpentCS
Copy link
Contributor Author

@YogeshMahera-SerpentCS Please review the PR. Thanks.

@@ -0,0 +1,16 @@
OpenERP, Open Source Management Solution
Copyright (C) 2011-Today Serpent Consulting Services Pvt. Ltd.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2020-Today

@@ -0,0 +1,29 @@
==================
HR Applicant
==================

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use = til 't'

"test": [],
"installable": True,
"auto_install": False,
"application": True,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may remove "demo", "test" attributes as not any value has been assigned to it. Here, "auto_install" is by default False, so we may remove it.

and datetime.datetime.strftime(
rec.training_start_date, DEFAULT_SERVER_DATE_FORMAT
)
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, We may optimise this as this version may accept the comparison/assignment directly by datetime object.

if not rec.training_attendees:
raise ValidationError(
_("Training Attendees should not be Zero!"))
rec.write({"state": "approved"})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can avoid this type of pattern for assignment rather we can use rec.state="approved".

raise ValidationError(
_("Training Attendees should not be Zero!"))
rec.write({"state": "approved"})
return True

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, may no need to return True. Please check for this.

)
)
self.write({"state": "cancel"})
return True

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, we can change the all state assignment and please check for return True if we can remove it.

for rec in self:
if str(rec.training_start_date) < datetime.datetime.now().strftime(
DEFAULT_SERVER_DATE_FORMAT
):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, we may compare datetime object directly, please check if we can remove string conversion , we may use fields.Datetime.now() rather using python lib datetime.

# See LICENSE file for full copyright and licensing details.

from odoo import models, fields, api
import datetime

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove import datetime if not used elsewhere.

@@ -0,0 +1,55 @@
# See LICENSE file for full copyright and licensing details.

from odoo import models, fields, api

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import should be in alphabetical order.

for rec in self:
class_obj = self.env["training.class"]
applicant = self.env["hr.applicant"].search(
[("id", "=", self._context.get("active_id"))])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, we can define self.env[''] object outside the loop.

@pragati-patel98
Copy link
Contributor

@ForamSerpentCS I solved the comments, Can you please review it?

@JayVora-SerpentCS JayVora-SerpentCS merged commit faa2a69 into JayVora-SerpentCS:14.0 Dec 1, 2020
@Chandresh-SerpentCS Chandresh-SerpentCS deleted the 14.0_MIG_hr_applicant branch July 19, 2021 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.