Skip to content

Delivery API: Only add default strategy if delivery API is not registered.#20982

Merged
kjac merged 3 commits intorelease/17.0from
17/hotfix/fix-expansionstrategy-regression
Nov 28, 2025
Merged

Delivery API: Only add default strategy if delivery API is not registered.#20982
kjac merged 3 commits intorelease/17.0from
17/hotfix/fix-expansionstrategy-regression

Conversation

@Zeegaan
Copy link
Member

@Zeegaan Zeegaan commented Nov 28, 2025

Fixes #20976

Notes

  • As noted in the issue, this was caused by attempting to fix Webhooks while delivery api was not registered.
  • It was incorrectly assumed that Delivery API would always be registered after webhooks, and thus the assumption that it would always overwrite the webhook expansion strategy.
  • This PR remedies that by ONLY adding the webhook expansion if the delivery API is NOT present, this is fine, as if Delivery API is registered after webhooks, it will overwrite, and if not, we will just stick with the delivery api expansion strategy 😁
  • I also tested this in 16.3.1 just to ensure the behavior is the same.

How to test

  • Enable delivery api by adding .AddDeliveryApi(), make sure you both test when its registered before AND after .AddComposers()
  • Also enable delivery api in the settings with:
     "DeliveryApi": {
        "Enabled": true
      },
  • Create a simple document type with a content picker(picker) & text string propery (title), with allow as root
  • Create 3 documents, I will name them Root, AnotherRoot & ThirdRoot, with titles filled out
  • From the root document, pick the AnotherRoot doc
  • From AnotherRoot, pick ThirdRoot
  • Now it's time to call the API! Using whatever software you prefer, call https://localhost:44339/umbraco/delivery/api/v2/content/item/0a75bce4-4ea2-40ec-a38e-0716adc07726
  • Assert that you see both properties
  • Try and filter on both property, example query: https://localhost:44339/umbraco/delivery/api/v2/content/item/0a75bce4-4ea2-40ec-a38e-0716adc07726?fields=properties[title]
  • Assert that you only see the property you filter
  • Lastly, try to expand the picker, so we can see the properties : https://localhost:44339/umbraco/delivery/api/v2/content/item/0a75bce4-4ea2-40ec-a38e-0716adc07726?expand=properties[picker[properties[$all]]]
  • Assert that you now also see the thirdroot node.

Copilot AI review requested due to automatic review settings November 28, 2025 02:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an issue where webhooks and the Delivery API conflicted when registering output expansion strategy services, depending on their registration order. The fix ensures that webhook registrations only occur if the Delivery API hasn't already registered them, regardless of which is registered first.

Key Changes

  • Changed from AddScoped/AddSingleton to TryAddScoped/TryAddSingleton for output expansion strategy registrations
  • Updated the using statement to include Microsoft.Extensions.DependencyInjection.Extensions to access the TryAdd* methods
  • Updated comments to clarify the registration behavior

Zeegaan and others added 2 commits November 28, 2025 11:47
…lderExtensions.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

@kjac kjac left a comment

Choose a reason for hiding this comment

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

💯

@kjac kjac merged commit e6b9993 into release/17.0 Nov 28, 2025
22 checks passed
@kjac kjac deleted the 17/hotfix/fix-expansionstrategy-regression branch November 28, 2025 07:25
AndyButland pushed a commit that referenced this pull request Dec 3, 2025
…ered. (#20982)

* Only add if not already present

* Update src/Umbraco.Cms.Api.Management/DependencyInjection/WebhooksBuilderExtensions.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Kenn Jacobsen <kja@umbraco.dk>
AndyButland pushed a commit that referenced this pull request Dec 3, 2025
…ered. (#20982)

* Only add if not already present

* Update src/Umbraco.Cms.Api.Management/DependencyInjection/WebhooksBuilderExtensions.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Kenn Jacobsen <kja@umbraco.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants