[IMP] booking_channex: consolidate Daily Sync - #27
Conversation
3b24ed6 to
4b81c19
Compare
|
Hello @chga-odoo, can you do a quick review on this before merging ? Thanks ! |
chga-odoo
left a comment
chga-odoo
left a comment
There was a problem hiding this comment.
@frva-odoo great work 🫡,
Flow looks good. I have only two suggestions
Thanks!
| try: | ||
| method, endpoint = env.context.get('method'), env.context.get('endpoint') | ||
| method, endpoint, res = env.context.get('method'), env.context.get('endpoint'), False | ||
| if not method or method not in ["GET", "POST", "PUT", "DELETE"]: raise UserError("Http request to channex cannot be sent because method is invalid (not GET, POST, PUT or DELETE)") | ||
| if not endpoint: raise UserError("Http request to channex cannot be sent because endpoint is empty") |
There was a problem hiding this comment.
I think this part (variable initialization and the UserError validations) should be outside the try block. Otherwise, if a local error occurs before the HTTP request is sent, it will be reported as a Channex API error even though it's unrelated to Channex.
| except Exception as e: log_sync_error_message('POST', 'restrictions', f"Daily sync: Failed to send rates to rate plans; error is :\n{str(e)}") | ||
|
|
||
| self.env['ir.cron']._commit_progress(remaining=2) | ||
| log_sync_error_message('/', '/', "Daily sync: The daily synchronization finished running") |
There was a problem hiding this comment.
if we want use this function also for info message(non error). than we should change name of this function log_sync_message
4b81c19 to
f4d04a0
Compare
|
Thanks for the review @chga-odoo. Looks okay to merge ? |
Yes, Looks okay 👍 |
8f3b2e6
into
saas-19.3-booking-channex-main-branch-frva
2 participants
Task-5914258