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

[ADD] 14.0 account_move_cutoff: deferred revenu/expenses #256

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

petrus-v
Copy link
Contributor

Using this module cuttrof happens while posting
entries based on start/end date

This module intentionally not depends on account_cutoff_base

@petrus-v petrus-v force-pushed the 14.0-add-account-move-cutoff branch 5 times, most recently from 914878e to 3a223a1 Compare July 31, 2023 12:27
Copy link
Member

@damdam-s damdam-s left a comment

Choose a reason for hiding this comment

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

some remarks but LGTM

account_move_cutoff/models/account_move.py Outdated Show resolved Hide resolved
account_move_cutoff/models/account_move_line.py Outdated Show resolved Hide resolved
account_move_cutoff/models/account_move_line.py Outdated Show resolved Hide resolved
account_move_cutoff/models/account_move_line.py Outdated Show resolved Hide resolved
account_move_cutoff/models/account_move_line.py Outdated Show resolved Hide resolved
account_move_cutoff/readme/CONFIGURE.rst Outdated Show resolved Hide resolved
@petrus-v
Copy link
Contributor Author

petrus-v commented Aug 1, 2023

Last empty commit is a workaround github issue: https://github.com/orgs/community/discussions/62366 I'll squash it...

@petrus-v petrus-v force-pushed the 14.0-add-account-move-cutoff branch from 7a00d88 to 3d6634d Compare August 1, 2023 09:30
Copy link

@Koudja Koudja left a comment

Choose a reason for hiding this comment

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

Looks good, minor typos comments (lots of them, sorry)

account_move_cutoff/README.rst Show resolved Hide resolved
account_move_cutoff/README.rst Show resolved Hide resolved
account_move_cutoff/README.rst Show resolved Hide resolved
account_move_cutoff/i18n/fr.po Show resolved Hide resolved
account_move_cutoff/i18n/fr.po Show resolved Hide resolved
account_move_cutoff/i18n/fr.po Show resolved Hide resolved
account_move_cutoff/models/account_move.py Show resolved Hide resolved
line_amounts = {}
if self.cutoff_method == "equal":
line_amounts = self._get_amounts_per_periods_equal()
else: # monthly_prorata_temporis
Copy link

Choose a reason for hiding this comment

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

elif with exception if cutoff_method is not recognized ? this seems dangerous otherwise

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cutoff_method is a field selection with 2 value today, ORM ensure possible value before to be able to call this code I think. So it will be hard to test exception case 🤔

account_move_cutoff/readme/DESCRIPTION.rst Show resolved Hide resolved
account_move_cutoff/readme/ROADMAP.rst Show resolved Hide resolved
@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). 🤖

@damdam-s
Copy link
Member

@pedrobaeza @fclementic2c any chance this one gets reviewed?

@pedrobaeza pedrobaeza added this to the 14.0 milestone Nov 29, 2023
@pedrobaeza
Copy link
Member

Merging due to existing reviews, but not reviewed by me: /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-256-by-pedrobaeza-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Nov 29, 2023
Signed-off-by pedrobaeza
@OCA-git-bot
Copy link
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-256-by-pedrobaeza-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@pedrobaeza
Copy link
Member

pre-commit should be fixed for this branch, as an update of the checkers make that some existing modules are not passing.

@damdam-s
Copy link
Member

thanks a lot. We will update this PR and ping you when it's all green again.

Using this module cuttoff happens while posting
entries based on start/end date defined on invoice lines

Co-authored-by: Damien Crier <Damien.crier@foodles.co>
@petrus-v
Copy link
Contributor Author

@damdam-s @pedrobaeza thanks !

I've rebase this branch and fix linters complaints

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-256-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit f276707 into OCA:14.0 Nov 30, 2023
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 3f320d7. 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.

5 participants