Flagsmith / flagsmith

Open Source Feature Flagging and Remote Config Service. Host on-prem or use our hosted version at https://flagsmith.com/
https://flagsmith.com/
BSD 3-Clause "New" or "Revised" License
4.9k stars 375 forks source link

fix: Handle null cancellation dates #4589

Closed zachaysan closed 2 months ago

zachaysan commented 2 months ago

Changes

Trials mean we don't always get cancellation dates in the response from Chargebee. This fixes this.

How did you test this code?

Edit: There is now testing in place. The one path where current_term_end is set was covered by an existing test, so I've added one new one for the case where it is unset.

vercel[bot] commented 2 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

3 Skipped Deployments | Name | Status | Preview | Comments | Updated (UTC) | | :--- | :----- | :------ | :------- | :------ | | **docs** | ⬜️ Ignored ([Inspect](https://vercel.com/flagsmith/docs/7nsfesySnGwZLaTbgUHLHkLQAwyt)) | [Visit Preview](https://docs-git-fix-handlenullcancellationdates-flagsmith.vercel.app) | | Sep 6, 2024 1:58pm | | **flagsmith-frontend-preview** | ⬜️ Ignored ([Inspect](https://vercel.com/flagsmith/flagsmith-frontend-preview/4TawMXNNF8ceTWBvH6coHC2ZyQjj)) | [Visit Preview](https://flagsmith-frontend-preview-git-fix-handlenullc-658350-flagsmith.vercel.app) | | Sep 6, 2024 1:58pm | | **flagsmith-frontend-staging** | ⬜️ Ignored ([Inspect](https://vercel.com/flagsmith/flagsmith-frontend-staging/2iHhGHSgXxULq5wF7NCuX2cx4Za8)) | [Visit Preview](https://flagsmith-frontend-staging-git-fix-handlenullc-458249-flagsmith.vercel.app) | | Sep 6, 2024 1:58pm |
sentry-io[bot] commented 2 months ago

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: api/organisations/chargebee/webhook_handlers.py

Function Unhandled Issue
process_subscription TypeError: 'NoneType' object cannot be interpreted as an integer /ap...
Event Count: 3

Did you find this useful? React with a 👍 or 👎

github-actions[bot] commented 2 months ago

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith:pr-4589 Finished :white_check_mark: Results :white_check_mark:
ghcr.io/flagsmith/flagsmith-private-cloud:pr-4589 Finished :white_check_mark: Results :white_check_mark:
ghcr.io/flagsmith/flagsmith-api-test:pr-4589 Finished :white_check_mark: Skipped
ghcr.io/flagsmith/flagsmith-e2e:pr-4589 Finished :white_check_mark: Skipped
ghcr.io/flagsmith/flagsmith-api:pr-4589 Finished :white_check_mark: Results :white_check_mark:
ghcr.io/flagsmith/flagsmith-frontend:pr-4589 Finished :white_check_mark: Results :white_check_mark:
ghcr.io/flagsmith/flagsmith:pr-4589 Finished :white_check_mark: Results :white_check_mark:
ghcr.io/flagsmith/flagsmith-private-cloud:pr-4589 Finished :white_check_mark: Results :white_check_mark:
github-actions[bot] commented 2 months ago

Uffizzi Preview deployment-55888 was deleted.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.16%. Comparing base (5c02c1e) to head (2100670). Report is 14 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #4589 +/- ## ======================================== Coverage 97.15% 97.16% ======================================== Files 1156 1157 +1 Lines 39934 40039 +105 ======================================== + Hits 38797 38902 +105 Misses 1137 1137 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

matthewelwell commented 2 months ago

@zachaysan I'm not sure why testing is N/A here? Let's make sure that we add some test coverage please.

zachaysan commented 2 months ago

@zachaysan I'm not sure why testing is N/A here? Let's make sure that we add some test coverage please.

This is done now @matthewelwell