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.

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

Issue/383 #385

Merged
merged 4 commits into from
Oct 30, 2023
Merged

Issue/383 #385

merged 4 commits into from
Oct 30, 2023

Conversation

pokornyd
Copy link
Member

Motivation

Which issue does this fix? Fixes #383

  • updated models to match API response
  • GetSyncAsync method updated
  • added support for dynamic resolution (otherwise JObject is returned)
  • fixed tests
  • adjusted some method descriptions

Checklist

  • Code follows coding conventions held in this repo
  • Automated tests have been added
  • Tests are passing
  • Docs have been updated (if applicable)
  • Temporary settings (e.g. variables used during development and testing) have been reverted to defaults

How to test

Initialize sync in your sample project, perform some changes, then get the token from initialization and run GetSyncAsync both with and without TypeProvider registered.

@pokornyd pokornyd requested a review from a team as a code owner October 24, 2023 06:39
@JiriLojda JiriLojda self-requested a review October 24, 2023 12:44
JiriLojda
JiriLojda previously approved these changes Oct 24, 2023
@codecov
Copy link

codecov bot commented Oct 29, 2023

Codecov Report

Merging #385 (c726a1b) into master (d108042) will increase coverage by 0.01%.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #385      +/-   ##
==========================================
+ Coverage   88.55%   88.57%   +0.01%     
==========================================
  Files         128      129       +1     
  Lines        2761     2774      +13     
  Branches      345      345              
==========================================
+ Hits         2445     2457      +12     
  Misses        185      185              
- Partials      131      132       +1     
Files Coverage Δ
Kontent.Ai.Delivery.Caching/DeliveryClientCache.cs 78.66% <ø> (ø)
Kontent.Ai.Delivery/DeliveryClient.cs 82.06% <100.00%> (+0.49%) ⬆️
Kontent.Ai.Delivery/Sync/DeliverySyncResponse.cs 62.50% <ø> (ø)
Kontent.Ai.Delivery/Sync/SyncItem.cs 100.00% <100.00%> (ø)
Kontent.Ai.Delivery/Sync/SyncItemData.cs 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pokornyd
Copy link
Member Author

the model differed from the API response format, without mapping implemented to account for that. I adjusted the model to match sync API response, content item information is now nested in Data property.

a strongly typed item can be retrieved from newly introduced StronglyTypedData property, as long as a TypeProvider is registered and can found a suitable model. the property is null otherwise

@pokornyd pokornyd self-assigned this Oct 30, 2023
@pokornyd pokornyd merged commit 6086535 into master Oct 30, 2023
8 checks passed
@pokornyd pokornyd deleted the issue/383 branch October 30, 2023 10:12
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.

.NET SDK of Sync Api not correctly deserializing JSON
2 participants