Merge lp://staging/~hbrunn/ocb-addons/6.1-knowledge_attachment_create_fix_duplicate_check into lp://staging/ocb-addons/6.1

Proposed by Holger Brunn (Therp)
Status: Needs review
Proposed branch: lp://staging/~hbrunn/ocb-addons/6.1-knowledge_attachment_create_fix_duplicate_check
Merge into: lp://staging/ocb-addons/6.1
Diff against target: 32 lines (+9/-8)
1 file modified
document/document.py (+9/-8)
To merge this branch: bzr merge lp://staging/~hbrunn/ocb-addons/6.1-knowledge_attachment_create_fix_duplicate_check
Reviewer Review Type Date Requested Status
Ronald Portier (Therp) Approve
Review via email: mp+285359@code.staging.launchpad.net
To post a comment you must log in.
6850. By Holger Brunn (Therp)

[FIX] don't return True as id

6851. By Holger Brunn (Therp)

[FIX] syntax

6852. By Holger Brunn (Therp)

[FIX] fix what this branch is actually intended to fix

Revision history for this message
Ronald Portier (Therp) (rportier1962) wrote :

In the if with the check_duplication, maybe first test the values, because the method call to check_duplication makes no sense if they are not filled.

Do I understand correctly that a create is turned into a write if records are found that are not duplicate according to the check, but that share the same resource and datas_fname?

review: Needs Fixing
Revision history for this message
Holger Brunn (Therp) (hbrunn) wrote :

check_duplication takes care for missing values itself, so I don't think we need a new branch for that.

And yes, that's what happens, which is quite broken in my opinion. But well, with this patch, it's less broken.

Revision history for this message
Ronald Portier (Therp) (rportier1962) wrote :

I was just thinking of saving a method call, I have seen that check_duplication does not break on missing values. (But is will do a nonsensical search).

Not blocking for me.

review: Approve

Unmerged revisions

6852. By Holger Brunn (Therp)

[FIX] fix what this branch is actually intended to fix

6851. By Holger Brunn (Therp)

[FIX] syntax

6850. By Holger Brunn (Therp)

[FIX] don't return True as id

6849. By Holger Brunn (Therp)

[FIX] don't choke on missing res_id, fix logic

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches