Skip to content

Conversation

@Shide
Copy link

@Shide Shide commented Nov 26, 2025

Adapted from #711 and #730.

Limit is set con code and now query_companies it's used to show properly the companies that are being queried.
Also fixed the account code that shows to False if the env.company is not the fetched company data.

Tests are mainly from @StefanRijnhart, but adapted a little bit.

Coauthored with @StefanRijnhart and @geraldo29

image

MT-11000 @moduon @rafaelbn @StefanRijnhart @geraldo29 @sbidoul @ThiagoMForgeFlow please review if you want 😄

@OCA-git-bot
Copy link
Contributor

Hi @sbidoul,
some modules you are maintaining are being modified, check this out!

@Shide Shide force-pushed the 18.0-multi_company_fix-mis_builder branch from f889173 to 995020c Compare November 26, 2025 10:36
Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

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

Code review.

Co-authored-by: Stefan Rijnhart <1033124+StefanRijnhart@users.noreply.github.com>
Co-authored-by: Geraldo Lopez <5943428+geraldo29@users.noreply.github.com>
@Shide Shide force-pushed the 18.0-multi_company_fix-mis_builder branch from 995020c to 4100527 Compare November 26, 2025 10:58
@Shide
Copy link
Author

Shide commented Nov 26, 2025

Just added translations to "and <nbr_companies_exceeded> more" and translations for other missing terms in es_ES.

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!

@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

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

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

Thanks!

The PR is fixes the issue but introduces a change, in the sense that before, there was one detail line per account and company.

I think I more or less know what to do with a first baby step towards configurable account detail expansion.

Marking as request change for now as I'm not convinced yet this is a good default behaviour.

I hope to come to this soon.

@yajo
Copy link
Member

yajo commented Dec 18, 2025

IMHO the fix should be merged and, later, add that configurability. Otherwise the module explodes in users' hands right now.

@rafaelbn
Copy link
Member

@Shide could we apply this PR in PROD? The issue is affecting production clients enviroments. Thank you!

@Shide
Copy link
Author

Shide commented Dec 18, 2025

@rafaelbn, unless @sbidoul are making another PR that makes this case better structured, I think we could.
When the final PR comes, we could remove this and adopt the standard way. This PR doesn't add new fields nor any fields that we need to revert in the future.

@sbidoul, could you ping us to know when the new PR will replace this one? Thanks

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants