Add the ability to set the segment in the PublishedRouter

[drpeck]

As things stand
The PublishedRouter handles creation of the VariationContext. It sets the Culture property with a value supplied by the IPublishedRequestBuilder. No segment is set.

The IPublishedRequestBuilder has a SetCulture (link), the value of which is used for setting the culture on the VariationContext (link). This method might be called by a content finder for example. There is no such comparable SetSegment() method.

Proposal
Would you support a PR that adds a SetSegment() to the IPublishedRequestBuilder, and also modifies PublishedRouter to use this set value when creating the new VariationContext()?

AFAIK the best current approach to set the Segment property on the VariationContext is by using a RoutingRequestNotification. In my case I'm identifying the segment in a IContentFinder (based on the URL) and then having to calculate the same value again later in a RoutingRequestNotification to set the VariationContent. This is because the VariationContext is overwritten after the content finders have run. This double handling could be avoided with this approach.

[drpeck]

This commit should be the totality of the change:
drpeck@5d8a271

[lassefredslund]

Hi @drpeck
Thanks for the idea and the PR.
To me, this sounds OK, however, I know that @PerplexDaniel uses segments a lot, so maybe he could verify it too?

[PerplexDaniel]

@drpeck You might be right they do not consider new members a breaking change, but I would think if I currently have some custom implementation of IPublishedRequestBuilder in my project (not sure why, but let's imagine) and then upgrade Umbraco which includes your commit then suddenly my code will not compile anymore since I have not specified the string? Segment property in my class. So build should fail with an exception like MyCustomPublishedRequestBuilder does not implement interface member IPublishedRequestBuilder.Segment (and also for the SetSegment method).

[nul800sebastiaan]

Ah sorry.. that was a new member on the interface, yeah that's hard to avoid. In the past we've then made a IPublishedRequestBuilder2 that inherits from IPublishedRequestBuilder and used that. It's ugly but it works.

[drpeck]

Fair point(s).

I can absolutely create IPublishedRequestBuilder2 but that's going to create some technical debt. Might this be better pushed to v12 as it's not causing anyone any issues right now? @PerplexDaniel 's workaround solves my issue for V11.

[nul800sebastiaan]

I agree, best to target v12 as there's a great workaround for now! 👍 (v12/dev branch)

[drpeck]

PR submitted but checks are proving to be an issue at the time of writing for some reason.