11.0 mig base exception by mourad-ehm · Pull Request #1025 · OCA/server-tools

mourad-ehm · 2017-10-13T08:35:23Z

MIG module base_exception and add test.

pedrobaeza · 2017-10-13T08:38:01Z

Please squash some commits according https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests

mourad-ehm · 2017-10-17T09:01:45Z

Runbot is red but in runbot log I had a no error. Someone know why runbot is red ?
ping @pedrobaeza @jbeficent
log message:

`
2017-10-16 15:27:52,164 151 INFO openerp_test odoo.addons.base.module.module: ALLOW access to module.button_install on ['base_exception'] to user admin #1 via n/a
2017-10-16 15:27:52,278 151 INFO openerp_test odoo.modules.loading: loading 12 modules...
2017-10-16 15:27:52,346 151 INFO openerp_test odoo.modules.loading: 12 modules loaded in 0.07s, 0 queries
2017-10-16 15:27:52,349 151 INFO openerp_test odoo.modules.loading: loading 13 modules...
2017-10-16 15:27:52,572 151 WARNING openerp_test odoo.fields: Field base.exception.test.model.line.lead_id with unknown comodel_name 'base.exception.test.model'
2017-10-16 15:27:52,761 151 INFO openerp_test odoo.modules.registry: module base_exception: creating or updating database tables
2017-10-16 15:27:52,863 151 INFO openerp_test odoo.models: Storing computed values of base.exception.test.purchase.main_exception_id
2017-10-16 15:27:52,872 151 INFO openerp_test odoo.models: Storing computed values of base.exception.test.purchase.amount_total
2017-10-16 15:27:53,028 151 INFO openerp_test odoo.modules.loading: loading base_exception/security/base_exception_security.xml
2017-10-16 15:27:53,139 151 INFO openerp_test odoo.modules.loading: loading base_exception/security/ir.model.access.csv
2017-10-16 15:27:53,233 151 INFO openerp_test odoo.modules.loading: loading base_exception/wizard/base_exception_confirm_view.xml
2017-10-16 15:27:53,262 151 INFO openerp_test odoo.modules.loading: loading base_exception/views/base_exception_view.xml
2017-10-16 15:27:53,352 151 INFO openerp_test odoo.modules.loading: 13 modules loaded in 1.00s, 0 queries
2017-10-16 15:27:53,513 151 INFO openerp_test odoo.modules.loading: Modules loaded.
2017-10-16 15:27:53,515 151 INFO openerp_test odoo.models: Computing parent left and right for table ir_ui_menu...
2017-10-16 15:27:53,532 151 INFO openerp_test odoo.service.server: Initiating shutdown
2017-10-16 15:27:53,533 151 INFO openerp_test odoo.service.server: Hit CTRL-C again or send a second signal to force the shutdown.

Module test summary
�[1;32mSuccess�[0;m base_exception

+=======================================

Tests summary:
test_server.py �[1;32mSuccess�[0;m
+=======================================
`

yajo · 2017-10-17T09:18:58Z

Runbot had some problems recently, I just rebuilt it, let's see.

mourad-ehm · 2017-10-17T13:59:00Z

I rebuild runbot and had this error:
Unable to find image 'oca-server-tools:8bfb488_2' locally
docker: Error response from daemon: pull access denied for oca-server-tools, repository does not exist or may require 'docker login'.
See 'docker run --help'.

pedrobaeza · 2017-10-17T14:01:01Z

It seems to be a problem with runbot. I have notified already @gurneyalex about it.

mourad-ehm · 2017-10-24T13:55:57Z

Now runbot is build correctly but is still red, someone know why ? ping @gurneyalex @legalsylvain

pedrobaeza · 2017-10-24T17:21:00Z

See logs. You will find this warning:

2017-10-24 13:29:09,284 151 WARNING openerp_test odoo.fields: Field base.exception.test.model.line.lead_id with unknown comodel_name 'base.exception.test.model'

JordiBForgeFlow · 2017-11-22T19:07:12Z

I have proposed small fixes here: https://github.com/Eficent/server-tools/tree/11.0-mig-base_exception

But cannot make a PR, the PR does not allow me to select the Akretion repo.

mourad-ehm · 2017-11-23T08:22:53Z

Thanks @jbeficent. I integrated your commit.

JordiBForgeFlow · 2017-11-23T10:51:51Z

@mourad-ehm I still added another commit ForgeFlow@b66bddf in https://github.com/Eficent/server-tools/tree/11.0-mig-base_exception.

bealdav · 2017-11-27T10:49:05Z

Here codecov is 55% but other tests coverage in purchase_exception will cover other parts of the code of this module. Then I think it's mergeable as it. Thanks @mourad-ehm for this work

hparfr

Some remarks and questions

hparfr · 2017-11-27T10:55:23Z

