[8.0][FIX][base_export_manager] Fix "Expected singleton" bug. by yajo · Pull Request #521 · OCA/server-tools

yajo · 2016-08-17T09:02:58Z

If you had a field that got translated in more than 1 addon, you'd possibly get
to this error:

  File "/opt/odoo/0079_ahk_openerp/oca/base_export_manager/models/ir_exports_line.py", line 105, in _compute_label
    field.name)),
  File "/opt/odoo/common/openerp/v8/openerp/fields.py", line 825, in __get__
    record.ensure_one()
  File "/opt/odoo/common/openerp/v8/openerp/models.py", line 5355, in ensure_one
    raise except_orm("ValueError", "Expected singleton: %s" % self)
except_orm: ('ValueError', 'Expected singleton: ir.translation(4899, 703976)')

With this patch, now we let Odoo return the translated string by using its
standard method to do so, so we have to care for less.

@Tecnativa

If you had a field that got translated in more than 1 addon, you'd possibly get to this error: File "/opt/odoo/0079_ahk_openerp/oca/base_export_manager/models/ir_exports_line.py", line 105, in _compute_label field.name)), File "/opt/odoo/common/openerp/v8/openerp/fields.py", line 825, in __get__ record.ensure_one() File "/opt/odoo/common/openerp/v8/openerp/models.py", line 5355, in ensure_one raise except_orm("ValueError", "Expected singleton: %s" % self) except_orm: ('ValueError', 'Expected singleton: ir.translation(4899, 703976)') With this patch, now we let Odoo return the translated string by using its standard method to do so, so we have to care for less.

yajo · 2016-08-17T09:03:34Z

Please could somebody forward-patch this to v9 when merged? Or do I open a separate PR?

This makes the whole installation to be able to roll back if something goes wrong, instead of entering an error loop.

yajo · 2016-08-17T11:47:08Z

Before 9ef4280, if you had a buggy record in database, you got an endless error loop. Now you get this beautiful message:

I considered adding a backup column of ir.exports.line.name, but it's not necessary because now installation gets rolled back if anything fails, and it does not lose data.

pedrobaeza · 2016-08-17T12:56:52Z

+    char field with stored values. We have to inverse it to avoid database
+    inconsistencies.
+    """
+    env = api.Environment(cr, SUPERUSER_ID, dict())


with api.Environment.manage(): env = api.Environment(cr, SUPERUSER_ID, {})

pedrobaeza · 2016-08-17T17:29:43Z

One thing I'm facing: can it be that the textual representation of the fields (field name) have changed? It seems that previously you have account_id / code for representing a sub field, and now it's account_id/code (without spaces).

pedrobaeza · 2016-08-17T18:27:01Z

+        result = self.env["ir.model.fields"].search(
            [("name", "=", name),
             ("model_id", "=", model.id)])
+        try:


Use if not result.exists(): instead

yajo · 2016-08-18T10:48:01Z

Just to clarify: #521 (comment) was not a bug.

Is this now ready to review?

pedrobaeza · 2016-08-18T10:48:29Z

Indeed it is

pedrobaeza · 2016-08-18T10:53:23Z

👍 the part I haven't done

pedrobaeza · 2016-08-18T10:58:04Z

@max3903 can you check this, please?

yajo · 2016-08-18T13:40:17Z

+            self.label = ("%s (%s)" % ("/".join(parts), self.name)
+                          if parts and self.name else False)
+        except KeyError:
+            pass


I'd thank a little comment about why this is necessary.

IMO it would also be a little nicer if we guarded a little closer to the key error. We could then just sub the pass for a return to accomplish the same effect

I like the idea.

I concur with @yajo: it's not obvious why the pass is needed, an explaining comment would be welcome.

yajo · 2016-08-18T13:45:22Z

I'm thinking that maybe a good improvement would be to give this addon its own menu item. Non-admins may want to manage this too, given what this addon ultimately does is make that task easier. Maybe it could be handled by a group setting. Not a requirement though.

lasley · 2016-08-19T15:04:41Z

+    @api.one
+    @api.depends("field3_id")
+    def _compute_model4_id(self):
+        """Get the related model for the third field."""


s/third/fourth

max3903 · 2016-08-30T15:38:26Z

@pedrobaeza Sorry, I missed your message. What do you want me to check?

pedrobaeza · 2016-08-30T16:23:43Z

Well, the idea is to see the improvements and give your bless.

max3903 · 2016-08-30T17:28:05Z

👍 LGTM

rafaelbn · 2016-09-12T20:50:01Z

Functionaly tested 👍

dreispt · 2016-09-16T09:08:36Z


-    @api.multi
-    @api.depends("field1_id", "field2_id", "field3_id")
+    @api.one


Isn't best to leave api.maulti here, since we have confirmation that Odoo intends to deprecated api.one ?

I prefer @api.one while it exists. And I repeat that it's still not clear that it will be deprecated.

I don't see the point for the extra work when doing the port to v9.
And when asking to reconsider deprecation, all comments I had were against keeping it. So I don't see must space for any hopes on that.

No, there's no work needed. @api.one is still in v9, v10, and counting.

Won't Lint report errors?

No, they are only warnings. The PR in MQT was for removing the warning.

pedrobaeza · 2016-10-14T01:29:55Z

@yajo, can you attend last comments (including yours)?

- Add some comments in complex parts. - Rename `onchange_name` to `_onchange_name` (guidelines). - Make `_compute_name`'s try block shorter and easier to understand.

pedrobaeza · 2016-10-18T10:52:24Z

Merging as the problem in Travis fluctuates between builds.

[FIX][base_export_manager] Fix "Expected singleton" bug. If you had a field that got translated in more than 1 addon, you'd possibly getto this error: File "/opt/odoo/0079_ahk_openerp/oca/base_export_manager/models/ir_exports_line.py", line 105, in _compute_label field.name)), File "/opt/odoo/common/openerp/v8/openerp/fields.py", line 825, in __get__ record.ensure_one() File "/opt/odoo/common/openerp/v8/openerp/models.py", line 5355, in ensure_one raise except_orm("ValueError", "Expected singleton: %s" % self) except_orm: ('ValueError', 'Expected singleton: ir.translation(4899, 703976)') With this patch, now we let Odoo return the translated string by using its standard method to do so, so we have to care for less. * Move installation outside a data file. This makes the whole installation to be able to roll back if something goes wrong, instead of entering an error loop. * Include envorionment in its manager. * Add 4th field * Move to api.multi, refactoring some stuff. - Add some comments in complex parts. - Rename `onchange_name` to `_onchange_name` (guidelines). - Make `_compute_name`'s try block shorter and easier to understand. * Allow R/W of name directly in model. * Update tests to cover new behaviors.

Syncing from upstream OCA/server-tools (12.0)

Move installation outside a data file.

9ef4280

This makes the whole installation to be able to roll back if something goes wrong, instead of entering an error loop.

pedrobaeza reviewed Aug 17, 2016
View reviewed changes

Include envorionment in its manager.

ac0a998

pedrobaeza reviewed Aug 17, 2016
View reviewed changes

pedrobaeza force-pushed the 8.0-base_export_manager-fix_singleton_error branch 4 times, most recently from 77dfc2c to 62e782f Compare August 18, 2016 09:31

[IMP] base_export_manager: Several changes

032f289

pedrobaeza force-pushed the 8.0-base_export_manager-fix_singleton_error branch 4 times, most recently from 92d4b2e to dbb0416 Compare August 18, 2016 10:09

[IMP] base_export_manager: Add 4th field

ee91e4e

pedrobaeza force-pushed the 8.0-base_export_manager-fix_singleton_error branch from dbb0416 to ee91e4e Compare August 18, 2016 10:15

yajo reviewed Aug 18, 2016
View reviewed changes

lasley reviewed Aug 19, 2016
View reviewed changes

max3903 added the needs review label Aug 30, 2016

max3903 added this to the 8.0 milestone Aug 30, 2016

dreispt reviewed Sep 16, 2016

View reviewed changes

yajo changed the title ~~[FIX][base_export_manager] Fix "Expected singleton" bug.~~ [8.0][FIX][base_export_manager] Fix "Expected singleton" bug. Oct 17, 2016

yajo added 3 commits October 17, 2016 14:38

Move to api.multi, refactoring some stuff.

65434c0

- Add some comments in complex parts. - Rename `onchange_name` to `_onchange_name` (guidelines). - Make `_compute_name`'s try block shorter and easier to understand.

Allow R/W of name directly in model.

615041c

Update tests to cover new behaviors.

afebf90

pedrobaeza approved these changes Oct 18, 2016

View reviewed changes

pedrobaeza merged commit 76f291b into OCA:8.0 Oct 18, 2016

pedrobaeza deleted the 8.0-base_export_manager-fix_singleton_error branch October 18, 2016 10:53

SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024

Merge pull request OCA#521 from OCA/12.0

1295f32

Syncing from upstream OCA/server-tools (12.0)

Uh oh!

Uh oh!

Conversation

yajo commented Aug 17, 2016

Uh oh!

yajo commented Aug 17, 2016

Uh oh!

yajo commented Aug 17, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pedrobaeza commented Aug 17, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yajo commented Aug 18, 2016

Uh oh!

pedrobaeza commented Aug 18, 2016

Uh oh!

pedrobaeza commented Aug 18, 2016

Uh oh!

pedrobaeza commented Aug 18, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yajo Aug 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dreispt Sep 16, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yajo commented Aug 18, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

max3903 commented Aug 30, 2016

Uh oh!

pedrobaeza commented Aug 30, 2016

Uh oh!

max3903 commented Aug 30, 2016

Uh oh!

rafaelbn commented Sep 12, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pedrobaeza commented Oct 14, 2016

Uh oh!

pedrobaeza commented Oct 18, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

yajo Aug 22, 2016 •

edited

Loading

dreispt Sep 16, 2016 •

edited

Loading