Skip to content

Publishing: Validate segment values on scheduled publish, publish with descendants and create/update-and-publish (closes #24558) - #24559

Merged
Zeegaan merged 3 commits into
v19/devfrom
v19/bugfix/24558-validate-segment-values-on-publish
Nov 14, 2026
Merged

Zeegaan merged 3 commits into
v19/devfrom
v19/bugfix/24558-validate-segment-values-on-publish

Conversation

@Zeegaan

@Zeegaan Zeegaan commented Nov 11, 2026

Copy link
Copy Markdown
Member

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes #24558

Fixes #24558

Description

A value in a non-default segment that fails the property's validation was published by a scheduled publish, for the descendants of Publish with descendants, and by create-and-publish / update-and-publish. The same invalid value in the default segment stopped the publish.

Cause

PropertyValidationService.IsPropertyDataValid only validated the default segment. It is the check the content service runs before publishing, so every path that relies on it alone let invalid segment values through. Publish and Save and publish were only protected because ContentPublishingService runs a separate, segment-aware validation first. Publish with descendants runs that validation on the page you publish from, which is why only that page was validated.

Change

IsPropertyDataValid now also reports a property as invalid when it holds an invalid value in a non-default segment of the cultures being published. It follows the existing rules:

  • Segment values in cultures that are not being published are ignored.
  • Mandatory is still only enforced on the default segment.
  • Culture-invariant properties are checked under the same conditions as their default-segment value.

Behaviour change

Anything that publishes through IContentService.Publish directly is now rejected with FailedPublishContentInvalid when a segment value is invalid.

Tests

  • Unit tests in PropertyValidationServiceTests: invalid and valid segment values for segment and culture+segment variation, and segment values of other cultures being ignored.
  • Integration tests in ContentPublishingServiceTests: scheduled publish and a branch descendant, for both variations.
  • The tests that expect a rejection fail without the change in PropertyValidationService.

Not covered by a test: the create-and-publish and update-and-publish endpoints. They go through the same check.

How to test

  1. Use a site with segments enabled and a segment available, for example a custom ISegmentService that returns a "VIP members" segment.
  2. Create a document type that is allowed at root, turn on "Allow segmentation", and add a Textstring property "Title" with "Vary by segment" turned on and the regex ^[A-Z].*$.
  3. Create content from it, enter "Valid" as Title, switch to VIP members, enter "Valid VIP" as Title, and click Save and publish.
  4. Schedule a publish a few minutes from now.
  5. Switch to VIP members, change Title to "invalid vip", and click Save.
  6. Wait until the scheduled time has passed, and reload the content.
  7. The published VIP members Title is still "Valid VIP", and the log has a "Failed to publish content" error for the content.

For Publish with descendants: give a child page an invalid VIP members value, and publish its parent with descendants including unpublished pages. The child is reported as invalid and is not published.

The issue is also labelled for 17 and 18. This PR only targets v19.

🤖 Generated with Claude Code

IsPropertyDataValid only validated the default segment, so an invalid
value in another segment was published by every path that relies on it
alone: scheduled publish, the descendants of a branch publish, and
create/update-and-publish.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings November 11, 2026 09:54
@claude

claude Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @Zeegaan's task in 1m 4s —— View job


PR Review

Target: origin/v19/dev · Based on commit: b5355c2f

Before this PR, IPropertyValidationService.IsPropertyDataValid only checked the default segment. With this change it also rejects invalid values in non-default segments of the cultures being published. That closes the gap where scheduled publish, the descendants of a branch publish, and create/update-and-publish could publish invalid segment values.

  • Other changes: Anything that calls IContentService.Publish / PerformScheduledPublish directly now gets FailedPublishContentInvalid when a non-default segment value fails validation (for example regex or editor validators). Before, the publish went through. This is the intended fix, and it is worth a line in the release notes.

Critical

None.

Important

None.

Suggestion

  • PropertyValidationServiceTests.cs:300: Add a unit test showing that a mandatory property with an empty value in a non-default segment still passes. The current behaviour is correct (ShouldValidateAsRequired skips non-default segments), but nothing tests it on this new path. (inline comment posted)

What I checked

  • Scope rules: HasInvalidSegmentValue decides whether to check a property the same way the existing default-segment checks do:
    • A culture-variant property is skipped for an invariant-only publish.
    • A culture-invariant property is skipped when !ImpactsAlsoInvariantProperties && content.Published, unless its editor can merge partial values.
    • CulturesBeingValidated is set to [impact.Culture] only for editors that can merge partial values.
  • Consistent with the API-level validation: ContentValidationServiceBase also validates every stored segment, so the scheduled/branch paths and Publish / Save and publish now reject the same values.
  • Empty segment values: DataValueEditor.Validate skips the regex check for null or whitespace values, and the mandatory check doesn't apply to non-default segments. So segment values that are empty or cleared don't block publishing.
  • Breaking changes: None. The new method is private, and no public signatures changed.
  • Tests: The unit tests cover segment and culture+segment variation, plus segment values in other cultures being ignored. The integration tests cover a branch descendant and scheduled publish. The inputs are built so the old code would publish and the new code fails.

Nice, focused fix with good test coverage. 👍


@claude claude Bot added the area/backend label Nov 11, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Validation skips invalid culture-specific block values in non-default segments when CultureImpact.All is used, allowing invalid content to pass and requiring a context-preservation fix with a regression test.

1 open issue
What changed in this PR

This PR addresses #24558 by extending shared publishing validation to non-default segment values, covering scheduled publishing, branch descendants, and create/update-and-publish flows.

Changes:

  • Validates segment overrides within the affected cultures while preserving default-segment mandatory rules.
  • Adds unit tests for valid and invalid segment values and culture filtering.
  • Adds integration tests for scheduled publishing and branch-descendant rejection.
File Description
tests/​Umbraco.Tests.UnitTests/​Umbraco.Infrastructure/​Services/​PropertyValidationServiceTests.cs Tests segment validation and exclusion of unaffected cultures.
tests/​Umbraco.Tests.Integration/​Umbraco.Infrastructure/​Services/​ContentPublishingServiceTests.Publish.cs Tests rejection of invalid segments during scheduled and descendant publishing.
src/​Umbraco.Core/​Services/​PropertyValidationService.cs Adds non-default segment checks to shared publishing validation.

Review effort: Balanced

@Zeegaan
Zeegaan requested a review from andr317c November 11, 2026 10:04

@andr317c andr317c left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tests good!

@sonarqubecloud

Copy link
Copy Markdown

@Zeegaan
Zeegaan merged commit c9f42f7 into v19/dev Nov 14, 2026
34 of 35 checks passed
@Zeegaan
Zeegaan deleted the v19/bugfix/24558-validate-segment-values-on-publish branch November 14, 2026 09:37
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.

4 participants