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

[ADD] 11.0 mig account mass reconcile #197

Merged
merged 52 commits into from
Jun 28, 2018

Conversation

mpanarin
Copy link

@mpanarin mpanarin commented Jan 4, 2018

No description provided.

@pedrobaeza pedrobaeza added this to the 11.0 milestone Jan 4, 2018
@mpanarin mpanarin force-pushed the 11.0-mig-account_mass_reconcile branch 3 times, most recently from f400410 to 7a79068 Compare January 5, 2018 11:31
Bug Tracker
===========

Bugs are tracked on `GitHub Issues <https://github.com/OCA/bank-statement-reconcile/issues>`_.
Copy link

Choose a reason for hiding this comment

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

* Vincent Renaville <vincent.renaville@camptocamp.com>
* Alexandre Fayolle <alexandre.fayolle@camptocamp.com>
* Joël Grand-Guillaume <joel.grandguillaume@camptocamp.com>
* Nicolas Bessis <nicolas.bessi@camptocamp.com>
Copy link

Choose a reason for hiding this comment

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

Bessi

@@ -0,0 +1,22 @@
# © 2012-2016 Camptocamp SA (Guewen Baconnier, Damien Crier, Matthieu Dietrich)
Copy link

Choose a reason for hiding this comment

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

(c) -> Copyright

for value, ovalue in product(values, opposite_values):
# we do not need to compare all values, if one matches
# we are done
if MassReconcileAdvanced._compare_values(key, value, ovalue):
Copy link

Choose a reason for hiding this comment

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

shouldn't we use self._* instead of calling the class straight?

'move_id')
return ["account_move_line.{}".format(col) for col in aml_cols]

def _select(self, *args, **kwargs):
Copy link

Choose a reason for hiding this comment

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

wouldn't be better to prefix these methods' names w/ something to avoid possible clashes w/ odoo internals?

line_rs.reconcile(
writeoff_acc_id=writeoff_account,
writeoff_journal_id=self.journal_id
)
Copy link

Choose a reason for hiding this comment

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

indent back

string='Date of reconciliation',
default='newest',
)
filter = fields.Char(
Copy link

Choose a reason for hiding this comment

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

can we prefix this field name to avoid clash w/ built-in?

]

def _get_rec_method(self):
return self._get_all_rec_method()
Copy link

Choose a reason for hiding this comment

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

this indirection is not needed

Copy link
Author

Choose a reason for hiding this comment

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

Not sure what you mean here

Copy link

Choose a reason for hiding this comment

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

you can merge "_get_all_rec_method" into this method and rename it _selection_name (I don't see those methods used somewhere else)

Copy link

Choose a reason for hiding this comment

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

hmm, thinking about it twice: _get_all_rec_method it's kind of an api method, so:

  1. rename _get_rec_method to _selection_name
  2. rename _get_all_rec_method to _get_reconcilation_methods

self.env['account.mass.reconcile.method']
)
self.mass_rec = self.mass_rec_obj.create(
{
Copy link

Choose a reason for hiding this comment

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

poor indentation here


class TestReconcile(common.TransactionCase):

def setUp(self):
Copy link

Choose a reason for hiding this comment

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

we can use SavepointCase + setUpClass to avoid creating records again and again on each test.

@mpanarin mpanarin force-pushed the 11.0-mig-account_mass_reconcile branch 2 times, most recently from 70cf63d to fcbf3e3 Compare January 10, 2018 09:31
@@ -0,0 +1,602 @@
# Translation of Odoo Server.
Copy link

Choose a reason for hiding this comment

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

Remove the file

@mpanarin mpanarin changed the title [WIP] 11.0 mig account mass reconcile [ADD] 11.0 mig account mass reconcile Feb 2, 2018
@mpanarin mpanarin force-pushed the 11.0-mig-account_mass_reconcile branch from 84c7018 to f5e3266 Compare February 2, 2018 10:14
Copy link
Member

@yvaucher yvaucher left a comment

Choose a reason for hiding this comment

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

LGTM in term of code, however for the history, I see some issues.

Here we are loosing all history prior to 2952cbe , we probably want to include the history of account_easy_reconcile and account_advanced_reconcile

https://github.com/OCA/account-reconcile/commits/10.0/account_easy_reconcile
https://github.com/OCA/account-reconcile/commits/10.0/account_advanced_reconcile

  • those translations commits can be squashed

Copy link
Contributor

@grindtildeath grindtildeath left a comment

Choose a reason for hiding this comment

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

Some non-blocking things, otherwise looks good but please consider @yvaucher 's last comment.

'license': 'AGPL-3',
"auto_install": False,
'installable': True,
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we want to keep this key ?

Choose a reason for hiding this comment

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

no they are defaults, not needed

)
reconcile_line_ids = fields.Many2many(
comodel_name='account.move.line',
relation='account_move_line_history_rel',
string='Reconciled Items',
compute='_get_reconcile_line_ids'
compute='_get_reconcile_line_ids',
Copy link
Contributor

Choose a reason for hiding this comment

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

Compute method should start with _compute

Choose a reason for hiding this comment

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

@mpanarin ping?

@simahawk
Copy link

@mpanarin check @yvaucher comment on history pls

@mpanarin mpanarin force-pushed the 11.0-mig-account_mass_reconcile branch from f5e3266 to a3edc47 Compare April 24, 2018 11:36
@mpanarin
Copy link
Author

@simahawk @yvaucher
history added + few fix commit that came after my pr

@mpanarin mpanarin force-pushed the 11.0-mig-account_mass_reconcile branch 4 times, most recently from d7dc959 to 8aab378 Compare April 24, 2018 12:59
sebastienbeau and others added 9 commits April 25, 2018 11:46
For reconciling entries automatically (lp:account-extra-addons rev 22)
based on account_easy_reconcile (lp:c2c-financial-addons/6.1 rev 24.1.20)
using account_easy_reconcile (lp:c2c-financial-addons/6.1 rev 24.2.1)
… of code

which have still to be exectuted, not sure if it will runs or just miserably crash
(lp:c2c-financial-addons/6.1 rev 24.2.2)
(lp:c2c-financial-addons/6.1 rev 24.2.6)
(lp:c2c-financial-addons/6.1 rev 24.2.7)
(lp:c2c-financial-addons/6.1 rev 58)
…ents that we made.

This is mostly based on :
  account_statement_ext -> provide profile per bank statement, remove period, choose to use balance check or not,...
  account_statement_base_completion -> provide a completion rule system to fullfill the bank statement (partner, account,...)
  account_statement_base_import -> provide a base to create your own file parser for each bank/office and link it to a profile
  account_statement_transactionid_completion and account_statement_transactionid_import to use the transaction ID recorded in th SO
  account_advanced_reconcile -> An advanced way to setup reconciliation rules on every account
  account_financial_report_webkit -> some little fixes
(lp:c2c-financial-addons/6.1 rev 63)
guewen and others added 26 commits April 25, 2018 12:38
…rtially reconciled items

from a profile to easy the verification and controlling
[IMP] add translations
[FIX] add decorator multi
@mpanarin mpanarin force-pushed the 11.0-mig-account_mass_reconcile branch from 8aab378 to 429716f Compare April 25, 2018 09:38
@yvaucher yvaucher merged commit a89652a into OCA:11.0 Jun 28, 2018
@emagdalenaC2i emagdalenaC2i mentioned this pull request Dec 28, 2018
26 tasks
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