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

8.0 Migration of account_constraints and account_default_draft_move #34

Merged
merged 18 commits into from
Nov 24, 2014

Conversation

adrienpeiffer
Copy link
Contributor

Port account default draft move and account constraint to 8.0

@pedrobaeza
Copy link
Member

Why are you including also account_constraints in this PR?

@adrienpeiffer
Copy link
Contributor Author

@pedrobaeza Because account_default_draft_move depends on account_constraints

@coveralls
Copy link

Coverage Status

Coverage decreased (-35.9%) when pulling cc7fdaf on acsone:8.0-account_default_draft_move into 9fb1c5a on OCA:8.0.

@eLBati
Copy link
Member

eLBati commented Sep 3, 2014

About travis configuration, please see #36

@coveralls
Copy link

Coverage Status

Coverage decreased (-35.9%) when pulling cc7fdaf on acsone:8.0-account_default_draft_move into 9fb1c5a on OCA:8.0.

@pedrobaeza
Copy link
Member

Ah, OK. Is there any test we can run to check functionality?

@coveralls
Copy link

Coverage Status

Coverage decreased (-31.1%) when pulling bd15bfa on acsone:8.0-account_default_draft_move into 9fb1c5a on OCA:8.0.

return True
err_msg = (_('Invoice name (id): %s (%s)') %
(line.invoice.name, line.invoice.id))
raise models.except_orm(
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to import directly openerp.exceptions and use directly exceptions.except_orm ?

@coveralls
Copy link

Coverage Status

Coverage decreased (-30.72%) when pulling 5767981 on acsone:8.0-account_default_draft_move into 9fb1c5a on OCA:8.0.

@coveralls
Copy link

Coverage Status

Coverage decreased (-30.72%) when pulling 5bb7b1e on acsone:8.0-account_default_draft_move into 9fb1c5a on OCA:8.0.

@coveralls
Copy link

Coverage Status

Coverage decreased (-30.72%) when pulling 5bb7b1e on acsone:8.0-account_default_draft_move into 9fb1c5a on OCA:8.0.

check=check)

@api.cr_uid_ids_context
def write(self, cr, uid, ids, vals, context=None, check=True,
Copy link
Member

Choose a reason for hiding this comment

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

You could use @api.multi here to remove cr, uid, ids and context

Then you could access context with: self._context

Copy link
Member

Choose a reason for hiding this comment

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

@yvaucher I prefer self.env.context because self._context accesses to a private attribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@guewen @yvaucher unlink and write method of account_move_line isn't yet migrated in new API and unlink attemps to write in context which is frozen.This is the major reason why these methods don't use the new API.
Also context argument isn't at the end of method declaration of these methods and this causes errors.

@coveralls
Copy link

Coverage Status

Coverage decreased (-22.29%) when pulling 43d50da on acsone:8.0-account_default_draft_move into 9fb1c5a on OCA:8.0.

@eLBati
Copy link
Member

eLBati commented Sep 12, 2014

Diff between 7.0 and 8.0:
account_default_draft_move
account_constraints

@coveralls
Copy link

Coverage Status

Coverage decreased (-18.32%) when pulling 65b2bca on acsone:8.0-account_default_draft_move into e4a06a3 on OCA:8.0.

@sbidoul
Copy link
Member

sbidoul commented Oct 3, 2014

Anyone willing to review this one?

@sbidoul
Copy link
Member

sbidoul commented Oct 24, 2014

Since account_constraint forbids the deletion of account moves that are linked to a bank statement, I added the possibility to delete them from the bank statement view, to provide a way to fix erroneous matches.

@sbidoul
Copy link
Member

sbidoul commented Oct 24, 2014

@fclementic2c would you like to test this one?

@adrienpeiffer
Copy link
Contributor Author

Thanks @sbidoul

@bwrsandman
Copy link

Needs are rebase

@sbidoul
Copy link
Member

sbidoul commented Nov 3, 2014

@bwrsandman rebased

@coveralls
Copy link

Coverage Status

Coverage decreased (-8.0%) when pulling 2ad0f86 on acsone:8.0-account_default_draft_move into 7894810 on OCA:8.0.

@tafaRU
Copy link
Member

tafaRU commented Nov 11, 2014

@guewen your comments are for sure useful, thanks for sharing them!

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling af67f03 on acsone:8.0-account_default_draft_move into 40af7a8 on OCA:8.0.

@adrienpeiffer
Copy link
Contributor Author

@guewen Thanks for your comments !

@guewen
Copy link
Member

guewen commented Nov 24, 2014

Thanks @adrienpeiffer !
👍

@sbidoul
Copy link
Member

sbidoul commented Nov 24, 2014

👍 as well

The only possible annoyance is that the cancel button (yellow arrow) on bank statement line will appear twice if account_cancel is installed). But since one goal of account_default_draft_move is to make account_cancel unncessary that should not be an issue.

guewen added a commit that referenced this pull request Nov 24, 2014
8.0 Migration of account_constraints and account_default_draft_move
@guewen guewen merged commit 7af8c08 into OCA:8.0 Nov 24, 2014
@sbidoul sbidoul deleted the 8.0-account_default_draft_move branch November 24, 2014 15:19
leemannd pushed a commit to camptocamp/account-financial-tools that referenced this pull request Oct 20, 2017
mileo added a commit to kmee/account-financial-tools that referenced this pull request Apr 17, 2019
* [FIX] Corrige a numeração dos lançamentos

* [REF] Simplificação da preparação do lançamento financeiro

* [REF] Remove accounting tab

* [NEW] Module: financial_account

* [REF] Refactoring create method to use it in another modules
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.

9 participants