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

[Holger Brunn (Therp)]

[lp:~hbrunn/ocb-addons/6.1-knowledge_attachment_create_fix_duplicate_check]

6850. By Holger Brunn (Therp) on 2016-02-08

[FIX] don't return True as id

6851. By Holger Brunn (Therp) on 2016-02-08

[FIX] syntax

6852. By Holger Brunn (Therp) on 2016-02-08

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

[Ronald Portier (Therp) (rportier1962)]

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

[Holger Brunn (Therp) (hbrunn)]

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.

[Ronald Portier (Therp) (rportier1962)]

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) on 2016-02-08

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

6851. By Holger Brunn (Therp) on 2016-02-08

[FIX] syntax

6850. By Holger Brunn (Therp) on 2016-02-08

[FIX] don't return True as id

6849. By Holger Brunn (Therp) on 2016-02-08

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

=== modified file 'document/document.py'
--- document/document.py	2013-09-10 15:12:35 +0000
+++ document/document.py	2016-02-08 16:40:45 +0000
@@ -293,20 +293,21 @@
         else:
             if vals.get('file_size'):
                 del vals['file_size']
-        result = self._check_duplication(cr, uid, vals)
-        if not result:
+        result = False
+        if not self._check_duplication(cr, uid, vals) and vals.get('res_id')\
+                and vals.get('res_model') and vals.get('datas_fname'):
             domain = [
                 ('res_id', '=', vals['res_id']),
                 ('res_model', '=', vals['res_model']),
                 ('datas_fname', '=', vals['datas_fname']),
             ]
             attach_ids = self.search(cr, uid, domain, context=context)
-            super(document_file, self).write(cr, uid, attach_ids, 
-                                             {'datas' : vals['datas']},
-                                             context=context)
-            result = attach_ids[0]
-        else:
-            #raise osv.except_osv(_('ValidateError'), _('File name must be unique!'))
+            if attach_ids:
+                super(document_file, self).write(cr, uid, attach_ids,
+                                                 {'datas' : vals['datas']},
+                                                 context=context)
+                result = attach_ids[0]
+        if not result:
             result = super(document_file, self).create(cr, uid, vals, context)
         return result