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.

Sign up for GitHub

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

Jump to bottom

[14.0][IMP] l10n_br_stock_account: displays tax document information outside of tabs #3528

Open
wants to merge 1 commit into
base: 14.0
Choose a base branch
from
Open

[14.0][IMP] l10n_br_stock_account: displays tax document information outside of tabs #3528

wants to merge 1 commit into from
+24 −10

Conversation

Tiago370
Copy link

@Tiago370 Tiago370 commented Dec 5, 2024 •
edited
Loading

Esse PR coloca os seguintes campos para fora da aba page_invoicing:

  • document_type_id
  • document_key
  • document_serie_id
  • document_serie
  • document_number
    image

Edit:
image

@OCA-git-bot
Copy link
Contributor

Hi @mbcosta, @renatonlima,
some modules you are maintaining are being modified, check this out!

@antoniospneto
Copy link
Contributor

antoniospneto commented Dec 5, 2024 •
edited
Loading

Acha que pra deixar mais organizado, essas informações poderia estar em um grupo com o titulo: Informações do documento Fiscal, pelo menos pra ter uma separação do que é informação da transferencia e o que é do documento fiscal.

@Tiago370 Tiago370 force-pushed the 14.0-imp-l10n_br_stock_account_form_document branch from 718775d to 5b34cca Compare December 5, 2024 12:38
@Tiago370
Copy link
Author

Tiago370 commented Dec 5, 2024

@antoniospneto corrigido.

rvalyi
rvalyi previously approved these changes Dec 7, 2024
View reviewed changes
Copy link
Member

@rvalyi rvalyi left a comment

Choose a reason for hiding this comment

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

LGTM. La na frente seria interessante esconder essas informações fiscais se a empresa não for do Brasil (caso multi empresas) mas pode ser feito num outo PR depois.

@antoniospneto
Copy link
Contributor

Eu acho que dá pra deixar a visão mais limpa, assim:

image

@rvalyi rvalyi dismissed their stale review December 7, 2024 14:39

Concordo com a observação do @antoniospneto

renatonlima
renatonlima requested changes Dec 7, 2024
View reviewed changes
l10n_br_stock_account/views/stock_picking.xml Outdated
<!-- Estes campos são read only momentaneamente, pode ser que em
um segundo momento essas informações também sejam relevantes para
pickings de entrada com relação com documentos fiscais ainda não
importados no sistema.
-->
<group name="document_info" string="Fiscal Document Info">
<group name="document_info" string="Fiscal Document Info" colspan="2">
Copy link
Member

Choose a reason for hiding this comment

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

Seria bom deixar invisível esse grupo caso não seja uma movimentação que não gere um documento fiscal

@Tiago370 Tiago370 force-pushed the 14.0-imp-l10n_br_stock_account_form_document branch from 5b34cca to 50d5d72 Compare December 9, 2024 11:50
antoniospneto
antoniospneto requested changes Dec 9, 2024
View reviewed changes
Copy link
Contributor

@antoniospneto antoniospneto left a comment •
edited
Loading

Choose a reason for hiding this comment

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

image

Ta ficando legal agora, mas veja que a serie tá repetindo, o campo document_serie não precisa ficar visivel, pois já tem o document_serie_id, se ele é necessário pode deixar, mas invisivel.

Falta implementar a sugestão do @renatonlima tbm.

@Tiago370 Tiago370 force-pushed the 14.0-imp-l10n_br_stock_account_form_document branch 2 times, most recently from 9290584 to 8026e0a Compare December 9, 2024 16:22
@Tiago370 Tiago370 force-pushed the 14.0-imp-l10n_br_stock_account_form_document branch from 8026e0a to 1c6e273 Compare December 9, 2024 17:20
@Tiago370
Copy link
Author

Tiago370 commented Dec 9, 2024

@renatonlima , corrigido.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antoniospneto antoniospneto antoniospneto requested changes

@rvalyi rvalyi rvalyi left review comments

@renatonlima renatonlima Awaiting requested review from renatonlima

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Tiago370 @OCA-git-bot @antoniospneto @rvalyi @renatonlima