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] sale_timesheet_task_exclude #440

Open
wants to merge 14 commits into
base: 14.0
Choose a base branch
from

Conversation

dsolanki-initos
Copy link
Contributor

No description provided.

@dsolanki-initos dsolanki-initos force-pushed the 14.0-mig-sale_timesheet_task_exclude branch 2 times, most recently from 4dd6a95 to 12b5306 Compare March 11, 2021 13:30
@dsolanki-initos dsolanki-initos mentioned this pull request Mar 12, 2021
17 tasks
@dsolanki-initos dsolanki-initos force-pushed the 14.0-mig-sale_timesheet_task_exclude branch from 12b5306 to ecfd587 Compare March 12, 2021 05:47
@dsolanki-initos
Copy link
Contributor Author

@alexey-pelykh will you please review this PR?

@dsolanki-initos
Copy link
Contributor Author

@davejames PR is ready for review you can review it.

@dsolanki-initos
Copy link
Contributor Author

@ALL can someone please review the PR?

@dsolanki-initos
Copy link
Contributor Author

It would be great if someone can review this PR.

@dsolanki-initos
Copy link
Contributor Author

@ALL can someone please review the PR.

Copy link
Contributor

@fshah-initos fshah-initos left a 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)
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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

Copy link
Contributor Author

@dsolanki-initos dsolanki-initos Jun 1, 2021

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.

@dsolanki-initos dsolanki-initos force-pushed the 14.0-mig-sale_timesheet_task_exclude branch from ecfd587 to 9b7d838 Compare June 1, 2021 11:03
@dsolanki-initos
Copy link
Contributor Author

It would be great if we get the reviews here

@dsolanki-initos
Copy link
Contributor Author

Hi @davejames
It would be nice you can add reviews here.

@dsolanki-initos
Copy link
Contributor Author

It would be nice if we could get the reviews here

@dsolanki-initos
Copy link
Contributor Author

@alexey-pelykh can you review this PR?

@dreispt
Copy link
Member

dreispt commented Sep 2, 2022

The use of this module makes that the hours of timesheets in the
tasks excluded, will not appear or taken in consideration as
"Quantity Delivered" in the Sales Order Lines and Sale Order Reports.

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.

image

@wildi1
Copy link

wildi1 commented Sep 2, 2022

@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

@wildi1
Copy link

wildi1 commented Sep 2, 2022

image

@andrewdhastings
Copy link

andrewdhastings commented Sep 2, 2022 via email

@francesco-ooops
Copy link
Contributor

@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 think this one is a feature of sale_timesheet_line_exclude, which is already ported

@wildi1
Copy link

wildi1 commented Sep 2, 2022

@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 think this one is a feature of sale_timesheet_line_exclude, which is already ported

Yes you are right ;-)

@alexey-pelykh
Copy link
Contributor

alexey-pelykh commented Sep 3, 2022

@dreispt I think you refer to clearing the SO line (sale_line_id)? But this migration targets https://github.com/odoo/odoo/blob/14.0/addons/sale_timesheet/models/project.py#L284

@alexey-pelykh
Copy link
Contributor

@dsolanki-initos I think for v14 I'd linked non_allow_billable (https://github.com/odoo/odoo/blob/14.0/addons/sale_timesheet/models/project.py#L224) with this module somehow, yet since it's not there for v15 not sure what's the best approach

@pedrobaeza
Copy link
Member

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.

@github-actions
Copy link

github-actions bot commented Jan 1, 2023

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.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jan 1, 2023
@github-actions github-actions bot closed this Feb 5, 2023
@dsolanki-initos
Copy link
Contributor Author

Maintainers team, Could you please reopen this PR?

@rvalyi rvalyi reopened this Mar 6, 2023
@rvalyi rvalyi added no stale Use this label to prevent the automated stale action from closing this PR/Issue. and removed stale PR/Issue without recent activity, it'll be soon closed automatically. labels Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no stale Use this label to prevent the automated stale action from closing this PR/Issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.