Fix Issue #215#225
Conversation
Signed-off-by: JeanCarlos Chavarria <jeancarlos.chavarria19@gmail.com>
There was a problem hiding this comment.
Pull request overview
Fixes Issue #215 by correcting how the emitter (“emisor”) phone number is validated/serialized so it appears in generated XML when it meets the intended digit-length constraints.
Changes:
- Update emisor phone validation from numeric range comparison to string-length checks (8–20 chars) across multiple XML generators.
- Add a PHPUnit assertion verifying
<Emisor><Telefono><NumTelefono>is present forgenXMLFe()output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
api/contrib/genXML/genXML.php |
Fixes the phone inclusion condition in several XML generator functions to use length-based validation. |
tests/api_contrib_genXML_FE.php |
Adds an assertion ensuring the emisor phone number is included in FE XML output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| if ($emisorCodPaisTel != '' && $emisorTel != '' && $emisorTel >= EMISORNUMEROTELMIN && $emisorTel <= EMISORNUMEROTELMAX) { | ||
| if ($emisorCodPaisTel != '' && $emisorTel != '' && strlen($emisorTel) >= EMISORNUMEROTELMIN && strlen($emisorTel) <= EMISORNUMEROTELMAX) { |
There was a problem hiding this comment.
The telefono inclusion check only validates length; it doesn’t trim or ensure the value is numeric. Per the v4.4 XSD, is xs:integer, so values like "2222-3333" or trailing spaces would now be emitted and cause schema validation failures. Consider normalizing (trim) and validating digits-only (e.g., ctype_digit after trim, or strip non-digits) before adding , and avoid calling strlen() twice by computing the length once.
| } | ||
|
|
||
| if ($emisorCodPaisTel != '' && $emisorTel != '' && $emisorTel >= EMISORNUMEROTELMIN && $emisorTel <= EMISORNUMEROTELMAX) { | ||
| if ($emisorCodPaisTel != '' && $emisorTel != '' && strlen($emisorTel) >= EMISORNUMEROTELMIN && strlen($emisorTel) <= EMISORNUMEROTELMAX) { |
There was a problem hiding this comment.
This PR changes the emisor telefono validation/serialization not only for genXMLFe(), but also for genXMLNC/ND/TE/Fec/Fee. There are PHPUnit tests for genXMLFe(), but no coverage for the other generators, so regressions in those document types won’t be caught. Please add at least a minimal test (or a data-driven test) asserting that is included when emisor_tel is 8–20 digits for each affected generator.
Bug Fix for issue #215