Skip to content

Conversation

svillegas-cdd
Copy link
Contributor

@svillegas-cdd svillegas-cdd commented Oct 6, 2025

  • Updated tasa_otro_impuesto data type to Decimal for improved precision.
  • Removed redundant validator for rounding tasa_otro_impuesto.
  • Adjusted parsers and tests for the new data type.

Ref: https://app.shortcut.com/cordada/story/16788

@svillegas-cdd svillegas-cdd requested review from jtrobles-cdd, a team and Copilot October 6, 2025 20:35
@svillegas-cdd svillegas-cdd self-assigned this Oct 6, 2025
@svillegas-cdd svillegas-cdd changed the title fix(rcv): Add validations for tasa_otro_impuesto rcv: Add validations for tasa_otro_impuesto Oct 6, 2025
Copy link

@Copilot Copilot AI left a 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 adds validation for the tasa_otro_impuesto field to round values to 6 decimal places and updates corresponding test data.

  • Implemented a field validator to round tasa_otro_impuesto to 6 decimal places when it exceeds this precision
  • Updated test data to use a value with more than 6 decimal places to test the validation
  • Modified test expectations to reflect the rounded values

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/cl_sii/rcv/data_models.py Added field validator to round tasa_otro_impuesto to 6 decimal places
src/tests/test_rcv_parse_csv.py Updated test expectations to use rounded values
src/tests/test_data/sii-rcv/RCV-compra-reclamado.csv Added test data with more than 6 decimal places

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@svillegas-cdd svillegas-cdd force-pushed the task/sc-16788-rcv-processing-errors-decimal-places-tasa-otro-impuesto branch from efb4453 to f7198fb Compare October 6, 2025 20:50
@svillegas-cdd svillegas-cdd marked this pull request as ready for review October 6, 2025 20:50
@jtrobles-cdd jtrobles-cdd added bug Something isn't working component: rcv labels Oct 6, 2025
@svillegas-cdd svillegas-cdd force-pushed the task/sc-16788-rcv-processing-errors-decimal-places-tasa-otro-impuesto branch from f7198fb to 2d235c1 Compare October 7, 2025 17:23
@svillegas-cdd svillegas-cdd changed the title rcv: Add validations for tasa_otro_impuesto rcv: Replace float with Decimal for tasa_otro_impuesto Oct 7, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@svillegas-cdd
Copy link
Contributor Author

@jtrobles-cdd I haven't found a solution for the build errors, do you have an idea?

@jtrobles-cdd
Copy link
Member

I haven't found a solution for the build errors, do you have an idea?

I merged a pair of PRs so that the GitHub Actions cache gets updated. I don't know if that'll help, but try rebasing onto develop and force-pushing the PR's branch.

- Updated `tasa_otro_impuesto` data type to `Decimal` for improved precision.
- Removed redundant validator for rounding `tasa_otro_impuesto`.
- Adjusted parsers and tests for the new data type.

Ref: https://app.shortcut.com/cordada/story/16788/
@svillegas-cdd svillegas-cdd force-pushed the task/sc-16788-rcv-processing-errors-decimal-places-tasa-otro-impuesto branch from 2d235c1 to 629383c Compare October 8, 2025 13:59
Copy link

sonarqubecloud bot commented Oct 8, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
30.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Copy link

codecov bot commented Oct 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.40%. Comparing base (b78de7b) to head (629383c).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #909   +/-   ##
========================================
  Coverage    89.39%   89.40%           
========================================
  Files           40       40           
  Lines         3726     3728    +2     
  Branches       378      378           
========================================
+ Hits          3331     3333    +2     
  Misses         243      243           
  Partials       152      152           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@svillegas-cdd svillegas-cdd merged commit fb70796 into develop Oct 8, 2025
20 of 21 checks passed
@svillegas-cdd svillegas-cdd deleted the task/sc-16788-rcv-processing-errors-decimal-places-tasa-otro-impuesto branch October 8, 2025 15:20
@svillegas-cdd svillegas-cdd mentioned this pull request Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working component: rcv
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants