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

10.0 mig pos cash move reason mgs #177

Conversation

GillesTephaneMeyomesse
Copy link

No description provided.

@rafaelbn rafaelbn added this to the 10.0 milestone Jun 8, 2017
@GillesTephaneMeyomesse GillesTephaneMeyomesse force-pushed the 10.0-mig-pos_cash_move_reason_mgs branch 3 times, most recently from 5962b9b to f575797 Compare June 27, 2017 10:30
# I get the statement from the session
statement = self.env['account.bank.statement'].search(
[('pos_session_id', '=', self.session.id),
('journal_id', '=', self.cash_journal.id)])
Copy link
Contributor

Choose a reason for hiding this comment

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

You should add limit=1 to prevent errors on line 93.

_register = False

@api.onchange('product_id')
def onchange_reason(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be named _onchange_product.

@GillesTephaneMeyomesse
Copy link
Author

@benwillig thanks to review this

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Hi @GillesTephaneMeyomesse. thanks for porting this usefull module !
I was working on the 8.0 version and made some improvment available here. (some refactor, reducing code size) #248
could you review the PR and cherry pick the according commit ?

regards.



class PosBoxCashMoveReason(PosBox):
_register = False

@api.onchange('product_id')
def onchange_reason(self):
def _onchange_reason(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you changed the name of this function ?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's the naming convention defined in the OCA contributing guidelines. You can check it there https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md#models

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks ! I didn't know.

@legalsylvain
Copy link
Contributor

Hi. @benwillig could you update your review ?
@GillesTephaneMeyomesse : could you cherry pick the commits.

After I think we can merge.

kind regards.

@pedrobaeza pedrobaeza mentioned this pull request Oct 26, 2018
11 tasks
@legalsylvain
Copy link
Contributor

ping @GillesTephaneMeyomesse and @benwillig.
If you have no time to finish this work, I can do it.
Take me in touch.

kind regards.

@Cedric-Pigeon
Copy link

@legalsylvain Please to proceed. You can make a PR to our branch. I will merge it .

@rousseldenis
Copy link
Sponsor Contributor

@legalsylvain

@legalsylvain
Copy link
Contributor

I will not work to the 10.0 version. (directly on 12.0).
I made a PR here : #384
but if this PR is ok, we can merge it.
regards.

'summary': """""",
'author': 'ACSONE SA/NV,'
'Odoo Community Association (OCA)',
'website': "http://acsone.eu",
Copy link

Choose a reason for hiding this comment

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

@legalsylvain
Copy link
Contributor

closing, due to inactivity.

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

Successfully merging this pull request may close these issues.

None yet

8 participants