8000 Manage domain on id in single value list as unique [PREVIEW] by nicoe · Pull Request #215 · coopengo/trytond · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Manage domain on id in single value list as unique [PREVIEW] #215

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nicoe
Copy link
Contributor
@nicoe nicoe commented Jan 17, 2023

Do not validate domain on empty fields unless they're required [PREVIEW]
(upstream: 11953)

Do not validate domain on empty fields unless they're required [PREVIEW]
(upstream: 11953)
@nicoe nicoe requested a review from JCavallo January 17, 2023 14:28
relations = set(records)
relations = {
r for r in records
if getattr(r, field.name) is not None}
Copy link
Contributor

Choose a reason for hiding this comment

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

Pas très fan de ça, c'est quand même un changement de comportement (il n'est pas choquant qu'un domaine > 0 implique le fait que la valeur soit non nulle...

Et le getattr à ce moment, ça va encore abimer les perfs, non ?

Copy link
Contributor Author
@nicoe nicoe Jan 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to ot 8000 hers. Learn more.

L'idée c'est que si tu veux que ton champs soit non NULL alors tu le mets requis. Mais si tu lui permets d'être NULL (et qu'il l'est) alors sa valeur étant indéterminée on ne peut pas vraiment valider le domaine.

Au niveau des perfs, ça aura fatalement un impact mais comme pour les champs relations pour lesquels le même pattern est utilisé. Ça devrait être minime.

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.

2 participants
0