New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Students are added to the wrong team #87

Closed
Seelge opened this Issue Nov 4, 2012 · 10 comments

Comments

Projects
None yet
3 participants
@Seelge

Seelge commented Nov 4, 2012

Existing teams ms1, ms2, ... , ms9, ms10, ms0.
Adding a user to team ms10 adds him to team ms1.
Adding a user to team ms0 adds him to team both ms1 and ms2.

@Seelge

This comment has been minimized.

Show comment
Hide comment
@Seelge

Seelge Nov 4, 2012

Looking at the ids, there is a pattern when adding users to teams
ms10 (id 10) -> add user to team id 1 and 0 (0 does not exist, no error message)
ms11 (id 11) -> add user to team id 1 and 1 (error because user is already in the team)
ms0 (id 12) -> add user to team id 1 and 2

Seelge commented Nov 4, 2012

Looking at the ids, there is a pattern when adding users to teams
ms10 (id 10) -> add user to team id 1 and 0 (0 does not exist, no error message)
ms11 (id 11) -> add user to team id 1 and 1 (error because user is already in the team)
ms0 (id 12) -> add user to team id 1 and 2

@moschlar

This comment has been minimized.

Show comment
Hide comment
@moschlar

moschlar Nov 4, 2012

Owner

Totally reproducible...

And I think I remember that someone wanted to point out a similar misbehaviour from Sprox and I was all like "that can't be true"... And now here we are.
Maybe @amol- can have a look at the issue with regards to the behaviour of Sprox.

User <-> Team is a Many-To-Many-Relation, maybe that's some reason? ...

Owner

moschlar commented Nov 4, 2012

Totally reproducible...

And I think I remember that someone wanted to point out a similar misbehaviour from Sprox and I was all like "that can't be true"... And now here we are.
Maybe @amol- can have a look at the issue with regards to the behaviour of Sprox.

User <-> Team is a Many-To-Many-Relation, maybe that's some reason? ...

@moschlar

This comment has been minimized.

Show comment
Hide comment
@moschlar

moschlar Nov 4, 2012

Owner

The same issue occurs for other many-to-many relations like User <-> Lesson.

Owner

moschlar commented Nov 4, 2012

The same issue occurs for other many-to-many relations like User <-> Lesson.

@moschlar

This comment has been minimized.

Show comment
Hide comment
@moschlar

moschlar Nov 4, 2012

Owner

Sorry for the spam, I'm logging my findings here, while I dive into the code...
Sprox isn't the bad guy, tgext.crud already passes in wrongly split values.

Owner

moschlar commented Nov 4, 2012

Sorry for the spam, I'm logging my findings here, while I dive into the code...
Sprox isn't the bad guy, tgext.crud already passes in wrongly split values.

@moschlar

This comment has been minimized.

Show comment
Hide comment
@moschlar

moschlar Nov 4, 2012

Owner

As can be seen in the traceback screenshot http://i.imgur.com/vMk0X.png, the problem lies within some validation that seems to occur in tg.controllers.decoratedcontroller._call() (params vs. validate_params).

Owner

moschlar commented Nov 4, 2012

As can be seen in the traceback screenshot http://i.imgur.com/vMk0X.png, the problem lies within some validation that seems to occur in tg.controllers.decoratedcontroller._call() (params vs. validate_params).

@moschlar

This comment has been minimized.

Show comment
Hide comment
@moschlar

moschlar Nov 4, 2012

Owner

For the record: It works, when you specify more than one value in the MultipleSelectionField...
It just tries to split up the id when there is only one value...

Owner

moschlar commented Nov 4, 2012

For the record: It works, when you specify more than one value in the MultipleSelectionField...
It just tries to split up the id when there is only one value...

@moschlar

This comment has been minimized.

Show comment
Hide comment
@moschlar

moschlar Nov 4, 2012

Owner

I think I found something!

@amol-, could you please have a look at that, when you have some spare time?
https://bitbucket.org/percious/sprox/src/7b88473f1b902438c54c1c6bc4165afdd7aed853/sprox/widgets/tw2widgets/widgets.py?at=default#cl-115
vs.
https://github.com/toscawidgets/tw2.forms/blob/develop/tw2/forms/widgets.py#L333

I assume, the check whether value is actually an instance of list or tuple is missing there, which causes all the trouble, right?

Owner

moschlar commented Nov 4, 2012

I think I found something!

@amol-, could you please have a look at that, when you have some spare time?
https://bitbucket.org/percious/sprox/src/7b88473f1b902438c54c1c6bc4165afdd7aed853/sprox/widgets/tw2widgets/widgets.py?at=default#cl-115
vs.
https://github.com/toscawidgets/tw2.forms/blob/develop/tw2/forms/widgets.py#L333

I assume, the check whether value is actually an instance of list or tuple is missing there, which causes all the trouble, right?

@moschlar moschlar closed this in a79bf4f Nov 4, 2012

@moschlar

This comment has been minimized.

Show comment
Hide comment
Owner

moschlar commented Nov 4, 2012

:)

@amol-

This comment has been minimized.

Show comment
Hide comment
@amol-

amol- Nov 4, 2012

Thanks for reporting this, https://bitbucket.org/percious/sprox/changeset/e79dc6a0d3d5daf95f8e0fa8d223f402839552c8 should provide a fix for it. The successive commit also has a test unit to avoid causing the issue again.

amol- commented Nov 4, 2012

Thanks for reporting this, https://bitbucket.org/percious/sprox/changeset/e79dc6a0d3d5daf95f8e0fa8d223f402839552c8 should provide a fix for it. The successive commit also has a test unit to avoid causing the issue again.

@moschlar

This comment has been minimized.

Show comment
Hide comment
@moschlar

moschlar Nov 4, 2012

Owner

Gourgeous! Thank you!

Owner

moschlar commented Nov 4, 2012

Gourgeous! Thank you!

@ghost ghost assigned moschlar Jan 15, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment