-
-
Notifications
You must be signed in to change notification settings - Fork 631
[18.0][IMP] date_range: Set multicompanies support for the date.range model #1172
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
base: 18.0
Are you sure you want to change the base?
Conversation
|
Hi @lmignon, |
cav-adhoc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Hi @lmignon can you review this PR? Thanks ❤️ |
|
Hello @StefanRijnhart , @sbidoul @lmignon ✋🏼 Please could you please take a look here just a quick review? 🙏🏼 😄 Thank you!!!! ❤️ |
|
@Andrii9090 can you rebase it for review it functionally? Thank you. |
9145b7e to
11d2e14
Compare
|
@EmilioPascual runboat is available, you can functionally review it. |
EmilioPascual
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Functional and code review.
Good work @Andrii9090
|
This PR has the |
|
Hello @StefanRijnhart , @sbidoul @lmignon Please could you take a look this PR just a quick review? |
Gelojr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Congratulations on the great work on this PR @Andrii9090 .
The following functional tests have been performed with the results indicated below.
- Test 1: Creation of a minimal Date Range Type – OK
- Test 2: Multi-company Date Range creation using company_ids – OK
- Test 3: Creation of a global Date Range without companies – OK
- Test 4: Overlap blocking when Date Ranges share at least one company and overlap is not allowed – OK
- Test 5: Overlap allowed when Date Ranges do not share any company – OK
- Test 6: Consistency check between Date Range Type company_id and Date Range company_ids – OK
- Test 7: Generate Date Ranges wizard creates Date Ranges with company_ids correctly set – OK
- Test 8: Multi-company record rule visibility based on company_ids – OK
- Test 9: Generate Date Ranges wizard with empty Company field generates global Date Ranges only (no duplication per company) – OK
yajo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review.
11d2e14 to
11c63e8
Compare
11c63e8 to
f6c266d
Compare
yajo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments to improve code. Mostly OK. Thanks!
The date.range model currently supports only a single company (company_id). This requires creating multiple date ranges for different companies, which complicates management. To resolve this, replace the company_id field with company_ids (Many2many). MT-10678
f6c266d to
f63cdf8
Compare
The date.range model currently supports only a single company (
company_id).This requires creating multiple date ranges for different companies, which complicates management.
To resolve this, we replace the field
company_idwithcompany_ids(Many2many) in the modeldate.rangeIt's a short video with funcional test
https://www.loom.com/share/f1a0719eafa443058e3a0befc0eed9b4?sid=fbbd55c0-f75d-48aa-b9ec-d93e28af320f
Issue
@moduon
MT-10678