Skip to content

[18.0][UPD] Misc maintenance - mainly stop relying on core patches - #178

Open
thomaspaulb wants to merge 7 commits into
OCA:18.0from
thomaspaulb:18.0-delete-merged-deps
Open

thomaspaulb wants to merge 7 commits into
OCA:18.0from
thomaspaulb:18.0-delete-merged-deps

Conversation

@thomaspaulb

@thomaspaulb thomaspaulb commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

@thomaspaulb
thomaspaulb force-pushed the 18.0-delete-merged-deps branch from 6336a3f to 9998800 Compare November 1, 2026 16:48
@OCA-git-bot OCA-git-bot added mod:oca_membership Module oca_membership series:18.0 mod:oca_membership_groups Module oca_membership_groups labels Nov 1, 2026
@oca-cla-bot

oca-cla-bot Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@OCA-git-bot OCA-git-bot added the mod:oca_all Module oca_all label Nov 1, 2026
@thomaspaulb
thomaspaulb force-pushed the 18.0-delete-merged-deps branch from 0bfb2a8 to e85b8d5 Compare November 1, 2026 20:13
invoice_line.price_unit,
invoice_line.id,
qty=invoice_line.quantity,
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@bguillot Does this address the case that you were trying to fix in your PR? You hooked both create and write; I do it here now only on write, but maybe the pre-create already addresses the create case? This is on test.odoo.community.org now so maybe the case that went wrong can be tested there? How?

@arnaudlayec arnaudlayec Nov 7, 2026 •

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.

Hello Tom,
Shortly said: the membership_variable_period was not fully working like it should have when Therp/OCB patch (PR2) was installed. Benoit's PR was a bridge/fix between the 2 to make membership_variable_period work with the Therp/OCB patch. If we leave the branch Therp/OCB to use OCA/OCB, both Benoit's PR or this file account_move_line.py are not needed.

Longly said:
The Therp/OCB patch is forcing every already existing membership lines to be re-written by _prepare_membership_line_data when updating account move lines here. This erase the behavior of membership_variable_period here.

The Therp/OCB patch was however helpful to update the membership's product when updated on the account move line. But this was in former OCA process. With the work done this year, there is now 1 single membership product in OCA instance (all levels, all years).

However I think Therp's PR was legitimate and proposed this one inspired from it, in v19 : OCA/vertical-association#241. It's in v19 only because it is much more easy than in v18, because the membership module was refactored in v19 in OCA (and again we do not need it for the OCA in v18).

Comment thread pyproject.toml
# Include odoo patch for providing the right unsubscribe link in mail group mails
odoo = { git = "https://github.com/Therp/OCB.git", rev = "refs/pull/2/head" }
odoo-addon-apps_product_creator = { git = "https://github.com/Therp/apps-store",branch = "18.0-mig-apps_product_creator",subdirectory = "apps_product_creator"}
odoo = { url = "https://nightly.odoo.com/18.0/nightly/src/odoo_18.0.20261101.tar.gz"}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@sbidoul This seemed neat, but are you aware of any downsides?

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.

I think OCA/OCB is better, As far as I know OCA/OCB#1258 is not present in Odoo which may mixup addons directories in site-packages once installed.

If you really prefer install from a tar file you may use https://github.com/OCA/OCB/archive/.tar.gz (as we do in pytest-odoo CI). make sure to use a commit as revision to freeze the version and get a tarball with the same footprint. does this make a lot of difference with current git source ?

uv seems to generate the sha256 of the tarball which seems good to prevent downloading a wrong file while rebuild the instance

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I generally prefer git references as they allow for a more precise pin. And we can do a gitaggregate to integrate patches.

If we need unmerged Odoo or OCB patches, one possibility could be a gitaggregate that generates a branch in the OCB repo. One downside of that is that maintainers need write access to the repo to do a gitaggregate refresh.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you know if with git, the syntax supports doing a shallow clone, or does it always come with the whole history?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is no syntax for that, I think, as it should remain an implementation detail. uv has it's own git cache so when it has done the first clone it is quite efficient, but that does not help with CI. It has ongoing work to improve in that area: astral-sh/uv#8190.

Comment thread .pre-commit-config.yaml
hooks:
# Update the uv lockfile
- id: uv-lock
args: ["--check"]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@petrus-v So this prevents downloading the whole thing

@thomaspaulb thomaspaulb changed the title [UPD] PRs that got merged; gitpython fix [UPD] Misc maintenance Nov 1, 2026
@thomaspaulb thomaspaulb changed the title [UPD] Misc maintenance [18.0][UPD] Misc maintenance - mainly stop relying on core patches Nov 1, 2026
Comment thread pyproject.toml
# Include odoo patch for providing the right unsubscribe link in mail group mails
odoo = { git = "https://github.com/Therp/OCB.git", rev = "refs/pull/2/head" }
odoo-addon-apps_product_creator = { git = "https://github.com/Therp/apps-store",branch = "18.0-mig-apps_product_creator",subdirectory = "apps_product_creator"}
odoo = { url = "https://nightly.odoo.com/18.0/nightly/src/odoo_18.0.20261101.tar.gz"}

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.

I think OCA/OCB is better, As far as I know OCA/OCB#1258 is not present in Odoo which may mixup addons directories in site-packages once installed.

If you really prefer install from a tar file you may use https://github.com/OCA/OCB/archive/.tar.gz (as we do in pytest-odoo CI). make sure to use a commit as revision to freeze the version and get a tarball with the same footprint. does this make a lot of difference with current git source ?

uv seems to generate the sha256 of the tarball which seems good to prevent downloading a wrong file while rebuild the instance

Comment thread oca_all/__manifest__.py
"account_statement_import_file",
"account_statement_import_file_reconcile_oca",
"account_statement_import_online_stripe",
"account_statement_import_online_wise",

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.

depending on how you plan to install this module, you may add the package dependency in pyproject tool.hatch-odoo.dependencies in order to get module present to uninstall it properly before remove the code in the release after

</t>
</xpath>
</template>
<!-- Port of OCB fix "[FIX] mail_group: 405 Method not allowed in

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.

why not using OCB ?

@arnaudlayec arnaudlayec Nov 6, 2026 •

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.

Same though
I prepared a PR on the OCB : OCA/OCB#1361 . I didn't find any previous PR on OCA/OCB so this is the cherry-pick of @Kiplangatdan 's PR on odoo/odoo

Comment thread uv.lock
[[package]]
name = "certifi"
version = "2026.7.22"
version = "2026.10.18"

@petrus-v petrus-v Nov 2, 2026 •

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.

certifi 2026.10.23 has been released in the meantime and https://discuss.python.org/t/information-for-python-users-regarding-emudhra-emsign-ca-certificate-security-incident/109576 seems to at least mentionned 2026.10.19 could you update it once again ?

@arnaudlayec

arnaudlayec commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Hi Tom, just a big thanks for all of this, we needed it!
I reviewed what I can

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

Labels

mod:oca_all Module oca_all mod:oca_membership_groups Module oca_membership_groups mod:oca_membership Module oca_membership series:18.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants