[FIX] test_booking_engine: set pickup/return times#2233
Conversation
|
|
abdrahmanrashed
force-pushed
the
saas-19.1-test-booking-engine-fix-overbooking-ecommerce-time-aras
branch
from
71c3d85 to
75b1156
Compare
frva-odoo
left a comment
frva-odoo
left a comment
There was a problem hiding this comment.
Hello 👋 Vava asked me to have a look as I'm working with timezones too lately, I have two comments about this. Not sure what the best way to do this but we could have a talk about it if needed.
| self.assertEqual(response['tracking_info'][0]['item_id'], self.product.id, "No item found in shopping cart.") | ||
| sale_order = self.env['sale.order'].search([], limit=1) | ||
| website_tz = ZoneInfo(self.env.company.website_id.tz) | ||
| picking_time = datetime.combine(self.start_date, float_to_time(self.product.pickup_time)) - timedelta(seconds=website_tz.utcoffset(self.start_date).total_seconds()) |
There was a problem hiding this comment.
Seems like utcoffset() can return a day difference, which I'm pretty sure is not wanted here in the time argument of datetime.combine() (You could go over the limit and so be off by a day in the result)
There was a problem hiding this comment.
That doesn't matter because the pickup and return times are fixed in the test case
| self.assertEqual(response['tracking_info'][0]['item_id'], self.product.id, "No item found in shopping cart.") | ||
| sale_order = self.env['sale.order'].search([], limit=1) | ||
| website_tz = ZoneInfo(self.env.company.website_id.tz) | ||
| picking_time = datetime.combine(self.start_date, float_to_time(self.product.pickup_time)) - timedelta(seconds=website_tz.utcoffset(self.start_date).total_seconds()) |
There was a problem hiding this comment.
Overall I find it pretty weird to use combine for a day and a time, and calculate the difference in timezone in seconds.
Wouldn't it be better if we changed the timezone, applied changes then come back ? That way we are sure to have the right time set in our db :
| picking_time = datetime.combine(self.start_date, float_to_time(self.product.pickup_time)) - timedelta(seconds=website_tz.utcoffset(self.start_date).total_seconds()) | |
| picking_time = self.start_date.astimezone(website_tz).replace(hour=self.product.pickup_time).astimezone(UTC).replace(tzinfo=None) |
What do you think about this approach, is this correctly what we want ?
There was a problem hiding this comment.
I did it like that to be coherent with the respective code in _cart_add that applied this logic.
Your suggestion is shorter, but honestly I am not sure if there is any functional difference between the 2 implementations in this case. I guess I will keep it as is unless there is a reason to change.
| self.assertEqual(response['tracking_info'][0]['item_id'], self.product.id, "No item found in shopping cart.") | ||
| sale_order = self.env['sale.order'].search([], limit=1) | ||
| website_tz = ZoneInfo(self.env.company.website_id.tz) | ||
| picking_time = datetime.combine(self.start_date, float_to_time(self.product.pickup_time)) - timedelta(seconds=website_tz.utcoffset(self.start_date).total_seconds()) |
Before this commit there were not pickup/return times added for product in overbooking ecommerce tests, which caused the start/end times of the rental order to be different from the intended start/end times. After this commit, the pickup/return times have been set and the tests now check that the sale order time is exactly the same as the pickup/return times, instead of withing a relative possible timeframe as how it was.
abdrahmanrashed
force-pushed
the
saas-19.1-test-booking-engine-fix-overbooking-ecommerce-time-aras
branch
from
75b1156 to
0dad83f
Compare
|
@robodoo r+ |
Before this commit there were not pickup/return times added for product in overbooking ecommerce tests, which caused the start/end times of the rental order to be different from the intended start/end times. After this commit, the pickup/return times have been set and the tests now check that the sale order time is exactly the same as the pickup/return times, instead of withing a relative possible timeframe as how it was. closes #2233 Signed-off-by: François Vasamillet (frva) <frva@odoo.com>
4 participants
Before this commit there were not pickup/return times added for product in overbooking ecommerce tests, which caused the start/end times of the rental order to be different from the intended start/end times. After this commit, the pickup/return times have been set and the tests now check that the sale order time is exactly the same as the pickup/return times, instead of withing a relative possible timeframe as how it was.