Repository navigation
feat(invoice)!: preserve ZATCA model fields - #53
Conversation
Preserve optional fields and accepted monetary values across Rust, XML and every shipped binding. Include matching SDK fixtures to check serialization changes. BREAKING CHANGE: credit/debit variants no longer embed references; party addresses are optional and identifiers/instructions are lists.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Reject negative imported discounts and missing required line VAT rates. Preserve omitted out-of-scope adjustment rates and repeated VAT classifications in percentage adjustments. Expose province in Python. Add constructor, source-XML and FFI ownership regressions. Keep the coverage threshold unchanged.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b991dfd4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let utc = time.ends_with('Z'); | ||
| let time = NaiveTime::parse_from_str(time.strip_suffix('Z').unwrap_or(time), "%H:%M:%S") | ||
| .map_err(|e| ParseError::InvalidValue { |
There was a problem hiding this comment.
When an invoice has the valid boundary time 24:00:00 or 24:00:00Z, this call rejects it because Chrono's NaiveTime only accepts hours through 23. The same change explicitly accepts these values in InvoiceTime::parse, so billing-reference times round-trip while the root cbc:IssueTime makes parse_finalized_invoice_xml fail; normalize midnight to the following day's 00:00:00 before constructing the UTC timestamp while retaining the original issue fields.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa6ac0d242
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self | ||
| } | ||
| pub fn payment_means(mut self, value: super::PaymentMeans) -> Self { | ||
| self.payment_means.push(value); |
There was a problem hiding this comment.
When payment_instruction() is called before this method, it creates a first PaymentMeans entry with an empty code; appending a valid entry here leaves that placeholder in the collection, and build() rejects it as an empty PaymentMeansCode. This affects the new bindings directly because InvoiceBuilder::new_with_reference calls payment_instruction() internally, so following it with the new add_payment_means() API cannot produce an invoice unless callers also know to invoke the legacy set_payment_means_code().
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94e76a2e90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| b.set_vat_rate(item.vat_rate) | ||
| b.set_vat_category(item.vat_category) | ||
| b.set_line_net_amount(item.total_amount) | ||
| b.set_line_vat_amount(item.vat_amount) |
There was a problem hiding this comment.
When item comes from invoice.line_item() for imported XML that omits an out-of-scope line's cbc:Percent, its line cac:TaxTotal, or its cbc:RoundingAmount, these unconditional setters reconstruct the item as though all three fields were present. The Python InvoiceLineItem value does not expose the corresponding omission flags, so builder.add_item(parsed.line_item(0)) silently inserts fields that the parser otherwise promises to preserve; carry those presence bits through the facade and only invoke the setters when the source field was present.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a0a098b63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| char **out, | ||
| FatooraError **error); | ||
| void fatoora_invoice_totals_free(FatooraInvoiceTotals *self); | ||
| FatooraStatus fatoora_line_item_builder_set_tax_total_present(FatooraLineItemBuilder *self, |
There was a problem hiding this comment.
The public C facade declares every setter and build operation for FatooraLineItemBuilder, but neither fatoora.h nor fatoora.c exposes a function that creates one, and no other facade function returns this handle. Consequently C callers cannot use any of the new line builder APIs needed for adjustments, base quantities, supplied totals, or omission controls; add a facade constructor wrapping fatoora_LineItemBuilder_new.
Useful? React with 👍 / 👎.
| #[serde(default)] | ||
| omitted_tax_amount: bool, | ||
| #[serde(default)] | ||
| omitted_gross_amount: bool, | ||
| #[serde(default)] | ||
| omitted_vat_rate: bool, |
There was a problem hiding this comment.
Because LineItem publicly derives Deserialize, callers can deserialize a line with omitted_tax_amount: true while retaining a nonzero vat_amount or tax subtotals, or set omitted_vat_rate for a standard-rated line, and then pass it to InvoiceBuilder::line_item. InvoiceBuilder::build does not revalidate these new flags, so serialization can omit required tax/rate elements and produce XML that this parser rejects; enforce the same omission invariants applied by LineItemBuilder::build when finalizing arbitrary LineItem values.
Useful? React with 👍 / 👎.
Description
Invoice import could drop buyer details, delivery dates and VAT exemption metadata. Construction also lacked line adjustments, price bases and prepayment references.
Add those fields across the Rust model and shipped bindings. Accepted supplied monetary amounts drive serialization and downstream totals; inconsistent totals return errors. An optional line builder keeps ordinary line construction unchanged.
Breaking API changes include credit/debit invoice variants, optional party addresses and repeated identifiers/instructions. The implementation contract records replacement APIs and supported XML limits. Unsupported UBL content can still be dropped during reconstruction.
Testing
Live ZATCA acceptance was not tested. Rebased onto the signed-XML hash fix in #52 and retained its regressions.
Checklist