[16.0] l10n_br_fiscal fixes + docstring love #3789
Conversation
|
Hi @renatonlima, |
rvalyi
force-pushed
the
16.0-fiscal-fixes
branch
2 times, most recently
from
a24e690 to
8f6c42f
Compare
rvalyi
force-pushed
the
16.0-fiscal-fixes
branch
4 times, most recently
from
9153f1e to
71afbb7
Compare
l10n_br_fiscal/models/document.py
Outdated
| @api.onchange("fiscal_operation_id") | ||
| def _onchange_fiscal_operation_id(self): | ||
| # Call the super method from document.mixin.methods. | ||
| # This will handle: | ||
| # - Setting self.fiscal_operation_type | ||
| # - Setting self.edoc_purpose | ||
| # - Rebuilding self.document_subsequent_ids using odoo.fields.Command | ||
| result = super()._onchange_fiscal_operation_id() | ||
|
|
||
| # Specific logic for document.py after super() has run. | ||
| # The assignments to fiscal_operation_type and edoc_purpose might be | ||
| # redundant now if super() handles them comprehensively. | ||
| # Review if these are still needed or if super() is sufficient. | ||
| if self.fiscal_operation_id: | ||
| # If super() already sets these based on self.fiscal_operation_id, | ||
| # these lines might not be strictly necessary unless there's a reason | ||
| # for them to be re-set or set differently in this child class. | ||
| self.fiscal_operation_type = self.fiscal_operation_id.fiscal_operation_type | ||
| self.edoc_purpose = self.fiscal_operation_id.edoc_purpose | ||
|
|
||
| if self.issuer == DOCUMENT_ISSUER_COMPANY and not self.document_type_id: | ||
| # This logic for setting document_type_id seems specific to | ||
| # 'document.py' context (e.g., based on 'self.issuer' which | ||
| # might be a field on 'document.py' or its mixins). | ||
| # Keep this if it's not handled by the mixin or needs to be | ||
| # explicitly here. | ||
| if ( | ||
| self.fiscal_operation_id | ||
| and self.issuer == DOCUMENT_ISSUER_COMPANY | ||
| and not self.document_type_id | ||
| ): # Only set if not already set | ||
| self.document_type_id = self.company_id.document_type_id | ||
|
|
||
| subsequent_documents = [(6, 0, {})] | ||
| for subsequent_id in self.fiscal_operation_id.mapped( | ||
| "operation_subsequent_ids" | ||
| ): | ||
| subsequent_documents.append( | ||
| ( | ||
| 0, | ||
| 0, | ||
| { | ||
| "source_document_id": self.id, | ||
| "subsequent_operation_id": subsequent_id.id, | ||
| "fiscal_operation_id": subsequent_id.subsequent_operation_id.id, | ||
| }, | ||
| ) | ||
| ) | ||
| self.document_subsequent_ids = subsequent_documents | ||
| # The logic for rebuilding 'document_subsequent_ids' with tuple commands | ||
| # has been removed from here. The super() call now handles this using | ||
| # odoo.fields.Command via the updated mixin method. | ||
|
|
||
| return result |
There was a problem hiding this comment.
Acho que não tá legal, é muito comentário pra pouco código
Documentar é importante, mas sem exagero. Comentários muito longos tornam o código mais difícil de manter, porque exigem que a gente fique sempre conferindo se ainda estão alinhados com o que o código faz.
There was a problem hiding this comment.
essa parte dos onchanges foi o Gemini que pegou uns bugs e propois esse refator. Deixei comentado porque ele comentou as duvidas que ficaram. Deixei apenas para a gente rever mesmo... Por isso o PR em rascunho mesmo...
Do restante fora esse onchange da Kmee com as operações subsequentes (sem teste para variar...) ele não achou muita coisa zoada não, o que é um sinal bastante bom e que eu queria checar antes de jogar o modulo para a 17.0...
There was a problem hiding this comment.
disclaimer: Eu não botei para revisar o res_company.py onde sabemos que a Kmee extrapolou muito nas questões das taxas efetivas e que ta sem teste. Mas como vcs @antoniospneto e @renatonlima tavam refartorando e botando testes, eu pensei que não adiantava revisar o código atual disso.
There was a problem hiding this comment.
entendi só pra validar se a alteração que ele fez faz sentido
rvalyi
force-pushed
the
16.0-fiscal-fixes
branch
5 times, most recently
from
efb4147 to
748fa4e
Compare
Some global AI assisted review of the main l10n_br_fiscal files. I carefully added docstrings to:
I could also fix small bugs in the process.
The idea is to lower the entry barrier to new comers to what is currently the largest f the 3000 OCA modules...