Skip to content

fix: Update SDK version handling in API and documentation to treat null SDK values as SDK6 (legacy scenes)#797

Merged
AndresMorelos merged 8 commits intomasterfrom
feat/re-discovery-places-and-worlds-with-sdk-null-are-treated-as-v6
Feb 26, 2026
Merged

fix: Update SDK version handling in API and documentation to treat null SDK values as SDK6 (legacy scenes)#797
AndresMorelos merged 8 commits intomasterfrom
feat/re-discovery-places-and-worlds-with-sdk-null-are-treated-as-v6

Conversation

@AndresMorelos
Copy link
Contributor

No description provided.

@github-actions
Copy link

github-actions bot commented Feb 11, 2026

Pull Request Test Coverage Report for Build 22353561345

Details

  • 15 of 15 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.01%) to 89.871%

Totals Coverage Status
Change from base Build 22314029934: -0.01%
Covered Lines: 15390
Relevant Lines: 16877

💛 - Coveralls

Copy link
Contributor

@LautaroPetaccio LautaroPetaccio left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Great job!

package.json Outdated
Comment on lines +119 to +121
"populate:sdk:dev": "DOTENV_CONFIG_PATH=.env.development ts-node -r dotenv/config bin/populateSdk.ts",
"populate:sdk:stg": "DOTENV_CONFIG_PATH=.env.stg ts-node -r dotenv/config bin/populateSdk.ts",
"populate:sdk:prd": "DOTENV_CONFIG_PATH=.env.prd ts-node -r dotenv/config bin/populateSdk.ts",
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind removing the scripts from the package.json?

)
})

test(`should count places with SDK=6 filter with prefix matching and include null SDK values`, async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

WDYT about, instead of adding these test that unit test the queries, we add integration tests for the endpoints that use this filter? We're testing it in the destination integration tests, but we could have others for the places one.

Comment on lines +115 to +116
const contentUrl = `${worldsContentServerUrl}/contents/${entityId}`
const contentResponse = await fetch(contentUrl)
Copy link
Contributor

Choose a reason for hiding this comment

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

By fetching here the entity, we would check if the runtimeVersion is already there to save some time and not fetch the scene.json if it is. Themetadata.runtimeVersion could have the SDK version.

@AndresMorelos AndresMorelos merged commit f3ebe8e into master Feb 26, 2026
3 of 4 checks passed
@AndresMorelos AndresMorelos deleted the feat/re-discovery-places-and-worlds-with-sdk-null-are-treated-as-v6 branch February 26, 2026 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants