Skip to content

fix: /api/v1/environment-document N+1 query issue related to the number of segment overrides in a version#6865

Merged
matthewelwell merged 3 commits intomainfrom
fix/environment-document-v2-versioning-n-plus-1-for-new-versions
Mar 6, 2026
Merged

fix: /api/v1/environment-document N+1 query issue related to the number of segment overrides in a version#6865
matthewelwell merged 3 commits intomainfrom
fix/environment-document-v2-versioning-n-plus-1-for-new-versions

Conversation

@matthewelwell
Copy link
Contributor

@matthewelwell matthewelwell commented Mar 6, 2026

Changes

Fixes an N+1 issue in the SDK environment document endpoint.

How did you test this code?

Updated an existing test case.

@matthewelwell matthewelwell requested a review from a team as a code owner March 6, 2026 17:22
@matthewelwell matthewelwell requested review from Zaimwa9 and removed request for a team March 6, 2026 17:22
@vercel
Copy link

vercel bot commented Mar 6, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

3 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs Ignored Ignored Preview Mar 6, 2026 5:42pm
flagsmith-frontend-preview Ignored Ignored Preview Mar 6, 2026 5:42pm
flagsmith-frontend-staging Ignored Ignored Preview Mar 6, 2026 5:42pm

Request Review

@github-actions github-actions bot added api Issue related to the REST API fix labels Mar 6, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026 •
edited
Loading

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-e2e:pr-6865 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-api-test:pr-6865 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-6865 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith:pr-6865 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith-api:pr-6865 Finished ✅ Results ✅
ghcr.io/flagsmith/flagsmith-private-cloud:pr-6865 Finished ✅ Results ✅

@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026 •
edited
Loading

Playwright Test Results (oss - depot-ubuntu-latest-16)

passed  10 passed

Details

stats  10 tests across 7 suites
duration  45.8 seconds
commit  e82c6f1
info  🔄 Run: #15142 (attempt 1)

@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026 •
edited
Loading

Playwright Test Results (oss - depot-ubuntu-latest-arm-16)

passed  10 passed

Details

stats  10 tests across 7 suites
duration  9.5 seconds
commit  e82c6f1
info  🔄 Run: #15142 (attempt 1)

emyller
emyller previously approved these changes Mar 6, 2026
View reviewed changes
@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026 •
edited
Loading

Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)

passed  16 passed

Details

stats  16 tests across 13 suites
duration  58.7 seconds
commit  e82c6f1
info  🔄 Run: #15142 (attempt 1)

@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026 •
edited
Loading

Playwright Test Results (private-cloud - depot-ubuntu-latest-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  44.3 seconds
commit  e82c6f1
info  🔄 Run: #15142 (attempt 1)

@matthewelwell matthewelwell changed the title fix: /api/v1/environment-document has an N+1 query issue proportional to the number of versions with segment overrides fix: /api/v1/environment-document has an N+1 query issue related to the number of segment overrides in a version Mar 6, 2026
@matthewelwell matthewelwell changed the title fix: /api/v1/environment-document has an N+1 query issue related to the number of segment overrides in a version fix: /api/v1/environment-document N+1 query issue related to the number of segment overrides in a version Mar 6, 2026
@matthewelwell matthewelwell requested a review from emyller March 6, 2026 17:43
@github-actions github-actions bot added fix and removed fix labels Mar 6, 2026
@codecov
Copy link

codecov bot commented Mar 6, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.33%. Comparing base (8bc089f) to head (e82c6f1).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #6865   +/-   ##
=======================================
  Coverage   98.33%   98.33%           
=======================================
  Files        1336     1336           
  Lines       49629    49634    +5     
=======================================
+ Hits        48804    48809    +5     
  Misses        825      825

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

emyller
emyller approved these changes Mar 6, 2026
View reviewed changes
Copy link
Contributor

@emyller emyller left a comment

Choose a reason for hiding this comment

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

:shipit:

khvn26
khvn26 requested changes Mar 6, 2026
View reviewed changes
Copy link
Member

@khvn26 khvn26 left a comment

Choose a reason for hiding this comment

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

Given that the issue is not related to EFVs after all, we need a better test.

@matthewelwell
Copy link
Contributor Author

matthewelwell commented Mar 6, 2026 •
edited
Loading

Given that the issue is not related to EFVs after all, we need a better test.

The thing is, that it is related to versioning. The issue with this test was that, while it enabled versioning, it then created all of the feature states without caring about that versioning. So the test previously was not testing what we thought it was. By moving the enable_v2_versioning function call to after the set up of the data, we are testing what we expected to be testing.

Edit: I left the additional versions there as a bonus in case we ever do accidentally add an N+1 issue related to the number of versions.

khvn26
khvn26 approved these changes Mar 6, 2026
View reviewed changes
Copy link
Member

@khvn26 khvn26 left a comment

Choose a reason for hiding this comment

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

LGTM

@matthewelwell matthewelwell merged commit 486e240 into main Mar 6, 2026
33 checks passed
@matthewelwell matthewelwell deleted the fix/environment-document-v2-versioning-n-plus-1-for-new-versions branch March 6, 2026 17:58
@flagsmithdev flagsmithdev mentioned this pull request Mar 6, 2026
Merged
matthewelwell added a commit that referenced this pull request Mar 9, 2026
@matthewelwell
fix: /api/v1/environment-document N+1 query issue related to the nu…
9a677ee 
…mber of segment overrides in a version (#6865)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@emyller emyller emyller approved these changes

@khvn26 khvn26 khvn26 approved these changes

@Zaimwa9 Zaimwa9 Awaiting requested review from Zaimwa9 Zaimwa9 is a code owner automatically assigned from Flagsmith/flagsmith-back-end

Assignees

No one assigned

Labels

api Issue related to the REST API fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@matthewelwell @emyller @khvn26