-
Notifications
You must be signed in to change notification settings - Fork 195
Tipagem do AssinaturaA1.assinar() e melhorias para a assinatura #405
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
Tipagem do AssinaturaA1.assinar() e melhorias para a assinatura #405
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
tests/test_certificadoA1.py:28
- [nitpick] The variable name 'certificado_incorreto' could be made more explicit (e.g., 'invalid_certificado_path') to improve clarity.
with self.assertRaises(FileNotFoundError) as context:
tests/test_nfse_serializacao.py
Outdated
xml_assinado = xml_assinado.replace("\n", "") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using string.replace() to remove newlines may unintentionally alter XML content. Consider serializing the XML with lxml.etree.tostring (with pretty_print disabled) to preserve valid formatting.
xml_assinado = xml_assinado.replace("\n", "") | |
xml_assinado = etree.tostring(etree.fromstring(xml_assinado), pretty_print=False, encoding="unicode") |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refatorado.
except (PermissionError, FileNotFoundError) as exc: | ||
raise Exception( | ||
except FileNotFoundError as exc: | ||
raise FileNotFoundError( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The multi-line error message may include unintended whitespace or formatting. Consider formatting or trimming the error string to improve clarity in error logs.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refatorado.
Acrescentei essa modificação: 072bc52 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces type annotations for the AssinaturaA1.assinar() method, refines error handling when reading certificate files, and updates unit tests to validate the digital signature process for NFSe.
- Added type hints for the signature method in pynfe/processamento/assinatura.py
- Updated error messages and exception types in certificate file handling
- Modified NFSe unit tests to use a static timestamp and include the new Signature XML block
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tests/test_nfse_serializacao_ginfes.py | Updated expected XML output and adapted test helpers for signing |
tests/test_nfse_serializacao_betha.py | Adjusted expected XML and function signatures for consistency |
tests/test_nfse_serializacao.py | Added type hinting and reworked signature generation in tests |
tests/test_certificadoA1.py | Refined exception type and error message in certificate tests |
pynfe/processamento/assinatura.py | Added type annotations for the assinar() method |
pynfe/entidades/certificado.py | Separated exception handling for FileNotFoundError and PermissionError |
.github/workflows/ci.yml | Updated CI configuration to use the latest Ubuntu image |
Neste PR:
AssinaturaA1.assinar()
;pfx
;Closes #340 and #408.