771 composite order mapper needs to remove invalid characters from po numbers

[ealexch]

Purpose

This PR addresses the issue where PO numbers with invalid characters were not being sanitized properly, potentially causing issues during migration. Fixes #771. The solution involves sanitizing PO numbers by removing invalid characters and logging any potential duplicates after sanitization.

Changes Made in this PR

• Implemented a sanitize_po_number method to remove invalid characters from PO numbers.
• Added logic to detect duplicates after sanitization and log them using Helper.log_data_issue.
• Used a class-level set to track existing PO numbers globally across the migration process.

Code Review Specifics

• Read the code and suggest improvements, if necessary.
• Ensure PO numbers with invalid characters are correctly sanitized, and duplicates are logged appropriately.

Warning Checklist

• New dependencies added.
• Includes breaking changes.

How to Verify

1. Run the order migrator with sample data containing PO numbers with invalid characters.
2. Verify that PO numbers are sanitized correctly and that duplicates are logged as data issues.

Open Questions

None at the moment.

Learn Anything Cool?

I learned about handling data sanitization in migration tools and ensuring data integrity by detecting and logging potential duplicates.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.22%. Comparing base (07b79a9) to head (c3c5f5c).
Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #825      +/-   ##
==========================================
+ Coverage   72.19%   72.22%   +0.02%     
==========================================
  Files         104      104              
  Lines       11521    11532      +11     
  Branches     1387     1388       +1     
==========================================
+ Hits         8318     8329      +11     
  Misses       2904     2904              
  Partials      299      299              

Flag	Coverage Δ
unittests	72.22% <100.00%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[bltravis]

@ealexch See comment regarding implementing the sanitize method as a static method, and please add a test to cover the new method.

[bltravis]

Is there a reason you didn't implement this as a static method? It doesn't need access to self and it would make it easier to write a test to cover the new code.

[mtrineyev]

The fix was completely redone and the new additional function requires access to self. Additionally, a corresponding test case was added.

[mtrineyev]

Because Alex can't work with this PR anymore, I take it.
Sanitization has been removed and instead implemented throwing an error TransformationRecordFailedError also improved logging for cases when the PO number contains invalid characters.
Unfortunately, I cannot push the commit because I do not have permission to do so.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[bltravis]

Ok. These changes look good. I'd probably have gone with defining the pattern as a global variable and then stuck with the validate po number method as a static method, but it's not critical.