Skip to content

Conversation

kkolhe-twilio
Copy link
Contributor

@kkolhe-twilio kkolhe-twilio commented Aug 28, 2025

This pull request is for fixing Customer.io timestamp validation to properly handle high-precision fractional seconds in ISO 8601 date strings.

  • Updated the [isIsoDate] function to support fractional seconds with up to 9 digits instead of the previous 6-digit limit
  • Improved Regex Pattern: Changed (\.\d{1,6})? to (\.\d{1,9})? to match ISO 8601 standard that allows more than 6 fractional digits
Screen.Recording.2025-08-29.at.11.57.04.AM.mov

Testing improvements:

  • Exported [isIsoDate] function and added test coverage for fractional seconds validation from 1-9 digits along with edge cases including invalid dates with more than 9 fractional digits

Testing

Include any additional information about the testing you have completed to
ensure your changes behave as expected. For a speedy review, please check
any of the tasks you completed below during your testing.

  • Added unit tests for new functionality
  • Tested end-to-end using the local server
  • [If destination is already live] Tested for backward compatibility of destination. Note: New required fields are a breaking change.
  • [Segmenters] Tested in the staging environment
  • [Segmenters] [If applicable for this change] Tested for regression with Hadron.

Copy link

codecov bot commented Aug 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.63%. Comparing base (67a677e) to head (6de7e91).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3212   +/-   ##
=======================================
  Coverage   79.62%   79.63%           
=======================================
  Files        1177     1177           
  Lines       21703    21704    +1     
  Branches     4220     4221    +1     
=======================================
+ Hits        17282    17283    +1     
  Misses       3679     3679           
  Partials      742      742           

☔ 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.

@kkolhe-twilio kkolhe-twilio force-pushed the STRATCONN-4121/Customer.io_timestamp_fix branch from e95e92a to d9ca0cb Compare August 28, 2025 16:41
marinhero
marinhero previously approved these changes Aug 28, 2025
rvadera12
rvadera12 previously approved these changes Aug 28, 2025
Copy link
Contributor

@rvadera12 rvadera12 left a comment

Choose a reason for hiding this comment

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

Make sure the tests are passing, before this is ready for release

@kkolhe-twilio kkolhe-twilio dismissed stale reviews from rvadera12 and marinhero via 2abf845 August 28, 2025 20:54
…timestamp to Unix- updated snapshots"

This reverts commit 2abf845.
@joe-ayoub-segment
Copy link
Contributor

Hi @kkolhe-twilio does this change warrant an end to end test before we deploy? Great that the unit tests are passing, but it would be nice to see an end to end test on local using the Actions Tester or on Staging, if possible.

@kkolhe-twilio
Copy link
Contributor Author

kkolhe-twilio commented Aug 29, 2025

@joe-ayoub-segment I have tested it in staging and in local using actions-tester and it works as expected. I have also added a demo of it working in staging in the PR description.

@joe-ayoub-segment joe-ayoub-segment merged commit bf34bb3 into main Sep 2, 2025
14 checks passed
@joe-ayoub-segment joe-ayoub-segment deleted the STRATCONN-4121/Customer.io_timestamp_fix branch September 2, 2025 11:16
@joe-ayoub-segment
Copy link
Contributor

Hi @kkolhe-twilio PR deployed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants