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

17.0 l10n ke edi oscu dako #162196

Draft
wants to merge 2 commits into
base: 17.0
Choose a base branch
from

Conversation

dbkosky
Copy link
Contributor

@dbkosky dbkosky commented Apr 17, 2024

🐳

@dbkosky dbkosky requested a review from oco-odoo April 17, 2024 11:08
@robodoo
Copy link
Contributor

robodoo commented Apr 17, 2024

@dbkosky dbkosky marked this pull request as draft April 17, 2024 11:09
@C3POdoo C3POdoo added the RD research & development, internal work label Apr 17, 2024
addons/l10n_ke_edi_oscu/wizard/__init__.py Outdated Show resolved Hide resolved
addons/l10n_ke_edi_oscu_mrp/__init__.py Outdated Show resolved Hide resolved
addons/l10n_ke_edi_oscu/__init__.py Outdated Show resolved Hide resolved
_logger.info("Company %s already has the Kenyan localization installed, updating...", company.name)
ChartTemplate = env['account.chart.template'].with_company(company)
ChartTemplate._load_data({
'account.tax': ChartTemplate._get_ke_account_tax_etims_type(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't it be made a little more straightforward by simply removing the _get_ke_account_tax_etims_type function, and running its code directly here ? It's just one line anyway, and the function is not called from other places.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the _get_ke_account_tax_etims_type function also serves the purpose of setting the l10n_ke_tax_type_id on taxes for new companies (or when reloading the CoA), thanks to its @template decorator.


def _post_init_hook(env):
# UNSPSC category codes can be used in Kenya.
product_unspsc = env['product.unspsc.code'].search([('active', '=', False), ('code', '=ilike', '%00')])
Copy link
Contributor

Choose a reason for hiding this comment

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

But doing it like that, we enable the codes for all other countries as well in the database. So, another company in a different country not needing them will get them as well. Isn't there a better way ? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it would cause issues for companies in other countries. As far as I can see, the UNSPSC codes are just needed for some EDIs. I assume that if an EDI doesn't accept a Category Code for a product, this would simply result in some kind of error message for the user, who would then need to choose a different code.

return super()._default_report_footer()

company_details = fields.Html(default=_default_company_details)
report_footer = fields.Html(default=_default_report_footer)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not very robust if another module does the same kind of override

**partner._l10n_ke_oscu_partner_content() # Partner details
}
company = partner.company_id or self.env.company
error, data, dummy = company._l10n_ke_call_etims('saveBhfCustomer', content)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing done with the returned value because it's like registering the partner to the government, is that it ?

class Users(models.Model):
_inherit = 'res.users'

l10n_ke_oscu_company_ids = fields.One2many('res.company', compute='_compute_l10n_ke_oscu_company_ids')
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it really useful to make a computed field just for that ? It seems to be used only in the function below, and is anyway a mere filtering of another field.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, need to how important it is to send for all branches.

'pwd': 'test',
'useYn': 'Y',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'pwd': 'test',
'useYn': 'Y',
'pwd': 'test',
'useYn': 'Y',

We usually write this kind of things like this. I'm not sure the style check will like your way ^^

))
)

def action_l10n_ke_create_branch_user(self): # TODO - this is (again) just called with a button.. maybe it's better to do this automatically in the background
Copy link
Contributor

@oco-odoo oco-odoo Apr 24, 2024

Choose a reason for hiding this comment

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

Not sure ; indeed we do that in a lot of places. And there is no way to know what has been submitted already, is there ?

- Disable Send to Fiscal Device button if l10n_ke_edi_oscu is installed
- Only add TREMOL fields to invoice PDF if l10n_ke_cu_invoice_number set
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants