-
-
Notifications
You must be signed in to change notification settings - Fork 346
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] sale_timesheet_task_exclude #440
base: 14.0
Are you sure you want to change the base?
[14.0] [mig] sale_timesheet_task_exclude #440
Conversation
* [FIX] sale_timesheet_line_exclude: fix manifest * [FIX] sale_timesheet_task_exclude: fix manifest
Currently translated at 100.0% (3 of 3 strings) Translation: timesheet-12.0/timesheet-12.0-sale_timesheet_task_exclude Translate-URL: https://translation.odoo-community.org/projects/timesheet-12-0/timesheet-12-0-sale_timesheet_task_exclude/de/
Currently translated at 100.0% (3 of 3 strings) Translation: timesheet-12.0/timesheet-12.0-sale_timesheet_task_exclude Translate-URL: https://translation.odoo-community.org/projects/timesheet-12-0/timesheet-12-0-sale_timesheet_task_exclude/es/
4dd6a95
to
12b5306
Compare
12b5306
to
ecfd587
Compare
@alexey-pelykh will you please review this PR? |
@davejames PR is ready for review you can review it. |
@ALL can someone please review the PR? |
It would be great if someone can review this PR. |
@ALL can someone please review the PR. |
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.
Code and functional review.
|
||
task.exclude_from_sale_order = False | ||
self.assertEqual(task.billable_type, "task_rate") | ||
self.assertTrue(timesheet.so_line) | ||
self.assertTrue(task.sale_order_id) |
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.
I don't really follow this change, you don't check that timesheets generated from the task became non-billable.
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.
In v14 it is has changes so I need to do it.
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.
But the change is dramatic, if timesheet still has so_line
on it. Please explain
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.
Yes if the task has allow_billable than timesheet will have so_line. So I have updated the testcase. @alexey-pelykh can you please check now.
ecfd587
to
9b7d838
Compare
It would be great if we get the reviews here |
Hi @davejames |
It would be nice if we could get the reviews here |
@alexey-pelykh can you review this PR? |
The use of this module makes that the hours of timesheets in the I believe this feature is out of the box: clearing the Task "Sales Order Item" field will cause it to not accumulate delivered hours in the project SO. |
@dreispt But in your case all timesheet entries of the task will not get billed. With this module you can exclude just one timesheet entry of the task. So in my opinion this module is not deprecated |
I actually like this improvement, I have often had times where an employee
messed something up and we decide not to bill the client for those hours,
but we still want the timesheet for records and employee payments.
I think this would be worth keeping.
…On Fri, Sep 2, 2022 at 9:19 AM Stefan Wild ***@***.***> wrote:
[image: image]
<https://user-images.githubusercontent.com/24716841/188181183-34b09ca4-8634-409f-97d2-1d4a40e30f1f.png>
—
Reply to this email directly, view it on GitHub
<#440 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHYGNQZICRVP3PWR452RQODV4ILHRANCNFSM4ZAEHABQ>
.
You are receiving this because you are on a team that was mentioned.Message
ID: ***@***.***>
--
Andrew Hastings
*The Catalyst*
Dream Mountain Services
*P:*
*M:*
(801) 770-4869
(435) 890-0526
*W:*
*E:*
DreamMtn.Services <http://dreammtn.services/>
***@***.*** ***@***.***>
<https://www.facebook.com/DreamMtnServices/>
<https://twitter.com/DreamMtnService>
<https://www.linkedin.com/in/andrewdhastings>
<https://www.instagram.com/dreammtnservices/>
Request a Free Odoo/ERP Consultation <http://dreammtn.services/contact-us>
|
I think this one is a feature of sale_timesheet_line_exclude, which is already ported |
Yes you are right ;-) |
@dreispt I think you refer to clearing the SO line ( |
@dsolanki-initos I think for v14 I'd linked |
The compute on sale_line_id is a computed writable one, which means that act as an onchange, but no problem on writing on it regularly. |
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
Maintainers team, Could you please reopen this PR? |
No description provided.