+    active = fields.Boolean('Active')
+    next_state = fields.Char(
+        'Next state',
+        help="If we detect exception we set de state of object (ex purchase) "


set the state

hparfr · 2017-11-27T10:56:58Z

+    code = fields.Text(
+        'Python Code',
+        help="Python code executed to check if the exception apply or "
+             "not. The code must apply block = True to apply the "


block = True or failed = True (code.default)

hparfr · 2017-11-27T10:59:04Z

+                    rule.model].fields_get()[
+                        'state']['selection']
+                if rule.next_state\
+                        not in [s[0] for s in select_vals]:


put comprehension list out of if clause to add readability

hparfr · 2017-11-27T11:01:24Z

+    @api.multi
+    def _popup_exceptions(self):
+        action = self._get_popup_action()
+        action = action.read()[0]


merge in one line or use different variable names

hparfr · 2017-11-27T11:06:21Z

+                'self': self.pool.get(rec._name),
+                'object': rec,
+                'obj': rec,
+                'pool': self.pool,


No, this is not old API but variables which can use in Python code with safe_eval

@mourad-ehm self.pool is not meant to be used.

I see it was migrated in v10 like that but it's incorrect. Use self.env[rec._name] and 'env': self.env instead of pool.

But I think changing function to @api.multi and returning {'self': self} should be sufficent. So, you can change the function signature removing obj_name and rec parameters!

hparfr · 2017-11-27T11:07:32Z

+                if rule.next_state:
+                    if not next_state_rule:
+                        next_state_rule = rule
+                    elif next_state_rule and\


useless next_state_rule condition (because of else)

if not next_state_rule or rule.sequence < next_state_rule.sequence next_state_rule = rule

hparfr · 2017-11-27T11:11:37Z

+        current_model = self._context.get('active_model')
+        model_except_obj = self.env[current_model]
+        active_ids = self._context.get('active_ids')
+        assert len(active_ids) == 1, "Only 1 ID accepted, got %r" % active_ids


raise instead of assert

rousseldenis

Code review

rousseldenis · 2017-11-27T16:16:59Z

+                'self': self.pool.get(rec._name),
+                'object': rec,
+                'obj': rec,
+                'pool': self.pool,


@mourad-ehm self.pool is not meant to be used.

I see it was migrated in v10 like that but it's incorrect. Use self.env[rec._name] and 'env': self.env instead of pool.

But I think changing function to @api.multi and returning {'self': self} should be sufficent. So, you can change the function signature removing obj_name and rec parameters!

rousseldenis · 2017-11-27T16:42:18Z

+
+    @api.model
+    def _exception_rule_eval_context(self, obj_name, rec):
+        user = self.env['res.users'].browse(self._uid)


user = self.env.user

rousseldenis · 2017-11-27T16:47:54Z

+                'obj': rec,
+                'pool': self.pool,
+                'cr': self._cr,
+                'uid': self._uid,


self.env.user.id

I thought self._cr, self._uid were shortcuts for self.env.cr, etc.
Are they now deprecated ?
Thanks for your review

@bealdav See https://github.com/odoo/odoo/blob/ecbf7aa4714479229658d14cce28fa00376ed390/odoo/models.py#L4254

They are kept for backward compatibility but their future is not sure.

thanks a lot

rousseldenis · 2017-11-27T16:48:00Z

+                'object': rec,
+                'obj': rec,
+                'pool': self.pool,
+                'cr': self._cr,


self.env.cr

mourad-ehm · 2017-12-04T13:01:31Z

Thanks @jbeficent, I integrated your commits.

rousseldenis

Code review.
Should add more tests on roadmap to improve coverage.

mourad-ehm · 2017-12-04T13:32:32Z

OK, thanks @rousseldenis

simahawk · 2017-12-04T16:46:36Z

@mourad-ehm remember to squash ;)

mourad-ehm · 2017-12-05T07:51:49Z

Ok, thanks @simahawk. I done it.

mourad-ehm · 2017-12-05T08:22:14Z

Hi @pedrobaeza, could you merge this PR ?

pedrobaeza · 2017-12-05T08:51:53Z

Can you please squash commits by author/logic set before merging?

* Fix menu in base_exception * Fix base_exception/views/base_exception_view.xml

…lled by constraint methods 'detect_exception' can be called on an empty recordset.

mourad-ehm · 2017-12-05T09:29:05Z

Squash done.

bealdav · 2017-12-05T09:45:43Z

@pedrobaeza thanks for the merge. No project in v11 at akretion for now.

It's just to help the community. Thanks

pedrobaeza · 2017-12-05T10:14:23Z

Thank you! Much appreciated.

Merging.

[t133303] Update project branch with last fixes

mourad-ehm mentioned this pull request Oct 13, 2017

[11.0] merge "purchase_fop_shipping" and "purchase_minimum_amount" OCA/purchase-workflow#464

Closed

pedrobaeza mentioned this pull request Oct 13, 2017

Migration to version 11.0 #1008

Closed

92 tasks

mourad-ehm force-pushed the 11.0-mig-base_exception branch 3 times, most recently from 351885c to 8bfb488 Compare October 16, 2017 15:12

mourad-ehm closed this Oct 24, 2017

mourad-ehm reopened this Oct 24, 2017

pedrobaeza added this to the 11.0 milestone Oct 31, 2017

mourad-ehm mentioned this pull request Nov 22, 2017

[ADD] purchase_exception to 11.0 OCA/purchase-workflow#467

Closed

1 task

mourad-ehm force-pushed the 11.0-mig-base_exception branch from 2ca58e9 to f9ad522 Compare November 23, 2017 08:20

JordiBForgeFlow mentioned this pull request Nov 23, 2017

[11.0][mig] purchase_order_block] OCA/purchase-workflow#480

Merged

2 tasks

mourad-ehm force-pushed the 11.0-mig-base_exception branch 3 times, most recently from deb3cdd to 2cbe214 Compare November 27, 2017 08:44

bealdav approved these changes Nov 27, 2017

View reviewed changes

hparfr reviewed Nov 27, 2017

View reviewed changes

mourad-ehm force-pushed the 11.0-mig-base_exception branch from 2cbe214 to b0f591f Compare November 27, 2017 14:37

rousseldenis suggested changes Nov 27, 2017

View reviewed changes

mourad-ehm force-pushed the 11.0-mig-base_exception branch from b0f591f to 736b73e Compare November 28, 2017 11:10

mourad-ehm closed this Dec 4, 2017

mourad-ehm reopened this Dec 4, 2017

mourad-ehm force-pushed the 11.0-mig-base_exception branch from cb443e9 to 785be4d Compare December 4, 2017 11:28

rousseldenis approved these changes Dec 4, 2017

View reviewed changes

simahawk approved these changes Dec 4, 2017

View reviewed changes

moylop260 approved these changes Dec 5, 2017

View reviewed changes

mourad-ehm force-pushed the 11.0-mig-base_exception branch from 785be4d to f886227 Compare December 5, 2017 07:57

mourad-ehm and others added 8 commits December 5, 2017 10:00

[ADD] split module sale_exception into base_exception

e40153b

OCA Transbot updated translations from Transifex

54581fe

Fix menu in base_exception (OCA#803)

4e7710f

* Fix menu in base_exception * Fix base_exception/views/base_exception_view.xml

[FIX] base_exception: '_check_exception' is 'api.multi' since it's ca…

aa68c72

…lled by constraint methods 'detect_exception' can be called on an empty recordset.

OCA Transbot updated translations from Transifex

d4b7773

[MIG] base_exception: Migration to 11.0

2102134

style fixes

49fbb3b

[FIX] temp model test access rule & fix OCA remarks

4342d25

mourad-ehm force-pushed the 11.0-mig-base_exception branch from f886227 to 4342d25 Compare December 5, 2017 09:02

pedrobaeza merged commit b5409b9 into OCA:11.0 Dec 5, 2017

bealdav deleted the 11.0-mig-base_exception branch December 5, 2017 10:37

JordiBForgeFlow mentioned this pull request Dec 28, 2017

[11.0][base_exception] Gives errors when depending modules are proposed #1110

Closed

joao-p-marques mentioned this pull request Nov 4, 2020

[13.0][MIG] base_exception: Migration to 13.0 #1920

Merged

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

Merge pull request OCA#1025 from brain-tec/15.0

7286584

[t133303] Update project branch with last fixes

Uh oh!

Uh oh!

Conversation

mourad-ehm commented Oct 13, 2017

Uh oh!

pedrobaeza commented Oct 13, 2017

Uh oh!

mourad-ehm commented Oct 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yajo commented Oct 17, 2017

Uh oh!

mourad-ehm commented Oct 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pedrobaeza commented Oct 17, 2017

Uh oh!

mourad-ehm commented Oct 24, 2017

Uh oh!

pedrobaeza commented Oct 24, 2017

Uh oh!

JordiBForgeFlow commented Nov 22, 2017

Uh oh!

mourad-ehm commented Nov 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JordiBForgeFlow commented Nov 23, 2017

Uh oh!

bealdav commented Nov 27, 2017

Uh oh!

hparfr left a comment

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!

Choose a reason for hiding this comment

Uh oh!

rousseldenis Nov 27, 2017 • 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!

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!

rousseldenis left a comment

Choose a reason for hiding this comment

Uh oh!

rousseldenis Nov 27, 2017 • 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!

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!

mourad-ehm commented Dec 4, 2017

Uh oh!

rousseldenis left a comment

Choose a reason for hiding this comment

Uh oh!

mourad-ehm commented Dec 4, 2017

mourad-ehm commented Oct 17, 2017 •

edited

Loading

mourad-ehm commented Oct 17, 2017 •

edited

Loading

mourad-ehm commented Nov 23, 2017 •

edited

Loading

rousseldenis Nov 27, 2017 •

edited

Loading

rousseldenis Nov 27, 2017 •

edited

Loading

mourad-ehm commented Dec 5, 2017 •

edited

Loading