Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[18.0][MIG] account_reconcile_model_oca: Migration to 18.0 #738

Merged

Conversation

xaviedoanhduy
Copy link
Contributor

@xaviedoanhduy xaviedoanhduy commented Nov 7, 2024

change in v18

depend on

Copy link
Member

@mmequignon mmequignon left a comment

Choose a reason for hiding this comment

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

I'm not sure this is the right moment for cosmetic improvements.
LG though

Comment on lines +68 to +77
SQL(
rf"""
{unaccent("%s")} ~* ('^' || (
SELECT STRING_AGG(CONCAT('(?=.*\m', chunk[1], '\M)'), '')
FROM regexp_matches({unaccent('partner.name')}, '\w{{3,}}', 'g')
AS chunk
))
""",
text_value,
)
Copy link
Member

Choose a reason for hiding this comment

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

Why not adding the wrapping parenthesis here, in order to avoid the ugly extra parenthesis in the query parts every where else:

  • query = "[...] ("
  • final = ") [...]"
  • parts = ") OR (".join(subqueries)

which IMO is more complicated to read/understand.

@TDu
Copy link
Member

TDu commented Nov 7, 2024

Are we not missing some commits from 17.0 https://github.com/OCA/account-reconcile/commits/17.0/account_reconcile_oca ?

@xaviedoanhduy
Copy link
Contributor Author

Are we not missing some commits from 17.0 https://github.com/OCA/account-reconcile/commits/17.0/account_reconcile_oca ?

hi @TDu, Maybe you a little confused about between account_reconcile_model_oca and account_reconcile_oca?

@xaviedoanhduy xaviedoanhduy force-pushed the 18.0-mig-account_reconcile_model_oca branch 2 times, most recently from ab10b31 to 0d217b9 Compare November 12, 2024 04:13
@victoralmau
Copy link
Member

Please, cherry-pick #790 to commit history

@xaviedoanhduy xaviedoanhduy force-pushed the 18.0-mig-account_reconcile_model_oca branch from 0d217b9 to 7d3576c Compare February 4, 2025 08:56
@victoralmau
Copy link
Member

Please, cherry-pick #804 to commit history.

@xaviedoanhduy
Copy link
Contributor Author

Please, cherry-pick #804 to commit history.

Hi @victoralmau, I'm a bit confused about your PR affecting account_reconcile_oca and how it relates to this module (account_reconcile_model_oca)?

@victoralmau
Copy link
Member

Please, cherry-pick #804 to commit history.

Hi @victoralmau, I'm a bit confused about your PR affecting account_reconcile_oca and how it relates to this module (account_reconcile_model_oca)?

Oops sorry, I got the module mixed up, you're right, there's nothing to add.

Copy link
Member

@JordiBForgeFlow JordiBForgeFlow 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). 🤖

@JordiBForgeFlow
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-738-by-JordiBForgeFlow-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 801bdf0 into OCA:18.0 Feb 25, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza
Copy link
Member

/ocabot migration account_reconcile_model_oca

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Feb 25, 2025
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.