[ADD] l10n_sa_edi_branch: Added Branch CRN #193213
Conversation
|
|
C3POdoo
requested review from
a team and
alialfie
and removed request for
a team
alny-odoo
force-pushed
the
16.0-l10n_sa_edi-imp-alny
branch
from
28e06fd to
9b10c74
Compare
alny-odoo
changed the title
alny-odoo
force-pushed
the
16.0-l10n_sa_edi-imp-alny
branch
2 times, most recently
from
3f4acb5 to
991fee5
Compare
There was a problem hiding this comment.
Thanks for your contribution! Just a few things to mention:
1- Please add the translation files.
2- Nitpicking, but to follow the git guidelines, let's add the task id at the end of the commit message in this format task-4119982.
3- The commit's full description should also follow the guidelines.
| @api.depends("company_id.l10n_sa_use_branch_cr") | ||
| def _compute_l10n_sa_branch_cr(self): | ||
| self.l10n_sa_branch_cr = False |
There was a problem hiding this comment.
Reading this the first time was confusing, I thought "why is he setting l10n_sa_branch_cr to False whenever l10n_sa_use_branch_cr changes, regardless of its value?" but then reading the chatter on the task page, I understood why. I think it could be better to only do this if l10n_sa_use_branch_cr is False too, or at least writing a comment explaining this as it's a bit odd.
Also, why do we even need this? You're checking if invoice.journal_id.l10n_sa_use_branch_cr and invoice.journal_id.l10n_sa_branch_cr so even if there is a value in l10n_sa_branch_cr, it won't be used if the checkbox is not checked.
There was a problem hiding this comment.
I added a filtered for records that have l10n_sa_use_branch_cr as False, and added a comment however I kept the check incase the field was added through studio and populated but the setting was turned off.
| class ResCompany(models.Model): | ||
| _inherit = "res.company" | ||
|
|
||
| l10n_sa_use_branch_cr = fields.Boolean(string='Branch CRN') |
There was a problem hiding this comment.
Shouldn't the string value be "Use branch CRN"? Same question for the rest of the field/method names.
There was a problem hiding this comment.
Using Branch CRN was just preferred from Yatin since it looks better on the config settings.
Additional Identification Number (CRN) submitted to ZATCA can be set on the Journal if a new Branch CRN setting is enabled. This allows to have different registration numbers per sales journal. This change is required as per ZATCA rule BR-KSA-08: In the case where a company has multiple commercial registrations, such as different branches, the seller should fill the commercial registration of the branch in respect of which the Tax Invoice is being issued. In the case of Odoo, the branch CRN will be set on the Journal. task-4119982
alny-odoo
force-pushed
the
16.0-l10n_sa_edi-imp-alny
branch
from
991fee5 to
2b2a129
Compare
| l10n_sa_branch_crn = fields.Char("Branch CRN", | ||
| copy=False, | ||
| help="Can be used when the company has multiple branches that share the same VAT number, but have different commercial registration numbers.\ | ||
| Keep this field empty to use the Identification Number set on the company.", | ||
| compute="_compute_l10n_sa_branch_crn", | ||
| store=True, | ||
| readonly=False, | ||
| tracking=True) |
There was a problem hiding this comment.
| l10n_sa_branch_crn = fields.Char("Branch CRN", | |
| copy=False, | |
| help="Can be used when the company has multiple branches that share the same VAT number, but have different commercial registration numbers.\ | |
| Keep this field empty to use the Identification Number set on the company.", | |
| compute="_compute_l10n_sa_branch_crn", | |
| store=True, | |
| readonly=False, | |
| tracking=True) | |
| l10n_sa_branch_crn = fields.Char( | |
| "Branch CRN", | |
| copy=False, | |
| help="Can be used when the company has multiple branches that share the same VAT number, " | |
| "but have different commercial registration numbers. Keep this field empty to use " | |
| "the Identification Number set on the company.", | |
| compute="_compute_l10n_sa_branch_cr", | |
| store=True, | |
| readonly=False, | |
| tracking=True, | |
| ) |
formatting + comma at the end
| # Part of Odoo. See LICENSE file for full copyright and licensing details. | ||
|
|
||
| { | ||
| 'name': 'Saudi Arabia - E-Invoicing Branches', |
There was a problem hiding this comment.
Please also include the translations for the strings in the manifest. They should be added to base/i18n/base.pot
#Task
As per ZATCA rule BR-KSA-08: In the case where a company has multiple commercial registrations, such as different branches, the seller should fill the commercial registration of the branch in respect of which the Tax Invoice is being issued. In the case of Odoo, the branch CRN will be set on the Journal.
Description of the issue/feature this PR addresses:
Currently there is no option to link a Branch CRN to a company, as the only option is to have multiple companies with the same VAT but different Additional Identification Number values.
Current behavior before PR:
Additional Identification Number submitted to ZATCA is set directly on the company and does not allow for multiple branches by company
Desired behavior after PR is merged:
Additional Identification Number (CRN) submitted to ZATCA can be set on the Journal which allows to have different registration numbers per journal
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr