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

[11.0] [BUG] Fix date range type unlink. #9

Merged
merged 2 commits into from
Mar 26, 2018

Conversation

feketemihai
Copy link
Member

No description provided.

@@ -26,3 +26,10 @@ def _default_company(self):
_sql_constraints = [
('date_range_type_uniq', 'unique (name,company_id)',
'A date range type must be unique per company !')]

@api.multi
def unlink(self):
Copy link
Member

Choose a reason for hiding this comment

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

Instead of this, use ondelete='restrict' in the many2one in date.range.

Copy link
Member Author

Choose a reason for hiding this comment

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

@pedrobaeza I know, but i wanted to track the exact error, and to benefit from it to super method in OCA/account-financial-tools#636, because there the super was not covered in tests, red build, and i only had an postgres integrity error, and wanted to track it as odoo error..

Copy link
Member

Choose a reason for hiding this comment

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

You can test it the same. Just use:

with self.assertRaises(Exception):
    with self.env.cr.savepoint():
        self.date_range_type.unlink()

Copy link
Member Author

Choose a reason for hiding this comment

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

@pedrobaeza Updated, but i think is fine if i leave also the unlink method, to have a nice error message for the user.

Copy link
Member

Choose a reason for hiding this comment

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

But this is code to maintain. Generic error is clear enough IMO.

@feketemihai feketemihai force-pushed the 11.0-fix-unlink_date_tange_type branch 6 times, most recently from e3c356f to 449a5fb Compare March 19, 2018 12:01
@pedrobaeza
Copy link
Member

It would be convenient to split in 2 commits: one for the unlink problem, and another for pylint warnings, coding removal, etc.

Copy link
Member

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

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

Splitting commits would is nice to have, but otherwise 👍

@feketemihai
Copy link
Member Author

I will do it later. Thanks for review.

@feketemihai feketemihai force-pushed the 11.0-fix-unlink_date_tange_type branch from 449a5fb to e4c6beb Compare March 24, 2018 15:41
@MiquelRForgeFlow
Copy link
Contributor

🍏

@pedrobaeza pedrobaeza merged commit 654d383 into OCA:11.0 Mar 26, 2018
@pedrobaeza
Copy link
Member

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants