From 914fc3fb527a8e5b1000f9454ce2a3999dfb6ef8 Mon Sep 17 00:00:00 2001 From: brandonkelly Date: Fri, 4 Sep 2026 06:31:06 -0700 Subject: [PATCH 1/5] 5.12 prep [ci skip] --- .github/workflows/ci.yml | 1 + CHANGELOG-5.12.md | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 CHANGELOG-5.12.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ef8e167717a..1fc8755f0cb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,6 +4,7 @@ on: push: branches: - 5.x + - '5.12' - '*-internal' pull_request: permissions: diff --git a/CHANGELOG-5.12.md b/CHANGELOG-5.12.md new file mode 100644 index 00000000000..853fba7f378 --- /dev/null +++ b/CHANGELOG-5.12.md @@ -0,0 +1,2 @@ +# Release Notes for Craft CMS 5.12 (WIP) + From ee306b2d2289e4f7b23119b7f08bda190c965894 Mon Sep 17 00:00:00 2001 From: brandonkelly Date: Fri, 4 Sep 2026 07:26:14 -0700 Subject: [PATCH 2/5] Language-aware case modification --- src/helpers/ElementHelper.php | 7 +-- src/helpers/StringHelper.php | 32 ++++++++--- src/i18n/Locale.php | 19 +++++-- src/web/twig/Extension.php | 55 +++++++++++++++++++ tests/unit/helpers/ElementHelperTest.php | 28 ++++++++++ tests/unit/helpers/StringHelperTest.php | 31 ++++++++--- tests/unit/i18n/LocaleTest.php | 47 ++++++++++++++++ tests/unit/web/twig/ExtensionTest.php | 69 ++++++++++++++++++++++++ 8 files changed, 268 insertions(+), 20 deletions(-) create mode 100644 tests/unit/i18n/LocaleTest.php diff --git a/src/helpers/ElementHelper.php b/src/helpers/ElementHelper.php index ee7091c1ac8..09435b219e4 100644 --- a/src/helpers/ElementHelper.php +++ b/src/helpers/ElementHelper.php @@ -108,17 +108,18 @@ public static function generateSlug(string $str, ?bool $ascii = null, ?string $l $slug = StringHelper::toAscii($slug, $language); } - return static::normalizeSlug($slug); + return static::normalizeSlug($slug, $language); } /** * Normalizes a slug. * * @param string $slug + * @param string|null $language The slug’s langauge * @return string * @since 3.5.0 */ - public static function normalizeSlug(string $slug): string + public static function normalizeSlug(string $slug, ?string $language = null): string { // Special case for the homepage if ($slug === Element::HOMEPAGE_URI) { @@ -134,7 +135,7 @@ public static function normalizeSlug(string $slug): string // Make it lowercase $generalConfig = Craft::$app->getConfig()->getGeneral(); if (!$generalConfig->allowUppercaseInSlug) { - $slug = mb_strtolower($slug); + $slug = StringHelper::toLowerCase($slug, $language); } // Get the "words". Split on anything that is not alphanumeric or allowed punctuation diff --git a/src/helpers/StringHelper.php b/src/helpers/StringHelper.php index 8fb013461ab..11be477b4ca 100644 --- a/src/helpers/StringHelper.php +++ b/src/helpers/StringHelper.php @@ -9,12 +9,14 @@ use BackedEnum; use Craft; +use craft\i18n\Locale; use HTMLPurifier_Config; use Illuminate\Support\Str; use IteratorAggregate; use LitEmoji\LitEmoji; use Normalizer; use Throwable; +use Transliterator; use voku\helper\ASCII; use yii\base\Exception; use yii\base\InvalidArgumentException; @@ -2313,11 +2315,12 @@ public static function toKebabCase(string $str, string $glue = '-', bool $lower * Converts all characters in the string to lowercase. An alias for PHP's mb_strtolower(). * * @param string $str The string to convert to lowercase. + * @param string|null $language The string’s langauge * @return string The lowercase string. */ - public static function toLowerCase(string $str): string + public static function toLowerCase(string $str, ?string $language = null): string { - return Str::lower($str); + return self::modifyCase($str, $language, 'Lower') ?? Str::lower($str); } /** @@ -2414,11 +2417,12 @@ public static function toTabs(string $str, int $tabLength = 4): string * Converts the first character of each word in the string to uppercase. * * @param string $str The string to convert case. + * @param string|null $language The string’s langauge * @return string The title-cased string. */ - public static function toTitleCase(string $str): string + public static function toTitleCase(string $str, ?string $language = null): string { - return Str::title($str); + return self::modifyCase($str, $language, 'Title') ?? Str::title($str); } /** @@ -2440,11 +2444,12 @@ public static function toTransliterate(string $str, bool $strict = false): strin * Converts all characters in the string to uppercase. An alias for PHP's mb_strtoupper(). * * @param string $str The string to convert to uppercase. + * @param string|null $language The string’s langauge * @return string The uppercase string. */ - public static function toUpperCase(string $str): string + public static function toUpperCase(string $str, ?string $language = null): string { - return Str::upper($str); + return self::modifyCase($str, $language, 'Upper') ?? Str::upper($str); } /** @@ -2803,4 +2808,19 @@ public static function invisibleCharsRegex(): string return sprintf('/%s/iu', implode('|', $invisibleCharCodes)); } + + private static function modifyCase(string $str, ?string $language, string $case): ?string + { + $language ??= Craft::$app->language; + $transliterator = Transliterator::create(sprintf('%s-%s', Locale::languageId($language), $case)); + + if (!$transliterator) { + return null; + } + + // Normalize NFD chars to NFC + $str = Normalizer::normalize($str, Normalizer::FORM_C); + + return $transliterator->transliterate($str); + } } diff --git a/src/i18n/Locale.php b/src/i18n/Locale.php index db9efd2b4fa..0af9cbdefa5 100644 --- a/src/i18n/Locale.php +++ b/src/i18n/Locale.php @@ -25,6 +25,19 @@ */ class Locale extends BaseObject { + /** + * Returns a locale’s language ID. + * + * @return string + * @since 5.12.0 + */ + public static function languageId(string $locale): string + { + $pos = strpos($locale, '-'); + $lang = $pos !== false ? substr($locale, 0, $pos) : $locale; + return strtolower($lang); + } + /** * @var int Positive prefix. */ @@ -296,11 +309,7 @@ public function __toString(): string #[AllowedInSandbox] public function getLanguageID(): string { - if (($pos = strpos($this->id, '-')) !== false) { - return substr($this->id, 0, $pos); - } - - return $this->id; + return static::languageId($this->id); } /** diff --git a/src/web/twig/Extension.php b/src/web/twig/Extension.php index bb32e26ee11..597dba81b80 100644 --- a/src/web/twig/Extension.php +++ b/src/web/twig/Extension.php @@ -230,6 +230,7 @@ public function getFilters(): array new TwigFilter('base64_encode', 'base64_encode'), new TwigFilter('boolean', 'boolval'), new TwigFilter('camel', [$this, 'camelFilter']), + new TwigFilter('capitalize', [$this, 'capitalizeFilter'], ['needs_charset' => true]), new TwigFilter('column', [$this, 'columnFilter'], ['needs_is_sandboxed' => true]), new TwigFilter('contains', [$this, 'containsFilter'], ['needs_is_sandboxed' => true]), new TwigFilter('currency', [$this, 'currencyFilter']), @@ -261,6 +262,7 @@ public function getFilters(): array new TwigFilter('length', [$this, 'lengthFilter'], ['needs_environment' => true]), new TwigFilter('lcfirst', [$this, 'lcfirstFilter']), new TwigFilter('literal', [$this, 'literalFilter']), + new TwigFilter('lower', [$this, 'lowerFilter']), new TwigFilter('map', [$this, 'mapFilter'], ['needs_environment' => true, 'needs_is_sandboxed' => true]), new TwigFilter('markdown', [$this, 'markdownFilter'], ['is_safe' => ['html']]), new TwigFilter('md', [$this, 'markdownFilter'], ['is_safe' => ['html']]), @@ -289,6 +291,7 @@ public function getFilters(): array new TwigFilter('string', 'strval'), new TwigFilter('time', [$this, 'timeFilter'], ['needs_environment' => true]), new TwigFilter('timestamp', [$this, 'timestampFilter']), + new TwigFilter('title', [$this, 'titleFilter']), new TwigFilter('translate', [$this, 'translateFilter']), new TwigFilter('truncate', [$this, 'truncateFilter']), new TwigFilter('t', [$this, 'translateFilter']), @@ -296,6 +299,7 @@ public function getFilters(): array new TwigFilter('ucwords', [$this, 'ucwordsFilter'], ['needs_environment' => true]), new TwigFilter('unique', 'array_unique'), new TwigFilter('unshift', [$this, 'unshiftFilter']), + new TwigFilter('upper', [$this, 'upperFilter']), new TwigFilter('values', 'array_values'), new TwigFilter('where', [$this, 'whereFilter'], ['needs_is_sandboxed' => true]), new TwigFilter('widont', [$this, 'widontFilter'], ['is_safe' => ['html']]), @@ -499,6 +503,18 @@ public function camelFilter(mixed $string): string return StringHelper::toCamelCase((string)$string); } + /** + * Capitalizes a string. + * + * @param string $charset + * @param string|null $string + * @param string|null $language + * @since 5.12.0 + */ + public function capitalizeFilter(string $charset, ?string $string, ?string $language = null): string + { + return StringHelper::toUpperCase(mb_substr($string ?? '', 0, 1, $charset), $language) . StringHelper::toLowerCase(mb_substr($string ?? '', 1, null, $charset), $language); + } /** * Throws a RuntimeError if the given name/key is a string containing a "." character and the environment is @@ -764,6 +780,19 @@ public function timestampFilter(mixed $value, ?string $format = null, bool $with } } + /** + * Title-cases a string + * + * @param string|null $string + * @param string|null $language + * @return string + * @since 5.12.0 + */ + public function titleFilter(?string $string, ?string $language = null): string + { + return StringHelper::toTitleCase($string ?? '', $language); + } + /** * This method will JSON encode a variable. We're overriding Twig's default implementation to set some stricter * encoding options on text/html/xml requests. @@ -982,6 +1011,19 @@ public function unshiftFilter(array $array): array return $array; } + /** + * Upper-cases a string + * + * @param string|null $string + * @param string|null $language + * @return string + * @since 5.12.0 + */ + public function upperFilter(?string $string, ?string $language = null): string + { + return StringHelper::toUpperCase($string ?? '', $language); + } + /** * Removes a class (or classes) from the given HTML tag. * @@ -1467,6 +1509,19 @@ public function literalFilter(mixed $value): string return Db::escapeParam((string)$value); } + /** + * Lower-cases a string + * + * @param string|null $string + * @param string|null $language + * @return string + * @since 5.12.0 + */ + public function lowerFilter(?string $string, ?string $language = null): string + { + return StringHelper::toLowerCase($string ?? '', $language); + } + /** * Parses text through Markdown. * diff --git a/tests/unit/helpers/ElementHelperTest.php b/tests/unit/helpers/ElementHelperTest.php index a99afaedf86..66214d9fbbd 100644 --- a/tests/unit/helpers/ElementHelperTest.php +++ b/tests/unit/helpers/ElementHelperTest.php @@ -69,6 +69,21 @@ public function testLowerRemoveFromCreateSlug(): void self::assertSame('word' . $general->slugWordSeparator . 'word', ElementHelper::normalizeSlug('word WORD')); } + /** + * @dataProvider normalizeSlugRespectsLanguageDataProvider + * @param string $expected + * @param string $slug + * @param string|null $language + */ + public function testNormalizeSlugRespectsLanguage(string $expected, string $slug, ?string $language): void + { + // The slug can only get lowercased if uppercase characters aren't allowed + $general = Craft::$app->getConfig()->getGeneral(); + $general->allowUppercaseInSlug = false; + + self::assertSame($expected, ElementHelper::normalizeSlug($slug, $language)); + } + /** * @dataProvider isTempSlugDataProvider * @param bool $expected @@ -219,6 +234,19 @@ public static function normalizeSlugDataProvider(): array ]; } + /** + * @return array + */ + public static function normalizeSlugRespectsLanguageDataProvider(): array + { + return [ + // Turkish lowercases a dotless "I" to a dotless "ı", not "i" + // (https://github.com/craftcms/cms/discussions/19555) + ['ıstanbul', 'Istanbul', 'tr'], + ['istanbul', 'Istanbul', null], + ]; + } + /** * @return array */ diff --git a/tests/unit/helpers/StringHelperTest.php b/tests/unit/helpers/StringHelperTest.php index 0672461a8f3..3888387825f 100644 --- a/tests/unit/helpers/StringHelperTest.php +++ b/tests/unit/helpers/StringHelperTest.php @@ -1354,10 +1354,11 @@ public function testToKebabCase(string $expected, string $string): void * @dataProvider toLowerCaseDataProvider * @param string $expected * @param string $string + * @param string|null $language */ - public function testToLowerCase(string $expected, string $string): void + public function testToLowerCase(string $expected, string $string, ?string $language = null): void { - $actual = StringHelper::toLowerCase($string); + $actual = StringHelper::toLowerCase($string, $language); self::assertSame($expected, $actual); } @@ -1423,10 +1424,11 @@ public function testToTabs(string $expected, string $string, int $tabLength = 4) * @dataProvider toTitleCaseDataProvider * @param string $expected * @param string $string + * @param string|null $language */ - public function testToTitleCase(string $expected, string $string): void + public function testToTitleCase(string $expected, string $string, ?string $language = null): void { - $actual = StringHelper::toTitleCase($string); + $actual = StringHelper::toTitleCase($string, $language); self::assertSame($expected, $actual); } @@ -1445,10 +1447,11 @@ public function testToTransliterate(string $expected, string $string): void * @dataProvider toUppercaseDataProvider * @param string $expected * @param string $string + * @param string|null $language */ - public function testToUppercase(string $expected, string $string): void + public function testToUppercase(string $expected, string $string, ?string $language = null): void { - $actual = StringHelper::toUpperCase($string); + $actual = StringHelper::toUpperCase($string, $language); self::assertSame($expected, $actual); } @@ -1657,6 +1660,10 @@ public static function toTitleCaseDataProvider(): array ['😘', '😘'], ['22 Alphan Numeric', '22 AlphaN Numeric'], ['!@#$% ^&*()', '!@#$% ^&*()'], + // Dutch "ij" is treated as a single letter, so title-casing it capitalizes both characters + // (https://github.com/craftcms/cms/discussions/19555) + ['IJsselmeer', 'ijsselmeer', 'nl'], + ['Ijsselmeer', 'ijsselmeer'], ]; } @@ -1676,6 +1683,10 @@ public static function toLowerCaseDataProvider(): array ['😘', '😘'], ['22 alphan numeric', '22 AlphaN Numeric'], ['!@#$% ^&*()', '!@#$% ^&*()'], + // Turkish lowercases a dotless "I" to a dotless "ı", not "i" + // (https://github.com/craftcms/cms/discussions/19555) + ['ıstanbul', 'Istanbul', 'tr'], + ['istanbul', 'Istanbul'], ]; } @@ -2249,6 +2260,14 @@ public static function toUppercaseDataProvider(): array ['😘', '😘'], ['22 ALPHAN NUMERIC', '22 AlphaN Numeric'], ['!@#$% ^&*()', '!@#$% ^&*()'], + // Turkish uppercases a dotted "i" to a dotted "İ", not "I" + // (https://github.com/craftcms/cms/discussions/19555) + ['İSTANBUL', 'istanbul', 'tr'], + ['ISTANBUL', 'istanbul'], + // Greek strips accents when uppercasing + // (https://github.com/craftcms/cms/discussions/19555) + ['ΑΝΘΡΩΠΟΣ', 'άνθρωπος', 'el'], + ['ΆΝΘΡΩΠΟΣ', 'άνθρωπος'], ]; } diff --git a/tests/unit/i18n/LocaleTest.php b/tests/unit/i18n/LocaleTest.php new file mode 100644 index 00000000000..da7e9ce684a --- /dev/null +++ b/tests/unit/i18n/LocaleTest.php @@ -0,0 +1,47 @@ + + * @since 5.12.0 + */ +class LocaleTest extends TestCase +{ + /** + * @param string $expected + * @param string $locale + * @dataProvider languageIdDataProvider + */ + public function testLanguageId(string $expected, string $locale): void + { + self::assertSame($expected, Locale::languageId($locale)); + } + + /** + * @return array[] + */ + public static function languageIdDataProvider(): array + { + return [ + ['en', 'en'], + ['en', 'EN'], + ['en', 'en-US'], + ['en', 'EN-US'], + ['zh', 'zh-Hans-CN'], + ['de', 'de-DE'], + ['', ''], + ['pt', 'pt-BR'], + ]; + } +} diff --git a/tests/unit/web/twig/ExtensionTest.php b/tests/unit/web/twig/ExtensionTest.php index 842c1597100..26769290f2c 100644 --- a/tests/unit/web/twig/ExtensionTest.php +++ b/tests/unit/web/twig/ExtensionTest.php @@ -372,6 +372,75 @@ public function testLcfirstFilter(): void ); } + /** + * `title`, `capitalize`, `upper`, and `lower` all accept an optional `language` argument + * that should be respected for language-specific casing rules + * (https://github.com/craftcms/cms/discussions/19555). + */ + public function testTitleFilter(): void + { + $this->testRenderResult( + 'Ijsselmeer', + '{{ "ijsselmeer"|title }}' + ); + + // Dutch title-cases the "ij" digraph as a single letter, capitalizing both characters + $this->testRenderResult( + 'IJsselmeer', + '{{ "ijsselmeer"|title("nl") }}' + ); + } + + /** + * @see testTitleFilter() + */ + public function testCapitalizeFilter(): void + { + $this->testRenderResult( + 'Istanbul', + '{{ "istanbul"|capitalize }}' + ); + + // Turkish uppercases a dotted "i" to a dotted "İ", not "I" + $this->testRenderResult( + 'İstanbul', + '{{ "istanbul"|capitalize("tr") }}' + ); + } + + /** + * @see testTitleFilter() + */ + public function testUpperFilter(): void + { + $this->testRenderResult( + 'ISTANBUL', + '{{ "istanbul"|upper }}' + ); + + $this->testRenderResult( + 'İSTANBUL', + '{{ "istanbul"|upper("tr") }}' + ); + } + + /** + * @see testTitleFilter() + */ + public function testLowerFilter(): void + { + $this->testRenderResult( + 'istanbul', + '{{ "Istanbul"|lower }}' + ); + + // Turkish lowercases a dotless "I" to a dotless "ı", not "i" + $this->testRenderResult( + 'ıstanbul', + '{{ "Istanbul"|lower("tr") }}' + ); + } + /** * */ From 931fdd737d55b01a9d02e9d0a94fe9c42f531c51 Mon Sep 17 00:00:00 2001 From: brandonkelly Date: Fri, 4 Sep 2026 07:36:24 -0700 Subject: [PATCH 3/5] Release notes for #19558 [ci skip] --- CHANGELOG-5.12.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG-5.12.md b/CHANGELOG-5.12.md index 853fba7f378..e2bf4cbc681 100644 --- a/CHANGELOG-5.12.md +++ b/CHANGELOG-5.12.md @@ -1,2 +1,11 @@ # Release Notes for Craft CMS 5.12 (WIP) +### Development + +- The `capitalize`, `lower`, `title`, and `upper` Twig filters now have `language` arguments, which default to the current application language. ([#19558](https://github.com/craftcms/cms/pull/19558)) + +### Extensibility + +- Added `craft\i18n\Locale::languageId()`. +- `craft\helpers\ElementHelper::normalizeSlug()` now has a `$language` argument, which defaults to the current application language. ([#19558](https://github.com/craftcms/cms/pull/19558)) +- `craft\helpers\StringHelper::toLowerCase()`, `::toTitleCase()`, and `::toUpperCase()` now have `$language` arguments, which default to the current application language. ([#19558](https://github.com/craftcms/cms/pull/19558)) From 7d31e213b4e598cfd90276d44a35119f2925308c Mon Sep 17 00:00:00 2001 From: brandonkelly Date: Sat, 5 Sep 2026 12:49:10 -0700 Subject: [PATCH 4/5] =?UTF-8?q?Fix=20=E2=80=9CField=20:=20is=20empty?= =?UTF-8?q?=E2=80=9D=20rules?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG-5.12.md | 6 ++ src/elements/db/AddressQuery.php | 7 ++- src/elements/db/EntryQuery.php | 10 +++ src/elements/db/NestedElementQueryTrait.php | 70 +++++++++++++++++---- 4 files changed, 80 insertions(+), 13 deletions(-) diff --git a/CHANGELOG-5.12.md b/CHANGELOG-5.12.md index e2bf4cbc681..2d3fa020cfb 100644 --- a/CHANGELOG-5.12.md +++ b/CHANGELOG-5.12.md @@ -7,5 +7,11 @@ ### Extensibility - Added `craft\i18n\Locale::languageId()`. +- Added `craft\elements\db\NestedElementQueryTrait::mustHaveField()`. +- Added `craft\elements\db\NestedElementQueryTrait::mustHaveOwner()`. - `craft\helpers\ElementHelper::normalizeSlug()` now has a `$language` argument, which defaults to the current application language. ([#19558](https://github.com/craftcms/cms/pull/19558)) - `craft\helpers\StringHelper::toLowerCase()`, `::toTitleCase()`, and `::toUpperCase()` now have `$language` arguments, which default to the current application language. ([#19558](https://github.com/craftcms/cms/pull/19558)) + +### System + +- Fixed a bug where entry and address indexes weren’t showing any results if they had a “Field” condition rule set to “is empty”. diff --git a/src/elements/db/AddressQuery.php b/src/elements/db/AddressQuery.php index 95f5db2c2c5..93659caa8c5 100644 --- a/src/elements/db/AddressQuery.php +++ b/src/elements/db/AddressQuery.php @@ -899,7 +899,7 @@ protected function beforePrepare(): bool $this->normalizeNestedElementParams(); // Only join the elements_owners table if fieldId is specified - if (!empty($this->fieldId)) { + if (isset($this->fieldId)) { $this->applyNestedElementParams('addresses.fieldId', 'addresses.primaryOwnerId'); } elseif (isset($this->primaryOwnerId) || isset($this->ownerId)) { // User addresses don't get rows in the elements_owners table @@ -968,6 +968,11 @@ protected function beforePrepare(): bool return true; } + protected function mustHaveField(): bool + { + return false; + } + /** * @inheritdoc */ diff --git a/src/elements/db/EntryQuery.php b/src/elements/db/EntryQuery.php index 3e0f3d7da34..86d84c5260c 100644 --- a/src/elements/db/EntryQuery.php +++ b/src/elements/db/EntryQuery.php @@ -968,6 +968,16 @@ protected function beforePrepare(): bool return true; } + protected function mustHaveField(): bool + { + return false; + } + + protected function mustHaveOwner(): bool + { + return false; + } + /** * @inheritdoc */ diff --git a/src/elements/db/NestedElementQueryTrait.php b/src/elements/db/NestedElementQueryTrait.php index 305001f6f9e..2189efecedf 100644 --- a/src/elements/db/NestedElementQueryTrait.php +++ b/src/elements/db/NestedElementQueryTrait.php @@ -197,16 +197,49 @@ public function allowOwnerRevisions(?bool $value = true): static return $this; } + /** + * Returns whether the resulting elements will always have a field assigned to them. + * + * @since 5.12.0 + */ + protected function mustHaveField(): bool + { + return true; + } + + /** + * Returns whether the resulting elements will always have an owner assigned to them. + * + * @since 5.12.0 + */ + protected function mustHaveOwner(): bool + { + return true; + } + private function applyNestedElementParams(string $fieldIdColumn, string $primaryOwnerIdColumn): void { $this->normalizeNestedElementParams(); - if ($this->fieldId === false || $this->primaryOwnerId === false || $this->ownerId === false) { + $mustHaveField = $this->mustHaveField(); + $mustHaveOwner = $this->mustHaveOwner(); + + if ( + ($mustHaveField && $this->fieldId === false) || + ($mustHaveOwner && ($this->primaryOwnerId === false || $this->ownerId === false)) || + $this->fieldId === [] || + $this->primaryOwnerId === [] || + $this->ownerId === [] + ) { throw new QueryAbortedException(); } - if (!empty($this->fieldId) || !empty($this->ownerId) || !empty($this->primaryOwnerId)) { + if (isset($this->fieldId) || isset($this->ownerId) || isset($this->primaryOwnerId)) { // Join in the elements_owners table + $joinType = $mustHaveField || $this->fieldId || $this->ownerId || $this->primaryOwnerId + ? 'INNER JOIN' + : 'LEFT JOIN'; + $ownersCondition = [ 'and', '[[elements_owners.elementId]] = [[elements.id]]', @@ -218,15 +251,16 @@ private function applyNestedElementParams(string $fieldIdColumn, string $primary 'elements_owners.ownerId', 'elements_owners.sortOrder', ]) - ->innerJoin(['elements_owners' => Table::ELEMENTS_OWNERS], $ownersCondition); - $this->subQuery->innerJoin(['elements_owners' => Table::ELEMENTS_OWNERS], $ownersCondition); + ->join($joinType, ['elements_owners' => Table::ELEMENTS_OWNERS], $ownersCondition); + + $this->subQuery->join($joinType, ['elements_owners' => Table::ELEMENTS_OWNERS], $ownersCondition); - if ($this->fieldId) { - $this->subQuery->andWhere([$fieldIdColumn => $this->fieldId]); + if (isset($this->fieldId)) { + $this->subQuery->andWhere([$fieldIdColumn => $this->fieldId ?: null]); } - if ($this->primaryOwnerId) { - $this->subQuery->andWhere([$primaryOwnerIdColumn => $this->primaryOwnerId]); + if (isset($this->primaryOwnerId)) { + $this->subQuery->andWhere([$primaryOwnerIdColumn => $this->primaryOwnerId ?: null]); } // Ignore revision/draft blocks by default @@ -234,7 +268,8 @@ private function applyNestedElementParams(string $fieldIdColumn, string $primary $allowOwnerRevisions = $this->allowOwnerRevisions ?? ($this->id || $this->primaryOwnerId || $this->ownerId); if (!$allowOwnerDrafts || !$allowOwnerRevisions) { - $this->subQuery->innerJoin( + $this->subQuery->join( + $joinType, ['owners' => Table::ELEMENTS], $this->ownerId ? '[[owners.id]] = [[elements_owners.ownerId]]' : "[[owners.id]] = [[$primaryOwnerIdColumn]]" ); @@ -246,6 +281,10 @@ private function applyNestedElementParams(string $fieldIdColumn, string $primary if (!$allowOwnerRevisions) { $this->subQuery->andWhere(['owners.revisionId' => null]); } + + if ($this->ownerId === false) { + $this->subQuery->andWhere(['owners.id' => null]); + } } $this->defaultOrderBy = ['elements_owners.sortOrder' => SORT_ASC]; @@ -263,7 +302,7 @@ private function normalizeNestedElementParams(): void } /** - * Normalizes the fieldId param to an array of IDs or null + * Normalizes the fieldId param to an array of IDs, false, or null */ private function normalizeFieldId(): void { @@ -285,22 +324,29 @@ private function normalizeFieldId(): void } /** - * Normalizes the primaryOwnerId param to an array of IDs or null + * Normalizes the primaryOwnerId param to an array of IDs, false, or null * * @param mixed $value * @return int[]|null|false */ private function normalizeOwnerId(mixed $value): array|null|false { + if ($value === false) { + return false; + } + if (empty($value)) { - return null; + return is_array($value) ? [] : null; } + if (is_numeric($value)) { return [$value]; } + if (!is_array($value) || !ArrayHelper::isNumeric($value)) { return false; } + return $value; } From 53cba141c10c3348504a333c20569c48ac370e24 Mon Sep 17 00:00:00 2001 From: brandonkelly Date: Thu, 10 Sep 2026 14:50:41 -0700 Subject: [PATCH 5/5] Fixed #19594 --- CHANGELOG-5.12.md | 2 + src/base/NestedElementTrait.php | 11 + src/elements/NestedElementManager.php | 10 + src/services/Elements.php | 40 +++ .../NestedElementOwnerDateUpdatedTest.php | 308 ++++++++++++++++++ 5 files changed, 371 insertions(+) create mode 100644 tests/unit/services/NestedElementOwnerDateUpdatedTest.php diff --git a/CHANGELOG-5.12.md b/CHANGELOG-5.12.md index 2d3fa020cfb..0ed79ab13a8 100644 --- a/CHANGELOG-5.12.md +++ b/CHANGELOG-5.12.md @@ -6,6 +6,7 @@ ### Extensibility +- Added `craft\base\NestedElementTrait::$touchOwnersOnSave`. - Added `craft\i18n\Locale::languageId()`. - Added `craft\elements\db\NestedElementQueryTrait::mustHaveField()`. - Added `craft\elements\db\NestedElementQueryTrait::mustHaveOwner()`. @@ -15,3 +16,4 @@ ### System - Fixed a bug where entry and address indexes weren’t showing any results if they had a “Field” condition rule set to “is empty”. +- Fixed a bug where saving a deeply-nested element on its own wouldn’t update its owners’ `dateUpdated` timestamps, which could cause new revisions to reuse stale nested content. ([#19594](https://github.com/craftcms/cms/issues/19594)) diff --git a/src/base/NestedElementTrait.php b/src/base/NestedElementTrait.php index 874d022a819..0a1e06e434a 100644 --- a/src/base/NestedElementTrait.php +++ b/src/base/NestedElementTrait.php @@ -101,6 +101,17 @@ public static function eagerLoadingMap(array $sourceElements, string $handle): a */ public bool $updateSearchIndexForOwner = false; + /** + * @var bool Whether the owner element’s `dateUpdated` timestamp should be updated (recursively, up + * through any further ancestors) when this (canonical) element is saved. + * + * This is set to `false` when a nested element is being saved as part of its owner’s own save + * operation, since the owner’s `dateUpdated` will already be getting updated in that case. + * + * @since 5.12.0 + */ + public bool $touchOwnersOnSave = true; + /** * @var ElementInterface|false|null The primary owner element, or false if [[primaryOwnerId]] is invalid * @see getPrimaryOwner() diff --git a/src/elements/NestedElementManager.php b/src/elements/NestedElementManager.php index be7ebabf134..e3432291d34 100644 --- a/src/elements/NestedElementManager.php +++ b/src/elements/NestedElementManager.php @@ -857,7 +857,17 @@ private function saveNestedElements(ElementInterface $owner): void // Only set $resaving=true if the element isn’t new. // Otherwise NestedElementTrait::saveOwnership() won’t do its thing. $element->resaving = $owner->resaving && $element->id; + // $owner is already being saved, so it (and its own ancestors, if any) will get its + // `dateUpdated` timestamp updated on its own; no need to do that here as well. + // see https://github.com/craftcms/cms/issues/19594 + $touchOwnersOnSave = property_exists($element, 'touchOwnersOnSave'); + if ($touchOwnersOnSave) { + $element->touchOwnersOnSave = false; + } $elementsService->saveElement($element, false); + if ($touchOwnersOnSave) { + $element->touchOwnersOnSave = true; + } // If this element's primary owner is $owner, and it’s a draft of another element whose owner is // $owner's canonical (e.g. a draft entry created by Matrix::_createEntriesFromSerializedData()), diff --git a/src/services/Elements.php b/src/services/Elements.php index c6192b16081..117312a9b8b 100644 --- a/src/services/Elements.php +++ b/src/services/Elements.php @@ -4381,6 +4381,19 @@ private function _saveElementInternal( } } + // Bump the owner elements' `dateUpdated` timestamps, recursively, so freshness checks based on + // `dateUpdated` (e.g. whether a new revision needs to be created for an ancestor) notice that + // something changed, even if this nested element was saved independently of its owner. + if ( + !$element->propagating && + $element instanceof NestedElementInterface && + $element->getIsCanonical() && + isset($element->touchOwnersOnSave) && + $element->touchOwnersOnSave + ) { + $this->touchOwners($element); + } + // Update the changed attributes & fields if ($trackChanges) { $userId = Craft::$app->getUser()->getId(); @@ -4472,6 +4485,33 @@ private function updateSearchIndex( } } + /** + * Updates the `dateUpdated` timestamp of a nested element’s owner, and its owner’s owner, and so on up + * the ownership chain, so that anything checking an ancestor’s `dateUpdated` to determine whether it’s + * changed (e.g. [[\craft\services\Revisions::createRevision()]], deciding whether a new revision needs + * to be created) will notice that it has, even though the ancestor itself wasn’t directly modified. + * + * @param NestedElementInterface $element The (canonical) nested element that was just saved + * @see https://github.com/craftcms/cms/issues/19594 + */ + private function touchOwners(NestedElementInterface $element): void + { + $timestamp = Db::prepareDateForDb($element->dateUpdated ?? DateTimeHelper::now()); + $owner = $element->getOwner(); + + while ($owner !== null) { + Db::update(Table::ELEMENTS, [ + 'dateUpdated' => $timestamp, + ], ['id' => $owner->id]); + + if (!$owner instanceof NestedElementInterface) { + return; + } + + $owner = $owner->getOwner(); + } + } + /** * Propagates an element to a different site * diff --git a/tests/unit/services/NestedElementOwnerDateUpdatedTest.php b/tests/unit/services/NestedElementOwnerDateUpdatedTest.php new file mode 100644 index 00000000000..97f29562e20 --- /dev/null +++ b/tests/unit/services/NestedElementOwnerDateUpdatedTest.php @@ -0,0 +1,308 @@ + outer Matrix block > inner Matrix block), + * establishes an initial revision, edits the innermost block directly (as if via its own edit page, + * without resaving the Page), and confirms that a subsequently-created Page revision is a genuinely new + * revision that captures the updated nested content — rather than reusing the stale prior revision. + * + * @author Pixel & Tonic, Inc. + */ +class NestedElementOwnerDateUpdatedTest extends TestCase +{ + protected Elements $elements; + private PlainText $blockTextField; + private EntryType $innerBlockEntryType; + private Matrix $innerMatrixField; + private EntryType $outerBlockEntryType; + private Matrix $outerMatrixField; + private EntryType $pageEntryType; + private Section $section; + + /** + * @inheritdoc + */ + public function _fixtures(): array + { + return [ + 'sites' => [ + 'class' => SitesFixture::class, + ], + ]; + } + + /** + * @inheritdoc + */ + protected function _before(): void + { + parent::_before(); + $this->elements = Craft::$app->getElements(); + + $primarySiteId = Craft::$app->getSites()->getPrimarySite()->id; + + $this->blockTextField = new PlainText(); + $this->blockTextField->name = 'Block Text'; + $this->blockTextField->handle = 'blockText'; + if (!Craft::$app->getFields()->saveField($this->blockTextField)) { + throw new RuntimeException('Could not save block text field.'); + } + + // The entry type used as the *inner* Matrix block type: Page > outer Matrix > inner Matrix > this. + $this->innerBlockEntryType = new EntryType(); + $this->innerBlockEntryType->name = 'Test Inner Block'; + $this->innerBlockEntryType->handle = 'testInnerBlock'; + $this->innerBlockEntryType->hasTitleField = false; + $this->innerBlockEntryType->titleFormat = '{id}'; + $this->innerBlockEntryType->setFieldLayout( + $this->_makeFieldLayout(Entry::class, $this->blockTextField) + ); + if (!Craft::$app->getEntries()->saveEntryType($this->innerBlockEntryType)) { + throw new RuntimeException('Could not save inner block entry type.'); + } + + $this->innerMatrixField = new Matrix(); + $this->innerMatrixField->name = 'Test Inner Matrix'; + $this->innerMatrixField->handle = 'testInnerMatrix'; + $this->innerMatrixField->setEntryTypes([$this->innerBlockEntryType]); + if (!Craft::$app->getFields()->saveField($this->innerMatrixField)) { + throw new RuntimeException('Could not save inner matrix field.'); + } + + // The entry type used as the *outer* Matrix block type: Page > outer Matrix > this. + $this->outerBlockEntryType = new EntryType(); + $this->outerBlockEntryType->name = 'Test Outer Block'; + $this->outerBlockEntryType->handle = 'testOuterBlock'; + $this->outerBlockEntryType->hasTitleField = false; + $this->outerBlockEntryType->titleFormat = '{id}'; + $this->outerBlockEntryType->setFieldLayout( + $this->_makeFieldLayout(Entry::class, $this->innerMatrixField) + ); + if (!Craft::$app->getEntries()->saveEntryType($this->outerBlockEntryType)) { + throw new RuntimeException('Could not save outer block entry type.'); + } + + $this->outerMatrixField = new Matrix(); + $this->outerMatrixField->name = 'Test Outer Matrix'; + $this->outerMatrixField->handle = 'testOuterMatrix'; + $this->outerMatrixField->setEntryTypes([$this->outerBlockEntryType]); + if (!Craft::$app->getFields()->saveField($this->outerMatrixField)) { + throw new RuntimeException('Could not save outer matrix field.'); + } + + $this->pageEntryType = new EntryType(); + $this->pageEntryType->name = 'Test Page'; + $this->pageEntryType->handle = 'testPage'; + $this->pageEntryType->hasTitleField = true; + $this->pageEntryType->setFieldLayout( + $this->_makeFieldLayout(Entry::class, $this->outerMatrixField) + ); + if (!Craft::$app->getEntries()->saveEntryType($this->pageEntryType)) { + throw new RuntimeException('Could not save page entry type.'); + } + + $this->section = new Section(); + $this->section->name = 'Test Page Section'; + $this->section->handle = 'testPageSection'; + $this->section->type = Section::TYPE_CHANNEL; + $this->section->enableVersioning = true; + $this->section->setEntryTypes([$this->pageEntryType]); + $this->section->setSiteSettings([ + new Section_SiteSettings([ + 'siteId' => $primarySiteId, + 'enabledByDefault' => true, + 'hasUrls' => false, + ]), + ]); + if (!Craft::$app->getEntries()->saveSection($this->section)) { + throw new RuntimeException('Could not save section.'); + } + } + + /** + * Builds a single-tab field layout containing a single custom field. + */ + private function _makeFieldLayout(string $type, \craft\base\FieldInterface $field): FieldLayout + { + $fieldLayout = new FieldLayout(['type' => $type]); + $tab = new FieldLayoutTab(['name' => 'Content']); + $tab->setLayout($fieldLayout); + $tab->setElements([new CustomField($field)]); + $fieldLayout->setTabs([$tab]); + return $fieldLayout; + } + + /** + * @inheritdoc + */ + protected function _after(): void + { + Craft::$app->getEntries()->deleteSection($this->section); + Craft::$app->getEntries()->deleteEntryType($this->pageEntryType); + Craft::$app->getFields()->deleteField($this->outerMatrixField); + Craft::$app->getEntries()->deleteEntryType($this->outerBlockEntryType); + Craft::$app->getFields()->deleteField($this->innerMatrixField); + Craft::$app->getEntries()->deleteEntryType($this->innerBlockEntryType); + Craft::$app->getFields()->deleteField($this->blockTextField); + parent::_after(); + } + + /** + * Reproduces #19594: saving a doubly-nested Matrix block on its own doesn’t bump its ancestors’ + * `dateUpdated` timestamps, so a subsequently-requested revision of the top-level owner gets reused + * (stale) rather than freshly created — losing track of the nested change entirely. + */ + public function testSavingNestedElementTouchesOwnersRecursively(): void + { + $primarySiteId = Craft::$app->getSites()->getPrimarySite()->id; + + // 1. Create a page with a nested (outer Matrix > inner Matrix > plain text) structure. + $page = new Entry(); + $page->sectionId = $this->section->id; + $page->typeId = $this->pageEntryType->id; + $page->siteId = $primarySiteId; + $page->title = 'Test Page'; + $page->setFieldValue('testOuterMatrix', [ + 'new1' => [ + 'type' => 'testOuterBlock', + 'fields' => [ + 'testInnerMatrix' => [ + 'new1' => [ + 'type' => 'testInnerBlock', + 'fields' => ['blockText' => 'original content'], + ], + ], + ], + ], + ]); + if (!$this->elements->saveElement($page)) { + throw new RuntimeException('Could not save page: ' . implode(', ', $page->getFirstErrors())); + } + + $outerBlockEntry = Entry::find() + ->ownerId($page->id) + ->fieldId($this->outerMatrixField->id) + ->siteId($primarySiteId) + ->status(null) + ->one(); + self::assertNotNull($outerBlockEntry, 'Expected to find the canonical outer Matrix block entry.'); + + $innerBlockEntry = Entry::find() + ->ownerId($outerBlockEntry->id) + ->fieldId($this->innerMatrixField->id) + ->siteId($primarySiteId) + ->status(null) + ->one(); + self::assertNotNull($innerBlockEntry, 'Expected to find the canonical inner Matrix block entry.'); + self::assertSame('original content', $innerBlockEntry->getFieldValue('blockText')); + + // 2. Establish an initial revision for the page, which cascades down and creates matching + // revisions for both nested block levels. + $revisionId1 = Craft::$app->getRevisions()->createRevision($page); + + $originalPageDateUpdated = $page->dateUpdated; + $originalOuterBlockDateUpdated = $outerBlockEntry->dateUpdated; + + // Make sure the upcoming edit gets a distinct (later) `dateUpdated` timestamp, since timestamps + // are only stored with second precision. + sleep(1); + + // 3. Edit the *innermost* block directly and save only that element — exactly as if it were + // edited via its own nested-entry edit page, without resaving the Page or the outer block. + $innerBlockEntry->setFieldValue('blockText', 'updated content'); + if (!$this->elements->saveElement($innerBlockEntry)) { + throw new RuntimeException('Could not save inner block: ' . implode(', ', $innerBlockEntry->getFirstErrors())); + } + + // 4. The fix: saving the canonical inner block should have bumped `dateUpdated` on both of its + // ancestors, recursively, even though neither was directly touched. + $refetchedOuterBlockEntry = Entry::find() + ->id($outerBlockEntry->id) + ->siteId($primarySiteId) + ->status(null) + ->one(); + self::assertNotNull($refetchedOuterBlockEntry); + self::assertGreaterThan( + $originalOuterBlockDateUpdated->getTimestamp(), + $refetchedOuterBlockEntry->dateUpdated->getTimestamp(), + 'Expected the outer Matrix block’s dateUpdated to be bumped when its nested inner block was saved.', + ); + + $refetchedPage = Entry::find() + ->id($page->id) + ->siteId($primarySiteId) + ->status(null) + ->one(); + self::assertNotNull($refetchedPage); + self::assertGreaterThan( + $originalPageDateUpdated->getTimestamp(), + $refetchedPage->dateUpdated->getTimestamp(), + 'Expected the page’s dateUpdated to be bumped (recursively) when its doubly-nested block was saved.', + ); + + // 5. Requesting a new revision for the page (using the freshly-refetched element, just like a + // real, separate request would) should now create a *genuinely new* revision... + $revisionId2 = Craft::$app->getRevisions()->createRevision($refetchedPage); + self::assertNotSame($revisionId1, $revisionId2, 'Expected a new revision to be created, since nested content changed.'); + + // 6. ...and that new revision’s nested content should reflect the update, all the way down. + $revisionOuterBlockEntry = Entry::find() + ->ownerId($revisionId2) + ->fieldId($this->outerMatrixField->id) + ->siteId($primarySiteId) + ->revisions(null) + ->status(null) + ->one(); + self::assertNotNull($revisionOuterBlockEntry, 'Expected the new revision to have its own nested outer Matrix block entry.'); + + $revisionInnerBlockEntry = Entry::find() + ->ownerId($revisionOuterBlockEntry->id) + ->fieldId($this->innerMatrixField->id) + ->siteId($primarySiteId) + ->revisions(null) + ->status(null) + ->one(); + self::assertNotNull($revisionInnerBlockEntry, 'Expected the new revision’s outer block to have its own nested inner Matrix block entry.'); + + self::assertSame( + 'updated content', + $revisionInnerBlockEntry->getFieldValue('blockText'), + 'Expected the new revision to capture the updated nested content, not the stale original.', + ); + } +}