Skip to content

Conversation

@parvezqureshi
Copy link

@OCA-git-bot
Copy link
Contributor

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

@parvezqureshi parvezqureshi force-pushed the 17.0-fix-pos_partner_birthdate branch 3 times, most recently from 3249a1f to d4598fe Compare March 12, 2025 14:04
@cvinh
Copy link
Contributor

cvinh commented Mar 12, 2025

cc @vehi-invitu

@cvinh
Copy link
Contributor

cvinh commented Mar 13, 2025

thanks for the PR, we tested here in french at GMT-10 and with several format input (ddmmyyyy or dd/mm/yyyy), it doesn't seem to work as expected
Can you do it language and format date proof ? At least ddmmyyyy and dd/mm/yyyy for french and mmyyyyyy or mm/dd/yyyy for english
Please be aware of timezone because if we are at another timezone than UTC, the date might move +/-1 day

As example @etobella did it like this

var result = super._partner_search_string(partner);

and it works well with the classical search

@parvezqureshi parvezqureshi force-pushed the 17.0-fix-pos_partner_birthdate branch 3 times, most recently from 3ed02fd to a74d7c8 Compare March 13, 2025 10:55
Copy link
Contributor

@cvinh cvinh left a comment

Choose a reason for hiding this comment

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

works as expected

Copy link

@vehi-invitu vehi-invitu left a comment

Choose a reason for hiding this comment

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

I tested it and it works as expected.

@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

@etobella etobella left a comment

Choose a reason for hiding this comment

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

Small, suggestion, why don't you ask Odoo to add a hook in order to modify the domain filter? this way, you don't need to have this big change.

I made other PRs asking for hooks and they accepted it without an issue:
odoo/odoo#150914

@cvinh
Copy link
Contributor

cvinh commented Mar 20, 2025

Small, suggestion, why don't you ask Odoo to add a hook in order to modify the domain filter? this way, you don't need to have this big change.

I made other PRs asking for hooks and they accepted it without an issue: odoo/odoo#150914

odoo/odoo#202641

@cvinh
Copy link
Contributor

cvinh commented Apr 16, 2025

Maybe we can merge this one and wait for odoo/odoo#202641 to improve it
Otherwise, we do a PR to OCB and improve it now
@etobella WDYT ?

@cvinh
Copy link
Contributor

cvinh commented May 14, 2025

@legalsylvain WDYT on that one please ?

@cvinh
Copy link
Contributor

cvinh commented May 14, 2025

please @parvezqureshi can you rebase because of runboat ?

…stomers and we search a customer using the birthdate with the button search more, result not found.
@parvezqureshi parvezqureshi force-pushed the 17.0-fix-pos_partner_birthdate branch from a74d7c8 to b564757 Compare May 14, 2025 06:56
@parvezqureshi
Copy link
Author

please @parvezqureshi can you rebase because of runboat ?

Rebased

@cvinh
Copy link
Contributor

cvinh commented Jun 4, 2025

Small, suggestion, why don't you ask Odoo to add a hook in order to modify the domain filter? this way, you don't need to have this big change.
I made other PRs asking for hooks and they accepted it without an issue: odoo/odoo#150914

odoo/odoo#202641

PR has been rejected by Odoo and I proposed to do a PR to OCB... In that case, this fix will only work with OCB... or we can merge as it is... and improve it in next version
PSC wdyt ?

@cvinh
Copy link
Contributor

cvinh commented Sep 18, 2025

Odoo answered this part has been changed in further versions, so we can merge this and then improve in next versions
@legalsylvain

@ivantodorovich
Copy link

I wouldn't have a problem merging like this, but please add some comments mentioning we're fully overwriting the core method and why.

Add comments before and after the modified lines to make it easier for devs working in future migrations to identify the actual patch.

Alternatively, there's a way to do this without a complete overwrite. It's not as clean as if Odoo had a hook method, but it works just fine: you can temporarily monkey-patch the this.orm.silent.call method with a method that injects your new domain leaves. Call super in a try..finally block, reverting the monkey patch after its called.

@ivs-cetmix
Copy link
Member

Hey @parvezqureshi than you for your contribution! Could you please check the comments and resolve them?

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.

7 participants