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

[16.0][MIG] hr_timesheet_sheet #580

Merged
merged 186 commits into from
Jun 29, 2023
Merged

Conversation

tarteo
Copy link
Member

@tarteo tarteo commented Mar 21, 2023

No description provided.

MiquelRForgeFlow and others added 30 commits February 28, 2023 14:40
* [10.0] hr_timesheet_sheet

* [11.0][MIG] hr_timesheet_sheet

* [REMOVE] hr_timesheet.sheet.account

* [REMOVE] 'new' state

* [ADD] Tests

* [UPD] Adapt to multicompany

* [ADD] Add more tests (include multicompany tests)

* [FIX] project_task_stage_allow_timesheet: show error message only if task

* [ADD] Migration scripts to v11
Currently translated at 98.9% (88 of 89 strings)

Translation: hr-timesheet-11.0/hr-timesheet-11.0-hr_timesheet_sheet
Translate-URL: https://translation.odoo-community.org/projects/hr-timesheet-11-0/hr-timesheet-11-0-hr_timesheet_sheet/ja/
Currently translated at 100,0% (89 of 89 strings)

Translation: hr-timesheet-11.0/hr-timesheet-11.0-hr_timesheet_sheet
Translate-URL: https://translation.odoo-community.org/projects/hr-timesheet-11-0/hr-timesheet-11-0-hr_timesheet_sheet/pt_BR/
…ay into this module, which adds a configuration to select the week start day.
Updated by Update PO files to match POT (msgmerge) hook in Weblate.
If you run tests on Sunday, test_4 was not prepared for it, as next day is Monday,
which belongs to other week, and thus not included in same timesheet. With this,
we cover that case, decreasing one day instead of adding it.
Updated by Update PO files to match POT (msgmerge) hook in Weblate.
@tarteo tarteo force-pushed the 16-mig-timesheet-sheet branch from 7dbd67f to 9403521 Compare March 24, 2023 12:05
Copy link
Member

@astirpe astirpe left a comment

Choose a reason for hiding this comment

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

Tested on runboat, works well and also the matrix has the same functionality as it was in previous versions. Great work @tarteo 👍

)
def _compute_available_task_ids(self):
project_task_obj = self.env["project.task"]
for rec in self:
Copy link
Contributor

Choose a reason for hiding this comment

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

some nitpick only if you want. instead of rec it will be better to have sheet as a variable name.

@@ -26,10 +26,4 @@
"views/account_analytic_line_views.xml",
"views/res_config_settings_views.xml",
],
"assets": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean, it is working from the web_widget_x2many_2d_matrix itself which was customized in those js and scss

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this didn't worked in 15.0 either btw.

@micheledic
Copy link

micheledic commented May 8, 2023

I tried the PR but there is a bug but not caused by the PR itself.
It is caused by the native write function that does not consent to write on analytic line that are connected to an holiday .

To replicate the issue:

  • Create a time off request and approve it
  • Create a hr timesheet_sheet with the date range that contains this time off analytic line
    The error appears .

image

I did this PR on native odoo code that will fix this issue allowing to set sheet_id also on timeoff analytic lines
odoo/odoo#120597

@tarteo
Copy link
Member Author

tarteo commented May 10, 2023

@micheledic Thanks for testing. Yes this comes from native Odoo indeed, thanks for creating a PR to fix this issue!!

@micheledic
Copy link

micheledic commented May 10, 2023

@micheledic Thanks for testing. Yes this comes from native Odoo indeed, thanks for creating a PR to fix this issue!!

odoo/odoo#120597 My PR is approved! Check the changes so you can full migrate the module ;)

Copy link

@AntoniRomera AntoniRomera left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@Yadier-Tecnativa Yadier-Tecnativa left a comment

Choose a reason for hiding this comment

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

You need to squash the administrative commits, in this case for "Translated using weblate" there are 34 commits, for "Update translation files" 26 and for "Update hr_timesheet_sheet.pot" 18, so there are at least 78 which can be reduced from 184 total.

TT43226

@micheledic
Copy link

@micheledic Thanks for testing. Yes this comes from native Odoo indeed, thanks for creating a PR to fix this issue!!

odoo/odoo#120597 My PR is approved! Check the changes so you can full migrate the module ;)

@tarteo i Guess you have also to implement that check or can't be created a sheet in a periodi with timeoff analytic line!

@tarteo
Copy link
Member Author

tarteo commented Jun 29, 2023

@micheledic Fixed it!

@astirpe
Copy link
Member

astirpe commented Jun 29, 2023

Is it ok to merge this one?

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@pedrobaeza
Copy link
Member

/ocabot migration hr_timesheet_sheet
/ocabot merge nobump

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Jun 29, 2023
@OCA-git-bot OCA-git-bot mentioned this pull request Jun 29, 2023
16 tasks
@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-580-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit e93ba7f into OCA:16.0 Jun 29, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at bf75fc4. Thanks a lot for contributing to OCA. ❤️

@chicco785
Copy link

@micheledic Thanks for testing. Yes this comes from native Odoo indeed, thanks for creating a PR to fix this issue!!

odoo/odoo#120597 My PR is approved! Check the changes so you can full migrate the module ;)

@tarteo i Guess you have also to implement that check or can't be created a sheet in a periodi with timeoff analytic line!

I tried the PR but there is a bug but not caused by the PR itself. It is caused by the native write function that does not consent to write on analytic line that are connected to an holiday .

To replicate the issue:

  • Create a time off request and approve it
  • Create a hr timesheet_sheet with the date range that contains this time off analytic line
    The error appears .

image

I did this PR on native odoo code that will fix this issue allowing to set sheet_id also on timeoff analytic lines odoo/odoo#120597

what's the configuration required to avoid this error message? I am running odoo 16.0 with OCA timesheet_sheet and I get it :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.