Skip to content
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

Make no type count as a type. #36

Merged
merged 2 commits into from
Feb 18, 2016
Merged

Make no type count as a type. #36

merged 2 commits into from
Feb 18, 2016

Conversation

yajo
Copy link
Member

@yajo yajo commented Feb 17, 2016

Fix #35. When an event had no type, it was not getting in the mapping because of the way Odoo generates recordsets.

@rafaelbn @pedrobaeza

Fix #35. When an event had no type, it was not getting in the mapping because of the way Odoo generates recordsets.
_("You cannot cancel registrations from events of different "
"types at once."))
res['event_type_id'] = event_type.id
first_type = registrations[0].event_id.type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put registrations[:1]to avoid errors in case of 0 registrations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that even possible?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, but making your code bullet-proof is always better

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I know, but I feel an exception here would be the right thing to happen. After all, what registrations are you going to cancel if there are none?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put then self.ensure_one()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but that doesn't serve for multi...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the other formula. It can silently pass to the other window without problems, and it's harmless

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still cannot find a real use case where you could possibly get to this wizard and registrations would be empty (and less where you'd wish the process not to get interrupted). But nevermind, let me change it.

@yajo
Copy link
Member Author

yajo commented Feb 18, 2016

Please tag as "needs review".

@dreispt dreispt added this to the 8.0 milestone Feb 18, 2016
@pedrobaeza
Copy link
Member

👍

@rafaelbn
Copy link
Member

Thanks! 👍

@JavierIniesta
Copy link

👍

pedrobaeza added a commit that referenced this pull request Feb 18, 2016
Make no type count as a type.
@pedrobaeza pedrobaeza merged commit 3cf93ba into OCA:8.0 Feb 18, 2016
@yajo yajo deleted the fix-35 branch April 4, 2016 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants