[12.0] [MIG] hr_contract_reference #541
Conversation
|
Hey @Nikul-Chaudhary, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
Nikul-Chaudhary
force-pushed
the
12-mig-hr_contract_reference
branch
from
d294996
to
f6e0d1b
Compare
|
@pedrobaeza Could you review please it? |
Nikul-Chaudhary
force-pushed
the
12-mig-hr_contract_reference
branch
from
f6e0d1b
to
46c86df
Compare
|
@tarteo @pedrobaeza COuld you please review it ? |
|
@Nikul-Chaudhary Increase the test coverage |
There was a problem hiding this comment.
code lgtm
please use the maintainer-tools to generate the hr_contract_reference/readme file
Nikul-Chaudhary
force-pushed
the
12-mig-hr_contract_reference
branch
3 times, most recently
from
816c78f
to
5ac5451
Compare
|
@Nikul-Chaudhary Will you keep working on this? |
Yes Sure @jarroyomorales |
…Management modules
|
@Nikul-Chaudhary Hi! How is this going? 😃 |
|
Hi @Nikul-Chaudhary, I just did a PR to your branch fixing the readme as suggested by @alexey-pelykh and added a mini test to increase coverage. Can you check? |
|
@Nikul-Chaudhary Could you squash the 3 commits? |
|
@Nikul-Chaudhary Functional Test, LGTM 👍 |
|
@jarroyomorales @Saran440 Sure, I will |
Nikul-Chaudhary
force-pushed
the
12-mig-hr_contract_reference
branch
from
babb94b
to
cb2f403
Compare
|
@jarroyomorales @Saran440 Done |
|
/ocabot merge nobump |
|
This PR looks fantastic, let's merge it! |
|
@Saran440 @pedrobaeza What happened with this merge? |
|
@jarroyomorales Maybe this pr have CR and not approved yet. |
|
Yes, but all changes have been attended, so we need to wait for them to recheck? |
| name = fields.Char( | ||
| 'Contract Reference', | ||
| required=False, | ||
| readonly=True, |
There was a problem hiding this comment.
Is it really needed to co-exist req=False and readonly=True?
There was a problem hiding this comment.
IMO name should be required, and even more if you are using this module, where you shouldnt have a contract without reference.
There was a problem hiding this comment.
It should be required=False on DB and required=true on UI
|
Hi @alexey-pelykh! Could you recheck please? Thank you! |
|
/ocabot merge nobump |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at 6a7abff. Thanks a lot for contributing to OCA. ❤️ |
HR Contract Reference
This module was written to extend the functionality of employees contracts to support sequence of contract reference which will be generated automatically from the sequence predefined.