[IMP] booking_channex: add availability and rate push to Channex#10
Conversation
frva-odoo
left a comment
frva-odoo
left a comment
There was a problem hiding this comment.
Great work @chga-odoo, I will probably have to go through this a second time after your changes as this is a pretty hard feature. Don't hesitate to answer the comments if you want to discuss about the issue I raised. Anyway really solid for a first version, good job !
| rules = env['product.pricelist.item'].search([ | ||
| ('company_id', '=', property.id) | ||
| ]) | ||
|
|
||
| for rule in rules: |
There was a problem hiding this comment.
I don't think we use those rules for the rates for channex, the prices are currently defined by the product variant price with its attribute values and a linked pricelist on x_rate_plan.
I want to be sure so let's ask SAVP as he should come back to work tomorrow but I don't think we need to bother with this.
There was a problem hiding this comment.
@frva-odoo @chga-odoo
the pricelist.item wont be pushed directly to channex
but leveraged to compute the daily price to push to channex
idea is that a product / room type has a default price and that every day, the price list item will increase or decrease it to be competitive or marge more
therefore pricelist items will ultimately be valid for 1 day and for 1 product / variant / room type (let's consolidate the granularity level later)
and if the question is "should this be part of the channex module or not", i'd say no while that to compute the daily price of a room (ARI) we'll use odoo compute price method, which leverage price rules if they exist
let me know if question was different
| rules = env['product.pricelist.item'].search([ | ||
| ('company_id', '=', property.id) | ||
| ]) | ||
|
|
||
| for rule in rules: |
There was a problem hiding this comment.
Defining the payload inside the for loop is good in a minimal setup, but if two pricelists are overlapping in dates you will send the first rule's rates then erase it with the second in the same http call.
Isn't this something we should take into account and avoid ? Don't we want to apply every relevant pricerule on a final rate ?
| if rule.product_tmpl_id: | ||
| if rule.product_id: | ||
| room_set = property_rooms.filtered(lambda r: r.id == rule.product_id.id) | ||
| else: | ||
| room_set = property_rooms.filtered(lambda r: r.product_tmpl_id.id == rule.product_tmpl_id.id) |
There was a problem hiding this comment.
Easier to read if you do an if rule.product_id ... elif rule.product_tmpl_id ...
|
|
||
| for room in room_set: | ||
| room_mapping = env['x_channex_mapping'].search([ | ||
| ('x_model_type', '=', 'room_type'), | ||
| ('x_local_id', '=', room.id) | ||
| ], limit=1) |
There was a problem hiding this comment.
_read_group before the for loop
| for room in room_set: | |
| room_mapping = env['x_channex_mapping'].search([ | |
| ('x_model_type', '=', 'room_type'), | |
| ('x_local_id', '=', room.id) | |
| ], limit=1) | |
| room_mappings_dict = dict(env['x_channex_mapping']._read_group(domain=[ ('x_model_type', '=', 'room_type'), ('x_local_id', 'in', room_set.ids)], groupby=['x_local_id'], aggregates=['x_remote_id:max'])) | |
| for room in room_set: | |
| room_mapping = room_mappings_dict.get(room, False) |
| rate_plan = env['x_rate_plan'].search([ | ||
| ('x_variant_id', '=', room.id), | ||
| ('x_pricelist_id', '=', rule.pricelist_id.id) | ||
| ], limit=1) |
There was a problem hiding this comment.
There are multiple ones like this if the product has the attribute "Rate Plan" (as it creates multiples with a different x_rate_plan_value_id) , so we cannot take only the first one and ignore the others.
| try: | ||
| resp = env.ref('booking_channex.requests_to_channex').with_context(method="POST", endpoint="availability", payload=payload).run() | ||
| resp.raise_for_status() | ||
| except Exception as e: | ||
| raise UserError(f"Error posting availability: {str(e)}") |
There was a problem hiding this comment.
Same, the try except is already in requests_to_channex
| is_same_qty = rec.x_available == current_qty | ||
| is_consecutive = ( | ||
| rec.x_date == | ||
| current_end + datetime.timedelta(days=1) | ||
| ) |
There was a problem hiding this comment.
What are those for ? I don't see them used anywhere
| if (rec.x_available == current_qty and rec.x_date == current_end + datetime.timedelta(days=1)): | ||
| current_end = rec.x_date |
There was a problem hiding this comment.
What happens here if they are not consecutive days ? I see they are ordered when passed to the method so it would mean there's a gap in the availabilities.
Shouldn't we send an availability of 0 for each day that is not represented (this shouldn't happen with the way it works but as a safe way) ? Or raise an error as this is unusual ?
There was a problem hiding this comment.
We have availability records for individual dates, and we are merging consecutive dates into a single date range to reduce the payload size.
If the quantity is the same for consecutive dates, we can merge them. In most cases the dates will already be consecutive, but for extra safety I added an additional check for that as well.
Regarding 0 availability, the API accepts quantity as a non-negative integer, so 0 should be valid and should not raise any error. We can also handle the 0 quantity case in our code.
What do you think — should we still add any additional validation/check specifically for 0 availability?
There was a problem hiding this comment.
I see why you made it that way.
I think that at least as a first version, it may be overcomplicating to add this as this is a case that should not happen, and the errors are planned to be detected in the 24h synchronization cron.
So for now you can ignore this edge case and move on, that would be unproductive to manage this here and now I think
| payload_values = [] | ||
| rooms = env['product.product'].search([('x_is_a_room_offer', '=', True), ('x_resource_ids', '!=', False)]) | ||
| for property in companies: | ||
| property_mapping = env['x_channex_mapping'].search([('x_model_type', '=', 'property'),('x_local_id', '=', property.id)], limit=1) |
|
|
||
| for room in property_rooms: | ||
| availability_list = [] | ||
| room_mapping = env['x_channex_mapping'].search([('x_model_type', '=', 'room_type'),('x_local_id', '=', room.id)], limit=1) |
Added code and logic to push availability and rate data to Channex with proper payload structure handling. Implemented payload preparation and data mapping. Task: 5913074
chga-odoo
force-pushed
the
saas-19.3-booking-channex-ari-push-chga
branch
from
50736ce to
2f2e429
Compare
3 participants
Added code and logic to push availability and rate data to Channex with proper payload structure handling.
Implemented payload preparation and data mapping.
Task: 5913074