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

Adding account move select reconciliation 7 #40

Merged
merged 12 commits into from
Feb 10, 2016

Conversation

eLBati
Copy link
Member

@eLBati eLBati commented Sep 9, 2014

This module allows to manually select the journal item to be reconciled while
registering a journal entry.

Example (also see the included test case)

You have an open credit and you need to close it by a manual journal entry.
With this module you can manually create the payment journal entry, select the
open credit and click 'Reconcile Line'. The system will close the credit
generating the respective reconciliation.

##############################################################################
#
# Copyright (C) 2014 Agile Business Group sagl
# (<http://www.agilebg.com>)
Copy link
Member

Choose a reason for hiding this comment

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

I think here has been used a tab as indentation instead spaces

Copy link
Member

Choose a reason for hiding this comment

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

See eLBati#1

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @tafaRU

@eLBati eLBati force-pushed the adding_account_move_select_reconciliation_7 branch from 4164bdf to e1eb829 Compare September 12, 2014 15:33
@coveralls
Copy link

Coverage Status

Coverage increased (+1.23%) when pulling e1eb829 on eLBati:adding_account_move_select_reconciliation_7 into a0220d8 on OCA:7.0.

@sbidoul
Copy link
Member

sbidoul commented Oct 3, 2014

I did a quick test of this module and it works well.

That said, I'm always a bit worried when I see modules adding columns to account.move.line. And in this case the new column is there only to satisfy a pure UI/ergonomy requirement. Would it not be better to have a wizard on the move form to select the line to reconcile and add it to the move (plus the red button to unreconcile)? What do you think?

On a related note, is it correct that the same effect can be achieved by selecting one move line (in the Journal Items view) and doing a reconcile with writeoff?

@eLBati
Copy link
Member Author

eLBati commented Oct 6, 2014

Hello @sbidoul , what problem do you see in adding a column to account.move.line?

About related note: the same effect can be achieved by the 'reconcile entries' wizard. This module allows to reconcile at the same time of registering a journal entry

@sbidoul
Copy link
Member

sbidoul commented Oct 6, 2014

Hello @eLBati, I can't really say the additional column will create hard issues. It's just making me uneasy to add a column that will use space (and time in foreign key checks), where this column is only used at data entry time.

'move_line_to_reconcile_id': fields.many2one(
'account.move.line',
'Move Line To Reconcile',
domain="[('reconcile_id', '=', False), \
Copy link

Choose a reason for hiding this comment

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

IMHO is not really interesting to set a domain directly in the model, especially in this case where you're passing an string as domain (it will be evaluated on the client side). If you're setting a client side domain, I think it's better to set it on the xml form.

@StefanRijnhart
Copy link
Member

LGTM 👍. I'm not too worried about nags expressed by the previous reviewers, i.e. the extra column in the move line table, nor the text domain on the column itself.

<field name="model">account.move</field>
<field name="inherit_id" ref="account.view_move_form"/>
<field name="arch" type="xml">
<xpath expr="/form/sheet/notebook/page[@string='Journal Items']/field[@name='line_id']/tree/field[@name='name']" position="after">
Copy link
Member

Choose a reason for hiding this comment

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

By changing this to //field[@name='line_id']/tree/field[@name='name'] the module works on 8.0 too.

@sbidoul
Copy link
Member

sbidoul commented May 1, 2015

Hi @eLBati, @tafaRU,

I finally had a chance to test this module (on 8.0). It works great!

Have you considered reconciling several moves of the same account at once? With move_line_to_reconcile_id would becoming a many2many instead of a many2one, the module would easily cover additional use cases (such as closing several open moves at once with a complex operation).

@max3903 max3903 added this to the 7.0 milestone Sep 18, 2015
@max3903
Copy link
Sponsor Member

max3903 commented Sep 18, 2015

👍

max3903 pushed a commit that referenced this pull request Feb 10, 2016
…liation_7

[ADD] Adding account move select reconciliation 7
@max3903 max3903 merged commit cac13d3 into OCA:7.0 Feb 10, 2016
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

8 participants