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

[18.0][MIG] hr_expense_payment: Migration to 18.0 #280

Merged
merged 26 commits into from
Jan 9, 2025

Conversation

lef-adhoc
Copy link

No description provided.

Saran440 and others added 25 commits December 4, 2024 13:26
Currently translated at 100.0% (6 of 6 strings)

Translation: hr-expense-16.0/hr-expense-16.0-hr_expense_payment
Translate-URL: https://translation.odoo-community.org/projects/hr-expense-16-0/hr-expense-16-0-hr_expense_payment/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: hr-expense-16.0/hr-expense-16.0-hr_expense_payment
Translate-URL: https://translation.odoo-community.org/projects/hr-expense-16-0/hr-expense-16-0-hr_expense_payment/
@lef-adhoc lef-adhoc force-pushed the 18.0-mig-hr_expense_payment branch from df98435 to 6660847 Compare December 4, 2024 19:11
@lef-adhoc lef-adhoc mentioned this pull request Dec 4, 2024
7 tasks
@mav-adhoc
Copy link

@pedrobaeza Hi! Can you review it please? Thanks!

@pedrobaeza
Copy link
Member

/ocabot migration hr_expense_payment

Sorry, not using this module.

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Dec 12, 2024
@mav-adhoc
Copy link

@pedrobaez Don't be sorry! Thank you very much for your time!

@ced-adhoc
Copy link

LGTM

@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). 🤖

@ced-adhoc
Copy link

Hi @Saran440 could you please check this? Thanks in advance

@ced-adhoc
Copy link

Hi @pedrobaeza, could you please check this pr? thanks!

@pedrobaeza
Copy link
Member

A PSC must review this PR.

@ced-adhoc
Copy link

@pedrobaeza
Copy link
Member

Yeah, but as said, I'm not using this module, so I can't validate that is correct. Their authors/contributors can also check.

@ced-adhoc
Copy link

Oops, I'm sorry @pedrobaeza , I missed your message 2 weeks ago. My bad.

Copy link
Member

@Saran440 Saran440 left a comment

Choose a reason for hiding this comment

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

Code Review.

In V16+, expense sheet will create journal entries with bill. some code can change.

def _create_payment_vals_from_batch(self, batch_result):
payment_vals = super()._create_payment_vals_from_batch(batch_result)
expense_sheet_ids = self._context.get("expense_sheet_ids", False)
if expense_sheet_ids:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if expense_sheet_ids:
if expense_sheet_ids:
moves = self.env["account.move"].browse(batch_result["lines"].move_id.ids)
payment_vals.update(expense_sheet_ids=moves.expense_sheet_id.ids)

reduct browse and filter function.

@lef-adhoc
Copy link
Author

Hi @Saran440,
I’ve made a new commit addressing your feedback. Let me know if it looks good!
Thanks! 🙌

Copy link
Member

@Saran440 Saran440 left a comment

Choose a reason for hiding this comment

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

Code reviewed

@Saran440
Copy link
Member

Saran440 commented Jan 9, 2025

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 18.0-ocabot-merge-pr-280-by-Saran440-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 92c5ac5 into OCA:18.0 Jan 9, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

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.