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

Improve search for another partner with same BSN #43

Merged
merged 2 commits into from
Apr 2, 2016
Merged

Improve search for another partner with same BSN #43

merged 2 commits into from
Apr 2, 2016

Conversation

astirpe
Copy link
Member

@astirpe astirpe commented Mar 13, 2016

Fixes #41

# refine search in case of multicompany setting
if partner.company_id:
args += [('company_id', '=', partner.company_id.id)]
partners = self.search(args, limit=1)
other_partner = self.search(args, limit=1)
# is another partner exists, display a warning
Copy link
Member

Choose a reason for hiding this comment

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

Typo, 'is' should be 'if' :)

@pankk
Copy link
Member

pankk commented Mar 14, 2016

👍 (code review only)

@hbrunn
Copy link
Member

hbrunn commented Mar 21, 2016

can't you search for the partner's id in the case it's available?

@hbrunn hbrunn added this to the 8.0 milestone Mar 21, 2016
@astirpe
Copy link
Member Author

astirpe commented Mar 21, 2016

@hbrunn I've chosen this solution for 2 reasons, one is technical and the other is functional.

Technical: searching in an onchange method, I cannot read my current record id, because self.id is always NewId. This occurs also in case the record was already stored in the database.

Functional: let's say, for any reason, you have some duplicated partners (individuals with the same name) and you want to keep them as they are. In this case, there is no reason to raise a warning, saying that another person has the same BSN. In facts, the same duplicated partners should get the same BSN.

@hbrunn
Copy link
Member

hbrunn commented Mar 21, 2016

@astirpe thanks, makes sense. 👍

@StefanRijnhart
Copy link
Member

👍

@hbrunn hbrunn merged commit 5ca9b6f into OCA:8.0 Apr 2, 2016
@astirpe astirpe deleted the 80_fix_bsn_unique branch April 2, 2016 13:31
@astirpe astirpe mentioned this pull request Oct 2, 2018
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.

None yet

4 participants