Tryton Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(162)

Issue 68381002: tryton-env: Ensure data action values are set on context when executing actions

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 1 week ago by pokoli
Modified:
1 month, 1 week ago
Reviewers:
ced, reviewbot
Visibility:
Public.

Description

Patch Set 1 #

Total comments: 8

Patch Set 2 : Do not update ctx with context #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -4 lines) Patch
M sao/src/action.js View 1 1 chunk +5 lines, -3 lines 0 comments Download
M tryton/tryton/action/main.py View 1 1 chunk +5 lines, -1 line 0 comments Download

Messages

Total messages: 8
pokoli
1 month, 1 week ago (2018-12-10 19:44:06 UTC) #1
reviewbot
flake8 OK URL: https://codereview.tryton.org/68381002
1 month, 1 week ago (2018-12-10 20:16:06 UTC) #2
ced
https://codereview.tryton.org/68381002/diff/1/tryton/tryton/action/main.py File tryton/tryton/action/main.py (right): https://codereview.tryton.org/68381002/diff/1/tryton/tryton/action/main.py#newcode117 tryton/tryton/action/main.py:117: 'active_ids': data.get('ids', []), I do not understand how such ...
1 month, 1 week ago (2018-12-10 21:23:11 UTC) #3
pokoli
Do not update ctx with context
1 month, 1 week ago (2018-12-11 11:30:58 UTC) #4
pokoli
https://codereview.tryton.org/68381002/diff/1/tryton/tryton/action/main.py File tryton/tryton/action/main.py (right): https://codereview.tryton.org/68381002/diff/1/tryton/tryton/action/main.py#newcode117 tryton/tryton/action/main.py:117: 'active_ids': data.get('ids', []), On 2018/12/10 21:23:11, ced wrote: > ...
1 month, 1 week ago (2018-12-11 11:31:02 UTC) #5
ced
https://codereview.tryton.org/68381002/diff/1/tryton/tryton/action/main.py File tryton/tryton/action/main.py (right): https://codereview.tryton.org/68381002/diff/1/tryton/tryton/action/main.py#newcode117 tryton/tryton/action/main.py:117: 'active_ids': data.get('ids', []), On 2018/12/11 11:31:01, pokoli wrote: > ...
1 month, 1 week ago (2018-12-11 11:45:44 UTC) #6
reviewbot
flake8 OK URL: https://codereview.tryton.org/68381002
1 month, 1 week ago (2018-12-11 11:47:33 UTC) #7
pokoli
1 month, 1 week ago (2018-12-11 12:18:47 UTC) #8
https://codereview.tryton.org/68381002/diff/1/tryton/tryton/action/main.py
File tryton/tryton/action/main.py (right):

https://codereview.tryton.org/68381002/diff/1/tryton/tryton/action/main.py#ne...
tryton/tryton/action/main.py:117: 'active_ids': data.get('ids', []),
On 2018/12/11 11:45:44, ced wrote:
> On 2018/12/11 11:31:01, pokoli wrote:
> > On 2018/12/10 21:23:11, ced wrote:
> > > I do not understand how such keys could be in the context.
> > 
> > Do not know exactly why but for sure this is the cause of the issue. The
> problem
> > is that active_id from previous action is used (as it's present on the
> context)
> > instead of using the current data ids.
> > 
> > Is it not expected to have active_* on the context? 
> 
> No.

Ok, I will investigate further but I suspect it is something related to using
two nested context models.

https://codereview.tryton.org/68381002/diff/1/tryton/tryton/action/main.py#ne...
tryton/tryton/action/main.py:121: ctx.update(context)
On 2018/12/11 11:45:44, ced wrote:
> On 2018/12/11 11:31:01, pokoli wrote:
> > On 2018/12/10 21:23:11, ced wrote:
> > > It does not seem to be related to the issue.
> > 
> > It is because otherwise the active_ids are replaced with the older ones from
> the
> > context and then they are replaced with the older ones. 
> > 
> > After a second thought I think it does not makes sense to update ctx with
> > context (action_ctx is a copy of context), so I removed directly the update.

> > 
> > Do you think it deserves a separate issue? 
> 
> For sure it is a change of behavior.

Done in https://bugs.tryton.org/issue7917
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 0147766