Merge lp:~camptocamp/oerpscenario/trunk-set-context-lep into lp:oerpscenario

Proposed by Leonardo Pistone
Status: Merged
Merged at revision: 334
Proposed branch: lp:~camptocamp/oerpscenario/trunk-set-context-lep
Merge into: lp:oerpscenario
Diff against target: 73 lines (+18/-9)
2 files modified
features/environment.py (+1/-0)
features/steps/dsl.py (+17/-9)
To merge this branch: bzr merge lp:~camptocamp/oerpscenario/trunk-set-context-lep
Reviewer Review Type Date Requested Status
Nicolas Bessi - Camptocamp (community) Approve
Alexandre Fayolle - camptocamp code reiview, test Approve
Review via email: mp+223207@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

Hello,

little fix in inline comments.

The only point that tickle me is that I'm not sure of the lifespan of ctx.
It may be dangerous if we do not reset it.

I would add a hook at Scenario initialization to reset it to {}.

review: Needs Fixing
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

Good start, but there are other places where you want to add the context:

* all calls to browse / search / create / write / read in dsl.py

* all the same calls in the other python files of oerpscenario

review: Needs Fixing (code review, no test)
Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

maybe we want a helper function too:

    def get_oe_context(ctx):
        return getattr(ctx, 'oe_context', None)

Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

For the browse / search / create / write / read in dsl.py I agreed.
Then for the rest we can agree on a convention and update the doc.

Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

Maybe a better approach would be to dynamically alter ctx.client._context
And reset it at the end of scenario.

My two cents.

Nicolas

Revision history for this message
Leonardo Pistone (lepistone) wrote :

I addressed Nicolas' remarks.

As for the Alexandre's comments, they are not done yet, because a cleaner solution could be to do that in erppeek and not in oerpscenario. See

https://github.com/tinyerp/erppeek/issues/40

So for now I could leave this MP as is, since it does what I need now (pass a context to create()) and then maybe adapt it when the new erppeek feature lands.

Thanks!

332. By Alexandre Fayolle - camptocamp

[MRG] new step to count number of results

334. By Leonardo Pistone

[imp] reset oe_context at scenario init

335. By Leonardo Pistone

[imp] use ast.literal_eval

Revision history for this message
Leonardo Pistone (lepistone) wrote :

rebased upstream

Revision history for this message
Alexandre Fayolle - camptocamp (alexandre-fayolle-c2c) wrote :

@lpisone : ok point taken.

review: Approve (code reiview, test)
Revision history for this message
Nicolas Bessi - Camptocamp (nbessi-c2c-deactivatedaccount) wrote :

LGTM can be improved with released 1.6 of erppeek

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'features/environment.py'
--- features/environment.py 2013-09-03 10:44:42 +0000
+++ features/environment.py 2014-06-17 08:46:43 +0000
@@ -39,6 +39,7 @@
3939
40# Work around https://github.com/behave/behave/issues/14540# Work around https://github.com/behave/behave/issues/145
41def before_scenario(ctx, scenario):41def before_scenario(ctx, scenario):
42 ctx.oe_context = {}
42 if not hasattr(ctx, 'data'):43 if not hasattr(ctx, 'data'):
43 ctx.data = {}44 ctx.data = {}
4445
4546
=== modified file 'features/steps/dsl.py'
--- features/steps/dsl.py 2014-06-17 08:14:33 +0000
+++ features/steps/dsl.py 2014-06-17 08:46:43 +0000
@@ -180,18 +180,25 @@
180 ctx.found_item.write(table_values)180 ctx.found_item.write(table_values)
181181
182182
183@step(u'I set the context to "{oe_context_string}"')
184def impl(ctx, oe_context_string):
185 ctx.oe_context = literal_eval(oe_context_string)
186
187
183def create_new_obj(ctx, model_name, values):188def create_new_obj(ctx, model_name, values):
184 values = values.copy()189 values = values.copy()
185 xmlid = values.pop('xmlid', None)190 xmlid = values.pop('xmlid', None)
186 record = model(model_name).create(values)191 oe_context = getattr(ctx, 'oe_context', None)
192 record = model(model_name).create(values, context=oe_context)
187 if xmlid is not None:193 if xmlid is not None:
188 ModelData = model('ir.model.data')194 ModelData = model('ir.model.data')
189 module, xmlid = xmlid.split('.', 1)195 module, xmlid = xmlid.split('.', 1)
190 _model_data = ModelData.create({'name': xmlid,196 _model_data = ModelData.create({
191 'model': model_name,197 'name': xmlid,
192 'res_id': record.id,198 'model': model_name,
193 'module': module,199 'res_id': record.id,
194 })200 'module': module,
201 }, context=oe_context)
195 return record202 return record
196203
197204
@@ -201,6 +208,7 @@
201 assert active_text in ('', 'inactive', 'active', 'possibly inactive')208 assert active_text in ('', 'inactive', 'active', 'possibly inactive')
202 Model = model(model_name)209 Model = model(model_name)
203 ctx.search_model_name = model_name210 ctx.search_model_name = model_name
211 oe_context = getattr(ctx, 'oe_context', None)
204 values = parse_domain(domain)212 values = parse_domain(domain)
205 active = True213 active = True
206 if active_text == 'inactive':214 if active_text == 'inactive':
@@ -209,14 +217,14 @@
209 active = None217 active = None
210 domain = build_search_domain(ctx, model_name, values, active=active)218 domain = build_search_domain(ctx, model_name, values, active=active)
211 if domain is not None:219 if domain is not None:
212 ids = Model.search(domain)220 ids = Model.search(domain, context=oe_context)
213 else:221 else:
214 ids = []222 ids = []
215 if len(ids) == 1:223 if len(ids) == 1:
216 ctx.found_item = Model.browse(ids[0])224 ctx.found_item = Model.browse(ids[0], context=oe_context)
217 else:225 else:
218 ctx.found_item = None226 ctx.found_item = None
219 ctx.found_items = Model.browse(ids)227 ctx.found_items = Model.browse(ids, context=oe_context)
220228
221229
222@step(u'I need a{n:optional}{active_text:optional} "{model_name}" with {domain}')230@step(u'I need a{n:optional}{active_text:optional} "{model_name}" with {domain}')

Subscribers

People subscribed via source and target branches

to all changes: