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.

Sign up for GitHub

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

Jump to bottom

[14.0][IMP] resource_booking: Allow a booking to span more than one calendar day #77

Merged
merged 2 commits into from Aug 28, 2023
Merged

[14.0][IMP] resource_booking: Allow a booking to span more than one calendar day #77

merged 2 commits into from Aug 28, 2023

Conversation

carmenbianca
Copy link
Member

@carmenbianca carmenbianca commented Jan 31, 2023 •
edited

This PR solves two issues:

  • Can't create a booking that lasts from e.g. 23:00 to 1:00.
  • Can't create a booking that lasts several days (e.g. rentals).

Internal task: https://gestion.coopiteasy.be/web#id=9834&action=475&active_id=434&model=project.task&view_type=form&menu_id=536

@OCA-git-bot
Copy link
Contributor

Hi @pedrobaeza,
some modules you are maintaining are being modified, check this out!

@pedrobaeza pedrobaeza added this to the 14.0 milestone Jan 31, 2023
@carmenbianca carmenbianca force-pushed the 14.0-booking-across-midnight branch 4 times, most recently from 87755a8 to ce78001 Compare January 31, 2023 16:55
@carmenbianca carmenbianca marked this pull request as ready for review January 31, 2023 16:57
@carmenbianca
Copy link
Member Author

Ready for review after a fair amount of tinkering.

@carmenbianca
Copy link
Member Author

carmenbianca commented Feb 1, 2023 •
edited

@huguesdk I tried hacking the same result by forcibly setting hour_to to 24.0 on resource.calendar.attendance records -> didn't work.

@carmenbianca carmenbianca mentioned this pull request Feb 9, 2023
Merged
@carmenbianca carmenbianca changed the title [IMP] resource_booking: Allow a booking to span more than one calendar day [14.0][IMP] resource_booking: Allow a booking to span more than one calendar day Feb 9, 2023
huguesdk
huguesdk reviewed Feb 17, 2023
View reviewed changes
Copy link
Member

@huguesdk huguesdk left a comment

Choose a reason for hiding this comment

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

thanks for this. it’s a little sad that this is not handled correctly by the core resource module. adding a comment explaining how this is a workaround for those limitations would be useful (resource.calendar.attendance cannot end at midnight so cannot end exactly at the end of the day (only a few milliseconds before)).

resource_booking/models/resource_booking.py Show resolved Hide resolved
resource_booking/models/resource_booking.py Outdated Show resolved Hide resolved
resource_booking/models/resource_booking.py
# Just ignore those items and pretend they don't exist; all the
# other items appear to make sense.
if item1_date < item0_date:
continue
Copy link
Member

Choose a reason for hiding this comment

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

adding a test for this would be useful.

@carmenbianca carmenbianca marked this pull request as draft February 17, 2023 16:26
@carmenbianca
Copy link
Member Author

This is now rebased on top of #80. Will need to merge that PR before merging this.

@carmenbianca carmenbianca marked this pull request as ready for review February 17, 2023 16:56
@carmenbianca
Copy link
Member Author

Now that #80 is merged, I rebased this on top of 14.0.

remytms
remytms approved these changes May 25, 2023
View reviewed changes
Copy link

@remytms remytms left a comment

Choose a reason for hiding this comment

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

LGTM

@norlinhenrik
Copy link

LGTM
When I configure the resource calender with work hours every day 00:00-23:59, I can book a resource for a month. :)

@pedrobaeza
Copy link
Member

Please squash commits a bit and check pre-commit.

@carmenbianca
[IMP] resource_booking: Allow a booking to span more than one calenda…
ff31f09 
…r day

Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
@carmenbianca
[ADD] resource_booking: Change log entry
267fb8d 
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
@carmenbianca
Copy link
Member Author

Rebased on top of 14.0. Commits squashed. pre-commit works locally.

@pedrobaeza
Copy link
Member

/ocabot merge patch

You can now fw-port it to upper versions.

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-77-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 661a233 into OCA:14.0 Aug 28, 2023
5 checks passed
@OCA-git-bot
Copy link
Contributor

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

@carmenbianca carmenbianca mentioned this pull request Aug 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@huguesdk huguesdk huguesdk left review comments

@remytms remytms remytms approved these changes

Assignees
No one assigned
Labels
merged 🎉
Projects
None yet
Milestone
14.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@carmenbianca @OCA-git-bot @norlinhenrik @pedrobaeza @huguesdk @remytms