Skip to content

Conversation

@tishmen
Copy link

@tishmen tishmen commented Oct 10, 2025

Scope

  • impersonate_login

Depends on

  • None

Summary

  • Migration to Odoo 19.0; tests included. No functional changes beyond compatibility.
  • History preserved via git format-patch --keep-subject from upstream/18.0 and git am -3 --keep onto upstream/19.0.
  • Commit structure kept clean for review:
    • [IMP] pre-commit auto fixes (single commit)
    • [MIG] migration to 19.0 (single commit)

Changes (highlights)

  • Use self.env._ for translations and lazy placeholders where needed.
  • Replace deprecated self._uid with self.env.uid.
  • Adapt mail.thread override to 19.0 signature: _message_compute_author(self, author_id=None, email_from=None).
  • Avoid attribute access on request.session; use session.get("impersonate_from_uid").
  • Security XML: res.groups field renamed to user_ids.
  • Manifest: version bumped to 19.0.1.0.0, website points to 19.0 path.
  • JS: backend user menu integration kept; prettier/eslint auto-fixes applied.
  • Tests: resilient on 19.0 runner (no demo data by default) – create demo user and grant required groups (base.group_user, base.group_partner_manager).

Temporary references

  • None

Pre-commit

  • Ran locally; auto-fixes committed as a single [IMP] commit.

Tests

  • Locally verified with fresh DB using --test-tags "/impersonate_login".
  • CI is expected to run pre-commit and the module tests against Odoo 19 and OCB.

Review

  • Files changed show the result on top of 19.0. For an easier review, consider commit-by-commit: import history (via am) → [IMP] pre-commit → [MIG] migration.

@tishmen
Copy link
Author

tishmen commented Oct 10, 2025

@legalsylvain Opened a new PR based on the OCA 18.0 → 19.0 migration guidelines: #849. It preserves history via format-patch/am from upstream/18.0, with two commits ([IMP] pre-commit auto-fixes, [MIG] migration). Please review commit-by-commit. I am closing the previous PR as this current approach looks good.

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Diff review. LGTM, but I don't use this module, so I let authors make a deeper review.

Two minors remarks inline.

Thanks a bot for the work.

@tishmen
Copy link
Author

tishmen commented Oct 15, 2025

Hi @legalsylvain, sorry for the late response. I’ve resolved the noted issues and pushed the updates. Thanks for the review! Please let me know if you spot anything else.

@tishmen
Copy link
Author

tishmen commented Oct 15, 2025

All remarks resolved and CI is green. @Kev-Roche @astirpe @mymage mind giving this a second review?

Copy link
Member

@mymage mymage left a comment

Choose a reason for hiding this comment

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

RunBoat functional OK

Copy link
Contributor

@CasVissers-360ERP CasVissers-360ERP left a comment

Choose a reason for hiding this comment

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

Functional review

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Thanks! Just some small nitpicks. And please squash the follow-up commit.

Copy link

@RyanTranAVB RyanTranAVB left a comment

Choose a reason for hiding this comment

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

Code LGTM

@tishmen tishmen force-pushed the 19.0-mig-impersonate_login branch from 8e2444b to 9aca0dd Compare December 21, 2025 21:48
@tishmen
Copy link
Author

tishmen commented Dec 21, 2025

Sorry for the delay — I’ve implemented the changes you requested. Thanks for the review!

@StefanRijnhart
Copy link
Member

@tishmen Thank you, but please squash the two follow-up commits into the migration commit.

@tishmen tishmen force-pushed the 19.0-mig-impersonate_login branch from 9aca0dd to b6b92f9 Compare December 28, 2025 11:03
@tishmen
Copy link
Author

tishmen commented Dec 28, 2025

@StefanRijnhart I squashed the two follow-up commits into the migration commit and force-pushed the updated branch. Thanks!

Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Thanks!

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 19.0-ocabot-merge-pr-849-by-StefanRijnhart-bump-nobump, awaiting test results.

@StefanRijnhart
Copy link
Member

/ocabot migration impersonate_login

@OCA-git-bot OCA-git-bot added this to the 19.0 milestone Dec 28, 2025
@OCA-git-bot OCA-git-bot mentioned this pull request Dec 28, 2025
19 tasks
@OCA-git-bot OCA-git-bot merged commit 118db7d into OCA:19.0 Dec 28, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants