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

[14.0][MIG] Module stock_picking_invoicing #1014

Merged
merged 112 commits into from
Nov 12, 2021

Conversation

mbcosta
Copy link
Contributor

@mbcosta mbcosta commented Oct 7, 2021

Migration module stock_picking_invoicing to v14, the PR #1010 are missing the last commits made in v13:

image

cc @rvalyi @renatonlima @mileo @Zar21

mileo and others added 30 commits October 5, 2021 12:47
* Loading
* Onchanges
Etc
* Good Price with pricelist
* Good Taxes
* group works
* Everything is filtered with company
* Get the correct taxes and account regarding Fiscal Position
[IMP]Add the availability to choose the company that will invoice
@@ -498,15 +498,15 @@ def _update_picking_invoice_status(self, pickings):
return pickings._set_as_invoiced()

def ungroup_moves(self, grouped_moves_list):
""" Ungrup your moves, split them again, grouping by
"""Ungrup your moves, split them again, grouping by
Copy link

Choose a reason for hiding this comment

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

Suggested change
"""Ungrup your moves, split them again, grouping by
"""Ungroup your moves, split them again, grouping by

fiscal position, max itens per invoice and etc
:param grouped_moves_list:
:return: list of grouped moves list
"""
return [grouped_moves_list]

def _create_invoice(self, invoice_values):
""" Overrite this metothod if you need to change any values of the
"""Overrite this metothod if you need to change any values of the
Copy link

Choose a reason for hiding this comment

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

Override

Copy link
Member

Choose a reason for hiding this comment

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

metothod -> method

Copy link
Member

@rvalyi rvalyi left a comment

Choose a reason for hiding this comment

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

Hello @mbcosta

  1. this commit seems an error: when removing such decorators, methods become @api.multi by default. Something there are not as they work on the RecordSet Model and not a recordset itself (look at other default_get declarations in other modules) f7662ba
  2. contributors are not expected to re-generate REAME files in specific commits such as 6773f41 as these are re-generated by the OCA bots anyway. So better if you remove that commit too.
  3. Once this commit is removed it's better if you squash 4fe918a onto the previous migration commit because such trivial code formating commits don't deserve dedicated commit (it pollutes the log).

Use rebase -i for this cleanup. The othercommits are OK.

@mbcosta mbcosta force-pushed the 14.0-mig-stock_picking_invoicing branch from 2d4d244 to ed6eeba Compare November 12, 2021 17:15
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@rvalyi
Copy link
Member

rvalyi commented Nov 12, 2021

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 14.0-ocabot-merge-pr-1014-by-rvalyi-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit d0d94e4 into OCA:14.0 Nov 12, 2021
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at b782aa5. Thanks a lot for contributing to OCA. ❤️

@mbcosta
Copy link
Contributor Author

mbcosta commented Nov 12, 2021

Thanks all for reviews, the PRs #1010 and #800 should be closed after this merge.

@rvalyi
Copy link
Member

rvalyi commented Nov 12, 2021

Thanks all for reviews, the PRs #1010 and #800 should be closed after this merge.

Done!

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