Skip to content

IBX-11493: Added language code validation#736

Open
Steveb-p wants to merge 7 commits intomainfrom
ibx-11493/language-code-validation
Open

IBX-11493: Added language code validation#736
Steveb-p wants to merge 7 commits intomainfrom
ibx-11493/language-code-validation

Conversation

@Steveb-p
Copy link
Copy Markdown
Contributor

@Steveb-p Steveb-p commented Mar 23, 2026

🎫 Issue IBX-11493

Description:

This PR adds metadata to LanguageCreateStruct, with regex condition for the shape of the language code.

Additionally, it refactors LanguageServiceTest code to use newer integration test setup, since the original one was using DummyValidator service, which prevented the test from performing correctly.

Tested manually against a migration:

-   type: language
    mode: create
    metadata:
        languageCode: .
        name: Polish
        enabled: true

Result:
image

Technically, operations that directly preceed the validator could be moved into validator itself as well (is name empty, is code empty, is code used already?), but I opted not to - since they would introduce more complexity to this PR. Can be done as a follow up if desired.

For QA:

Documentation:

@Steveb-p Steveb-p force-pushed the ibx-11493/language-code-validation branch from 8a5e2cf to 4e03481 Compare March 23, 2026 16:09
@Steveb-p Steveb-p force-pushed the ibx-11493/language-code-validation branch from 4e03481 to 8dac6a5 Compare March 23, 2026 16:26
@Steveb-p Steveb-p changed the base branch from main to fix-phpstan March 23, 2026 16:27
@Steveb-p Steveb-p force-pushed the ibx-11493/language-code-validation branch from 8dac6a5 to 85a616c Compare March 23, 2026 16:35
@Steveb-p Steveb-p changed the base branch from fix-phpstan to main March 23, 2026 17:57
@Steveb-p Steveb-p force-pushed the ibx-11493/language-code-validation branch from 90267e6 to 4f830d7 Compare March 23, 2026 17:58
@Steveb-p Steveb-p requested a review from a team March 23, 2026 18:00
@Steveb-p Steveb-p added Bug Something isn't working Ready for review labels Mar 23, 2026
@ibexa-workflow-automation-1 ibexa-workflow-automation-1 bot requested review from ViniTou, alongosz, barw4, ciastektk, konradoboza, mikadamczyk, tbialcz and wiewiurdp and removed request for a team March 23, 2026 18:00
…Kernel` and added `CoreSolrTestKernel` implementation
@sonarqubecloud
Copy link
Copy Markdown

Comment on lines +24 to 26
#[Assert\NotBlank]
#[Assert\Regex('~^[a-zA-Z\_\-]+$~')]
public ?string $languageCode = null;
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.

Aren't we making a slight BC break here? Or maybe we are validating if later on on the UI side? POV ping @alongosz.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Technically it can be a BC break if someone has migrations that were executing successfully until this regular expression assertion is introduced.

That said, as mentioned in the Jira ticket, creating certain languages (for example, with . as code) breaks the application entirely.

* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/

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.

Missing declaring strict types.

if ($value instanceof ValueObject) {
self::assertStructPropertiesCorrect($value, $actualObject->$propertyName[$key]);
} else {
self::assertPropertiesEqual("$propertyName\[$key\]", $value, $actualObject->$propertyName[$key]);
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.

Suggested change
self::assertPropertiesEqual("$propertyName\[$key\]", $value, $actualObject->$propertyName[$key]);
self::assertPropertiesEqual(
"$propertyName\[$key\]",
$value,
$actualObject->$propertyName[$key]
);

*
* If the property type is array, it will be sorted before comparison.
*
* @TODO: introduced because of randomly failing tests, ref: https://issues.ibexa.co/browse/EZP-21734
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.

I don't think this is relevant anymore, or?

$languageService->createLanguage($languageCreate);
/* END: Use Case */
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('Argument \'languageCreateStruct\' is invalid: language with the "nor-NO" language code already exists');
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.

Suggested change
$this->expectExceptionMessage('Argument \'languageCreateStruct\' is invalid: language with the "nor-NO" language code already exists');
$this->expectExceptionMessage(
'Argument \'languageCreateStruct\' is invalid: language with the "nor-NO" language code already exists'
);

* @covers \Ibexa\Contracts\Core\Repository\LanguageService::loadLanguageListById
*
* @depends testCreateLanguage
* @testWith ["."]
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.

Why this instead of dataProvider?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's simpler.

It will also look a lot better with attributes:

    #[TestWith(['EUR'])]
    #[TestWith(['USD'])]
    #[TestWith([''])]
    #[TestWith(['pln'])]
    public function foo(string $currency) {
    }

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.

The one you posted looks even better.

* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/

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.

Suggested change
declare(strict_types=1);

@konradoboza
Copy link
Copy Markdown
Contributor

PHPStan is failing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working Ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants