[14.0][ADD] account_move_default_journal: A module to have more meaningful … #1287

ddejong-therp · 2021-12-06T12:39:50Z

…journals defaults in forms.

This can help you prevent having old Journals being used as the default in forms. For example, when you create a new booking entry, you don't accidentally pick a journal that should not be used anymore.

NL66278 · 2023-07-12T13:14:05Z

account_move_default_journal/models/account_move.py

+            ('type', 'in', journal_types),
+        ]
+        if self._context.get('default_currency_id'):
+            domain.push(


list does not have a push method, replace by append

NL66278

There is an obvious programming mistake with trying to use a push method on a domain (that is on a list).

Also the code does not adhere to conventions. Please use pre-commit to fix this.

NL66278 · 2023-07-13T10:08:55Z

@ddejong-therp pre-commit created setup files. Those must be git added and included in the PR. See details of pre-commit check. You might want to squash your commits.

NL66278 · 2023-07-13T10:09:30Z

account_move_default_journal/models/account_config_settings.py

@@ -0,0 +1,64 @@
+# Copyright 2021 Therp BV <https://therp.nl>


Yeah, the PR was put up in dec 2021 :p

NL66278 · 2023-07-13T10:10:52Z

account_move_default_journal/models/account_config_settings.py

+class ResConfigSettings(models.TransientModel):
+    _inherit = "res.config.settings"
+
+    account_move_default_general_journal_id = fields.Many2one(


This will not really work in a multi-company situation as journals are always linked to a specific company.

Have a look at option two here for company specific settings. ~~Note that set_values and get_values are not really needed any more if you use the config_parameter key in the field definition (see option 3 in the link):~~
https://gist.github.com/ryanc-me/a0e35fb6741d73c39f9e3968eaa485ee

What I've seen used a lot throughout Odoo 14.0, is the use of properties for this. A property has a default value for each company, and then in addition to that can also be set per each record. Examples of this are found in the stock_account module where a Stock Journal can be configured per product.category, but also in account module where a Receivable Account can be configured on res.partner with a default on the per-company default one.

This concept would be powerful here as well, and would simplify the code, because we could define a property on account.journal.type which could then contain a per-company default.

Anyway, it would just be a refactor to achieve the same, so not blocking IMO

NL66278 · 2023-07-13T10:36:52Z

account_move_default_journal/models/account_move.py

+                "account_move_default_journal.general_journal_id"
+            )
+
+        # Try to get the journal, or default back to normal behavior


If you use option 2 above (related field in res.config.settings to journal fields in company, you will have a journal you could directly return.

However if there is a default currency in the context that differs from the journal currency, you must still default to the standard search method.

NL66278 · 2023-07-13T10:39:21Z

account_move_default_journal/models/account_move.py

+        # Try to get the journal, or default back to normal behavior
+        if journal_id:
+            journal_id = int(journal_id)
+            journal = self.env["account.journal"].search(


Note that this might result in an empty record set. If you apply the other changes I suggested you do not need a search at all, therefore you could dispense with creating a domain altogether as well.

NL66278 · 2023-08-16T10:17:51Z

account_move_default_journal/models/account_move.py

+    def _search_default_journal(self, journal_types):
+        company_id = self._context.get("default_company_id", self.env.company.id)
+        company = self.env["res.company"].browse(company_id)
+        domain = [


You do not need this domain, so delete setting it.

AH I forgot to remove that yes.

NL66278

👍 LGTM

NL66278 · 2023-08-23T07:46:46Z

Please add some unit test to get this green. Suggestion:

Create journals of different types in the test setup, and set three of them up as company defaults;
Check whether they are correctly are found using the different types (sales, general...), including a type with no default (should fallback to default search)
Check searching for a type with the context set to a different currency (should fallback to default and find journal with correct currency.

I think this will get you to 100% overage,

ddejong-therp · 2023-11-20T19:29:13Z

I've now added the missing files. Checks are green now.

thomaspaulb · 2023-12-01T12:48:11Z

account_move_default_journal/models/account_config_settings.py

+class ResConfigSettings(models.TransientModel):
+    _inherit = "res.config.settings"
+
+    account_move_default_general_journal_id = fields.Many2one(


What I've seen used a lot throughout Odoo 14.0, is the use of properties for this. A property has a default value for each company, and then in addition to that can also be set per each record. Examples of this are found in the stock_account module where a Stock Journal can be configured per product.category, but also in account module where a Receivable Account can be configured on res.partner with a default on the per-company default one.

This concept would be powerful here as well, and would simplify the code, because we could define a property on account.journal.type which could then contain a per-company default.

Anyway, it would just be a refactor to achieve the same, so not blocking IMO

OCA-git-bot · 2023-12-01T13:00:02Z

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

rafaelbn · 2023-12-02T01:38:39Z

I'm not blocking, just asking because I don't understand.

Usually when I'm not using a Journal anymore I archive it!

I'm reading the readme and I don't understand it sorry

Best! 😄

github-actions · 2024-03-31T12:21:17Z

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

NL66278 · 2024-04-02T08:55:53Z

@rafaelbn Preventing the use of old journals was just an example of what the module can achieve. The basics is that the users get to control what journal is picked as default.

rafaelbn

/ocabot merge patch

OCA-git-bot · 2024-04-05T12:50:20Z

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-1287-by-rafaelbn-bump-patch, awaiting test results.

Signed-off-by rafaelbn

OCA-git-bot · 2024-04-05T13:07:59Z

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

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.

rafaelbn · 2024-04-06T15:24:24Z

/ocabot rebase

…default journals in account move forms.

OCA-git-bot · 2024-04-06T15:24:43Z

Congratulations, PR rebased to 14.0.

dreispt · 2024-05-02T17:18:47Z

/ocabot merge nobump

OCA-git-bot · 2024-05-02T17:19:01Z

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

OCA-git-bot · 2024-05-02T17:35:09Z

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

ddejong-therp changed the title ~~[ADD] account_move_default_journal: A module to have more meaningful …~~ [14.0][ADD] account_move_default_journal: A module to have more meaningful … Dec 6, 2021

ddejong-therp force-pushed the 14.0-account_move_default_journal branch from e6d1b4e to 96cd47b Compare May 3, 2023 09:18

NL66278 reviewed Jul 12, 2023

View reviewed changes

NL66278 suggested changes Jul 12, 2023

View reviewed changes

NL66278 reviewed Jul 13, 2023

View reviewed changes

ddejong-therp force-pushed the 14.0-account_move_default_journal branch 6 times, most recently from 62df896 to c862895 Compare August 15, 2023 13:24

NL66278 reviewed Aug 16, 2023

View reviewed changes

ddejong-therp force-pushed the 14.0-account_move_default_journal branch from c862895 to 992e5a5 Compare August 18, 2023 09:49

NL66278 approved these changes Aug 18, 2023

View reviewed changes

ddejong-therp force-pushed the 14.0-account_move_default_journal branch 5 times, most recently from 3b8efce to b27c9d4 Compare August 31, 2023 15:03

ddejong-therp force-pushed the 14.0-account_move_default_journal branch from b27c9d4 to eeb1323 Compare November 20, 2023 19:12

thomaspaulb approved these changes Dec 1, 2023

View reviewed changes

OCA-git-bot added approved ready to merge labels Dec 1, 2023

rafaelbn added this to the 14.0 milestone Dec 2, 2023

github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Mar 31, 2024

rafaelbn approved these changes Apr 5, 2024

View reviewed changes

OCA-git-bot added a commit that referenced this pull request Apr 5, 2024

Merge PR #1287 into 14.0

6170762

Signed-off-by rafaelbn

OCA-git-bot added the bot is merging ⏳ label Apr 5, 2024

OCA-git-bot removed the bot is merging ⏳ label Apr 5, 2024

ddejong-therp added 2 commits April 6, 2024 15:24

[ADD] account_move_default_journal: A module to have more meaningful …

cc8c6fc

…default journals in account move forms.

[ADD] account_move_default_journal: Added pre-commit generated files.

fd18fc0

OCA-git-bot force-pushed the 14.0-account_move_default_journal branch from eeb1323 to fd18fc0 Compare April 6, 2024 15:24

github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Apr 7, 2024

OCA-git-bot added the bot is merging ⏳ label May 2, 2024

OCA-git-bot merged commit 3b50a19 into OCA:14.0 May 2, 2024
11 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels May 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[14.0][ADD] account_move_default_journal: A module to have more meaningful … #1287

[14.0][ADD] account_move_default_journal: A module to have more meaningful … #1287

ddejong-therp commented Dec 6, 2021

NL66278 Jul 12, 2023

NL66278 left a comment

NL66278 commented Jul 13, 2023

NL66278 Jul 13, 2023

ddejong-therp Aug 14, 2023

NL66278 Jul 13, 2023

NL66278 Jul 13, 2023 •

edited

Loading

thomaspaulb Dec 1, 2023

NL66278 Jul 13, 2023

NL66278 Jul 13, 2023

NL66278 Aug 16, 2023

ddejong-therp Aug 18, 2023

NL66278 left a comment

NL66278 commented Aug 23, 2023

ddejong-therp commented Nov 20, 2023

thomaspaulb Dec 1, 2023

OCA-git-bot commented Dec 1, 2023

rafaelbn commented Dec 2, 2023

github-actions bot commented Mar 31, 2024

NL66278 commented Apr 2, 2024

rafaelbn left a comment

OCA-git-bot commented Apr 5, 2024

OCA-git-bot commented Apr 5, 2024

rafaelbn commented Apr 6, 2024

OCA-git-bot commented Apr 6, 2024

dreispt commented May 2, 2024

OCA-git-bot commented May 2, 2024

OCA-git-bot commented May 2, 2024

		@@ -0,0 +1,64 @@
		# Copyright 2021 Therp BV <https://therp.nl>

[14.0][ADD] account_move_default_journal: A module to have more meaningful … #1287

[14.0][ADD] account_move_default_journal: A module to have more meaningful … #1287

Conversation

ddejong-therp commented Dec 6, 2021

Choose a reason for hiding this comment

NL66278 left a comment

Choose a reason for hiding this comment

NL66278 commented Jul 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NL66278 Jul 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NL66278 left a comment

Choose a reason for hiding this comment

NL66278 commented Aug 23, 2023

ddejong-therp commented Nov 20, 2023

Choose a reason for hiding this comment

OCA-git-bot commented Dec 1, 2023

rafaelbn commented Dec 2, 2023

github-actions bot commented Mar 31, 2024

NL66278 commented Apr 2, 2024

rafaelbn left a comment

Choose a reason for hiding this comment

OCA-git-bot commented Apr 5, 2024

OCA-git-bot commented Apr 5, 2024

rafaelbn commented Apr 6, 2024

OCA-git-bot commented Apr 6, 2024

dreispt commented May 2, 2024

OCA-git-bot commented May 2, 2024

OCA-git-bot commented May 2, 2024

NL66278 Jul 13, 2023 •

edited

Loading