Skip to content

[16.0][ADD] hr_planning_resources#1

Open
antoniodavid wants to merge 4 commits into
OCA:16.0from
BinhexTeam:16.0
Open

[16.0][ADD] hr_planning_resources#1
antoniodavid wants to merge 4 commits into
OCA:16.0from
BinhexTeam:16.0

Conversation

@antoniodavid

@antoniodavid antoniodavid commented Sep 26, 2024

Copy link
Copy Markdown

No description provided.


def has_working_hours(start_dt, end_dt):
"""
Returns `True` if there are any working days between `start_dt` and `end_dt`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be imperative style https://stackoverflow.com/a/54868794
Same in previous docstring

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the tips!!!

@antoniodavid antoniodavid requested a review from rvalyi September 27, 2024 12:29

@victor-champonnois victor-champonnois left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, thank you for the work!
I did a small functional exploration on the interface.

  • maybe a small doc on how to use the module would be nice
  • I went on a project and clicked on "assign planning", there is a confirmation popup, and then nothing. What is this supposed to be doing ?
    image

@FernandoRomera

FernandoRomera commented Oct 10, 2024

Copy link
Copy Markdown

@antoniodavid
Can you split this PR in two:
hr_planning_oca, only with menu app.
hr_planning_resources, all other.

@antoniodavid

Copy link
Copy Markdown
Author

@antoniodavid Can you split this PR in two: hr_planning_oca, only with menu app. hr_planning_resources, all other.

Hi, could you explain the purpose of this feature?

@antoniodavid

Copy link
Copy Markdown
Author

@antoniodavid Can you split this PR in two: hr_planning_oca, only with menu app. hr_planning_resources, all other.

Thanks for the advice!!!

@rrebollo

rrebollo commented Jan 7, 2025

Copy link
Copy Markdown

@antoniodavid @FernandoRomera why the split? I don't get it.

[16.0][ADD] hr_planning_resources

[REF][16.0] hr_planning_resources: Refactoring app

[16.0][ADD] hr_planning_resources: Add readme

[16.0][ADD] hr_planning_resources: Add test-requirements

Revert "[16.0][ADD] hr_planning_resources: Add readme"

This reverts commit 4073ee9.

[16.0][IMP] hr_planning_resources: Refactoring app
@christian-ramos-tecnativa

Copy link
Copy Markdown

I also don't think it's necessary to split it since the scope of the module is clear

@antoniodavid antoniodavid marked this pull request as draft January 28, 2025 13:59
@antoniodavid antoniodavid marked this pull request as ready for review January 28, 2025 20:47
@rrebollo

Copy link
Copy Markdown

Depends on:

I think you can edit and mention this is not really needed anymore.

@antoniodavid

Copy link
Copy Markdown
Author

Depends on:

I think you can edit and mention this is not really needed anymore.

Fixed!!!!

@jelenapoblet jelenapoblet left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

On functional review, everything looks good, i was able to plan task, tickets and projects from the gantt view and viceversa.

@antoniodavid

Copy link
Copy Markdown
Author

ping @victor-champonnois @rvalyi

@api.model_create_multi
def create(self, vals_list):
if self.env.context.get("default_user_id", False):
vals_list[0]["user_id"] = [(4, self.env.context["default_user_id"])]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
vals_list[0]["user_id"] = [(4, self.env.context["default_user_id"])]
vals_list[0]["user_id"] = self.env.context["default_user_id"]

@github-actions

Copy link
Copy Markdown

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants