kontent-ai / delivery-sdk-php

Kontent.ai Delivery SDK for PHP

https://kontent.ai

MIT License

46 stars 15 forks source link

Extend InlineLinkedItemsResolverInterface.resolveInlineLinkedItems with linked items #106

Closed Simply007 closed 3 years ago

Simply007 commented 3 years ago

Motivation

Extend InlineLinkedItemsResolverInterface.resolveInlineLinkedItems with linked items.

Fixes #104

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

If manual testing is required, what are the steps?

codecov[bot] commented 3 years ago

Codecov Report

Merging #106 (542d9fb) into master (c69006f) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master     #106   +/-   ##
=========================================
  Coverage     98.13%   98.13%           
  Complexity      195      195           
=========================================
  Files            22       22           
  Lines           591      591           
=========================================
  Hits            580      580           
  Misses           11       11

Impacted Files	Coverage Δ
src/Kentico/Kontent/Delivery/DefaultMapper.php	`98.46% <100.00%> (ø)`
src/Kentico/Kontent/Delivery/ModelBinder.php	`99.15% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c69006f...542d9fb. Read the comment docs.

stevie-mccomb commented 3 years ago

@Simply007 I've made the same change in our wrapper of the SDK by redefining a number of private methods in our custom model binder and it seems to be working as intended without issues. We've used it to resolve inline linked items within components inside other components and it has resolved the issue for us.

Simply007 commented 3 years ago

and it seems to be working as intended without issues. We've used it to resolve inline linked items within components inside other components and it has resolved the issue for us.

Thanks for letting me know!

(EDIT - sorry @stevie-mccomb I have just skimmed your response and missed you already test out the exact changes) ~Were the changes you did in your wrapper similar to in #104?~

My plan is to finish this up and release a new version since the new milestone already contains a bunch of features.

Simply007 commented 3 years ago

Hello @stevie-mccomb,

I have released version 3.2.0-beta1, but I think by this enhancement I have done a breaking change.

Do you think #107 is OK to prevent this feature be a breaking change? I think there is no way to prevent it because even with #107, you still need to fix the signature of your resolver.