Skip to content

[19.0][MIG] web_notify_upgrade: Migration to 19.0 - #3441

Open
AnmollGarg wants to merge 10 commits into
OCA:19.0from
AnmollGarg:19.0-mig-web_notify_upgrade
Open

AnmollGarg wants to merge 10 commits into
OCA:19.0from
AnmollGarg:19.0-mig-web_notify_upgrade

Conversation

@AnmollGarg

@AnmollGarg AnmollGarg commented Feb 19, 2026

Copy link
Copy Markdown

Migration of web_notify_upgrade to Odoo 19.0.

Depends on:

@AnmollGarg AnmollGarg mentioned this pull request Feb 19, 2026
46 tasks

@alexey-pelykh alexey-pelykh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the migration, @AnmollGarg!

The shift from raw SQL on bus_presence to ORM search on mail.presence with the status field is the right approach for 19.0. The env._() translation calls are also correct. Nice touch adding the DND (manual_im_status != "busy") filtering.

A few observations:

Hardcoded presence constants (ir_model.py lines 9-12) — The constants UPDATE_PRESENCE_DELAY, DISCONNECTION_TIMER, AWAY_TIMER, and PRESENCE_OUTDATED_TIMER are copy-pasted from odoo.addons.mail.models.mail_presence (the commented-out import hints at this). Since the module already depends on mail (transitively through web_notify), these could be imported directly:

from odoo.addons.mail.models.mail_presence import AWAY_TIMER, DISCONNECTION_TIMER

This avoids value drift if upstream changes these timers. That said, looking at _get_active_users_to_notify_of_upgrade, the method doesn't actually use any of these constants — it relies on the status field computed by mail.presence itself. So the constants can simply be removed entirely.

POT file versionweb_notify_upgrade.pot header says Odoo Server 18.0, should be 19.0.

Redundant bus dependencyweb_notify already pulls in bus, so listing it in depends is not necessary. Not blocking, just a note.

CI: tests pass (with OCB and Odoo). The "Detect unreleased dependencies" failure is expected given the dependency on #3377 which is still open.

Overall looks good — the core migration logic is sound.

@AnmollGarg

Copy link
Copy Markdown
Author

@alexey-pelykh I've pushed the fixes!

Removed the redundant bus dependency.

Updated the POT file header to 19.0.

Removed the unused constants from ir_model.py.

Ready for another look!

@AnmollGarg
AnmollGarg force-pushed the 19.0-mig-web_notify_upgrade branch from d7df3a7 to 5822085 Compare March 9, 2026 07:44
@AnmollGarg

Copy link
Copy Markdown
Author

Followed the Technical Method by OCA.

@alexey-pelykh alexey-pelykh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @AnmollGarg — confirmed all three points from my earlier review are addressed in the current diff:

  • Hardcoded presence constants removed from models/ir_model.py_get_active_users_to_notify_of_upgrade now relies purely on the mail.presence status field, as intended.
  • POT header now reads Odoo Server 19.0.
  • Redundant bus dependency dropped — depends is now just ["web_notify"].

One thing to sort out before I can approve, though: the test jobs are currently red on the latest commit (test with OCB and test with Odoo both fail), whereas they were green when I reviewed the earlier commit. Note that Detect unreleased dependencies now passes (it was failing before, while web_notify 19.0 was still open), so the tests may now be exercising the interaction with the released web_notify and catching something. The CI run is also stale (2026-03-09) and its logs have since expired, so I can't see the failure detail.

Could you rebase on the latest 19.0 and re-trigger CI? Once the test jobs are green again I'm happy to approve — the migration logic itself looks sound.

Co-Reviewed-By: Claude Opus 4.8 noreply@anthropic.com

@AnmollGarg
AnmollGarg force-pushed the 19.0-mig-web_notify_upgrade branch from 5822085 to d01cecd Compare July 8, 2026 10:51
@OCA-git-bot OCA-git-bot added series:19.0 mod:web_notify_upgrade Module web_notify_upgrade labels Jul 8, 2026
@AnmollGarg

Copy link
Copy Markdown
Author

Thanks for the review, @alexey-pelykh. I did the rebase to 19.0, but the tests are still failing. It looks like it can't find the 19.0 version of odoo-addon-web_notify:

ERROR: No matching distribution found for odoo-addon-web_notify==19.0.*

@AnmollGarg
AnmollGarg force-pushed the 19.0-mig-web_notify_upgrade branch from ca58722 to b9151e5 Compare July 30, 2026 10:03
@AnmollGarg
AnmollGarg requested a review from alexey-pelykh July 30, 2026 10:07

@petrus-v petrus-v 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.

LGTM with a suggested improvement

Comment on lines +24 to +30
# Find users who are currently online or away
online_presences = self.env["mail.presence"].search(
[("status", "in", ("online", "away"))]
)
users = online_presences.mapped("user_id")
# Respect Do Not Disturb (busy status)
return users.filtered(lambda u: u.manual_im_status != "busy")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe we could improve performance if a lot of users performing a join in SQL (also mapped is useless in such case):

Suggested change
# Find users who are currently online or away
online_presences = self.env["mail.presence"].search(
[("status", "in", ("online", "away"))]
)
users = online_presences.mapped("user_id")
# Respect Do Not Disturb (busy status)
return users.filtered(lambda u: u.manual_im_status != "busy")
# Find users who are currently online or away and not busy
return self.env["mail.presence"].search(
[("status", "in", ("online", "away")), ("user_id.manual_im_status", "!=", "busy")]
).user_id

@AnizR

AnizR commented Sep 9, 2026

Copy link
Copy Markdown

Thanks for your work but, it doesn't work correctly, here is the notification message that it creates:
image

Comment on lines +44 to +53
return dict(
message=self.env._(
"Your odoo instance has been upgraded, please reload the web page."
)
+ "<br />"
'<button onclick="location.reload(true)" class="btn btn-primary mt-4">'
'<i class="fa fa-refresh"></i>' + self.env._("Reload") + "</button>",
title=self.env._("Upgrade Notification"),
sticky=True,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Defining the html in message doesn't work any more.

Here is a proposition of fix

Suggested change
return dict(
message=self.env._(
"Your odoo instance has been upgraded, please reload the web page."
)
+ "<br />"
'<button onclick="location.reload(true)" class="btn btn-primary mt-4">'
'<i class="fa fa-refresh"></i>' + self.env._("Reload") + "</button>",
title=self.env._("Upgrade Notification"),
sticky=True,
)
action = {
'type': 'ir.actions.client',
'tag': 'reload',
"context":{
"params":{
"button_name":self.env._("Reload"),
"button_icon": "fa-refresh"
}
}
}
return dict(
message=self.env._(
"Your odoo instance has been upgraded, please reload the web page."
),
action=action,
title=self.env._("Upgrade Notification"),
sticky=True,
)

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

Labels

mod:web_notify_upgrade Module web_notify_upgrade series:19.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants