Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: extend SDK with content source maps support [TOL-2043] #2209

Merged
merged 13 commits into from
Apr 29, 2024
Merged

feat: extend SDK with content source maps support [TOL-2043] #2209

merged 13 commits into from
Apr 29, 2024

Conversation

2wce
Copy link
Contributor

@2wce 2wce commented Apr 25, 2024 •
edited
Loading

Summary

Content source maps will change the sys object in the entity for the response and this has been implemented as follows

  • When creating the Client for the SDK, we have added an option to include alpha features where csm can be enabled/disabled at client level.
  • We have added a wrapper function which will act like middleware of sorts & check if the alpha feature is enabled before adding the includeContentSourceMaps param to the query to enable CSM
  • We have added tests to ensure that the alpha feature can be correctly enabled
  • we have also added tests to show that adding Content Source Maps won't break any existing logic for fetching assets & entries including the current supported chain modifers

Motivation and Context

This has been added as part of this CEP and this ticket in order to let users fetch Content Source Maps with the REST API

@2wce 2wce mentioned this pull request Apr 25, 2024
Closed
2 tasks
@2wce 2wce changed the title feat: experiment/content source maps [TOL-2043] feat: extend SDK with content source maps support [TOL-2043] Apr 25, 2024
@2wce 2wce marked this pull request as ready for review April 25, 2024 11:33
@2wce 2wce requested a review from a team as a code owner April 25, 2024 11:33
@2wce 2wce requested review from chrishelgert and z0al April 25, 2024 11:33
@2wce 2wce self-assigned this Apr 25, 2024
z0al
z0al reviewed Apr 25, 2024
View reviewed changes
Copy link
Member

@z0al z0al left a comment

Choose a reason for hiding this comment

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

Looks great overall. Just a few pieces to iron out.

lib/contentful.ts Show resolved Hide resolved
lib/create-contentful-api.ts Show resolved Hide resolved
test/integration/getEntries.test.ts Show resolved Hide resolved
test/integration/getEntries.test.ts Outdated Show resolved Hide resolved
test/unit/contentful.test.ts Show resolved Hide resolved
@2wce 2wce requested a review from z0al April 26, 2024 08:59
@2wce 2wce merged commit eb2cebd into master Apr 29, 2024
3 checks passed
@2wce 2wce deleted the experiment/content-source-maps branch April 29, 2024 06:57
@contentful-automation
Copy link
Contributor

🎉 This PR is included in version 10.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@nishanthmahasamudram nishanthmahasamudram mentioned this pull request May 23, 2024
Open
@brightonsbox brightonsbox mentioned this pull request May 31, 2024
Closed
@joezilla joezilla mentioned this pull request Jun 7, 2024
Open
@andrewrlee andrewrlee mentioned this pull request Jun 7, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jjolton-contentful jjolton-contentful jjolton-contentful approved these changes

@chrishelgert chrishelgert Awaiting requested review from chrishelgert

@z0al z0al Awaiting requested review from z0al

@colomolo colomolo Awaiting requested review from colomolo

@aodhagan-cf aodhagan-cf Awaiting requested review from aodhagan-cf

Assignees

@2wce 2wce

Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@2wce @z0al @aodhagan-cf @jjolton-contentful @marcolink