-
-
Notifications
You must be signed in to change notification settings - Fork 379
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
[9.0] account mass reconcile (migration of account_easy_reconcile and account_advanced_reconcile) #136
[9.0] account mass reconcile (migration of account_easy_reconcile and account_advanced_reconcile) #136
Conversation
1d98bcc
to
719f8dd
Compare
1a236b7
to
97dbfd0
Compare
I finished fixing the tests and a view not displaying correctly. |
get_module_resource('account', 'test', | ||
'account_minimal_test.xml'), | ||
{}, 'init', False, 'test') | ||
self.rec_history_obj = self.registry('mass.reconcile.history') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use self.env
thus remove the self.cr
and self.uid
efbbcd8
to
6b0c30e
Compare
@@ -46,7 +45,7 @@ Bug Tracker | |||
Bugs are tracked on `GitHub Issues <https://github.com/OCA/bank-statement-reconcile/issues>`_. | |||
In case of trouble, please check there if your issue has already been reported. | |||
If you spotted it first, help us smashing it by providing a detailed and welcomed feedback | |||
`here <https://github.com/OCA/bank-statement-reconcile/issues/new?body=module:%20account_easy_reconcile%0Aversion:%208.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_. | |||
`here <https://github.com/OCA/bank-statement-reconcile/issues/new?body=module:%20account_mass_reconcile%0Aversion:%209.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the good news, you can simplify that:
https://github.com/OCA/maintainer-tools/blame/master/template/module/README.rst#L51
Almost good, some security model access are not on the right group. Otherwise, a bit of nitpicking. |
|
||
@api.one | ||
@api.multi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that api.multi
is implicit with api.depends
👍 Thanks for the fix and the work done here. For me it's ready to be merged. My lasts comments are optional. |
👍 |
Hi, Thanks for the work ! I've made functional tests and it work for all methods of the module. LGTM 👍 |
Use svg badge for coveralls
PR to migrate to 9.0 both
account_easy_reconcile
andaccount_advanced_reconcile
.A few points:
account_mass_reconcile
, since both are more often than not used jointly.