"Send for Approval" on segments not working

[cornehoskam]

Which Umbraco version are you using?

13.9.3

Bug summary

Ever sinds the introduction of the "Send for Approval" option, segments have never been supported. The Core includes a hard "NotSupportedException" when you do try to send a segment to approval.

See:

Umbraco-CMS/src/Umbraco.Core/Services/NotificationService.cs

Line 458 in ae44f30

throw new NotSupportedException();

Now that segments are a first-class citizen again, we should really be picking this up.

Specifics

In the days of uMarketingSuite, we had a workaround for V8 available, that removed the else statement that that included the throw new NotSupportedException() exception in favor of gracefully handling it instead.

We should get around to further refining how we expect this to behave, and implement it. For now this Issue report is to serve as a note that this is still an issue.

Steps to reproduce

-Remove Publish Permissions from User
-Give user Send to Publish permissions.
-Go to a Document that has a segment, and open said segment.
-Click on "Send to Approval"
The NotYetSupported() exception will now be thrown.

Expected result / actual result

To be determined 😇

---

This item has been added to our backlog AB#56370

[kjac]

Hi @cornehoskam

Thanks for reporting this 💪 I do agree that this is quite ... insufficient.

Since segments aren't managed, I'm rather inclined to ignore them in the NotificationService and let it (continue to) be all about cultures. Of course, the exception should not be thrown, so I'd suggest a handling something along these lines:

if (content.ContentType.VariesByCulture()) {
  // handle as culture variant (same handling as today)
}
else {
  // handle as culture invariant (same handling as today)
}

Does that make sense to you?

[cornehoskam]

Hi @kjac,

Unfortunately I think that is the only real option that we have right now, seeing as we don't have any segment data available here. This is also how we have done it in the patched version of the NotificationService that we have previously provided. So yes, I think it makes sense to atleast gracefully handle it like that!

[AndyButland]

Fixed via #20160 for 13.11 and 16.3.