Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Validation messages translation #800

Closed
wants to merge 4 commits into from
Closed

Validation messages translation #800

wants to merge 4 commits into from

Conversation

LastDragon-ru
Copy link
Contributor

Probably the most simple solution :)

  1. Add
<?php declare(strict_types = 1);

namespace App\GraphQL;

use GraphQL\Validator\Rules\ValuesOfCorrectType;

use function sprintf;

class ValuesOfCorrectTypeTranslated extends ValuesOfCorrectType {
    public function getName() {
        return ValuesOfCorrectType::class;
    }

    public static function badArgumentValueMessage($typeName, $valueName, $fieldName, $argName, $message = null) {
        return sprintf('Свойство "%s" аргумент "%s" должен быть %s, но передан %s', $fieldName, $argName, $typeName, $valueName);
    }
}
  1. Add rule
DocumentValidator::addRule(new ValuesOfCorrectTypeTranslated());
  1. For lighthouse:
<?php declare(strict_types = 1);

namespace App\GraphQL;

use GraphQL\Validator\Rules\ValuesOfCorrectType;
use Nuwave\Lighthouse\Execution\ValidationRulesProvider;

class ProvidesValidationRulesTranslated extends ValidationRulesProvider {
    public function validationRules(): ?array {
        return [
                ValuesOfCorrectType::class => new ValuesOfCorrectTypeTranslated(),
            ] + parent::validationRules();
    }
}

And in AppServiceProvider:

$this->app->bind(ProvidesValidationRules::class, ProvidesValidationRulesTranslated::class);

Voila
image

Closes #711, overrides #713 and #718

@coveralls
Copy link

coveralls commented Mar 25, 2021 •
edited
Loading

Coverage Status

Coverage increased (+0.1%) to 86.482% when pulling 4c7f7df on LastDragon-ru:validation-messages-translation into 65428ce on webonyx:master.

@spawnia
Copy link
Collaborator

spawnia commented Mar 25, 2021

Can you add a test to ensure overwriting ValuesOfCorrectType?

@LastDragon-ru
Copy link
Contributor Author

Can you add a test to ensure overwriting ValuesOfCorrectType?

Do you mean for DocumentValidator?

@spawnia
Copy link
Collaborator

spawnia commented Mar 25, 2021

Yeah. Basically what you wrote in the description:

  • extend ValuesOfCorrectType
  • register your custom implementation
  • trigger a document validation error
  • assert the message comes from your custom implementation

@LastDragon-ru
Copy link
Contributor Author

trigger a document validation error

Could you please give me an example how I can do it?

spawnia
spawnia reviewed Mar 26, 2021
View reviewed changes
tests/Validator/ValuesOfCorrectTypeTest.php Outdated Show resolved Hide resolved
tests/Validator/ValuesOfCorrectTypeTest.php
{
public function getName()
{
return (new ValuesOfCorrectType())->getName();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this function overwritten at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because getName() used by DocumentValidator::addRule() and we want to replace ValuesOfCorrectType completely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And the default realization of getName() uses static::class

graphql-php/src/Validator/Rules/ValidationRule.php

Lines 16 to 19 in 65428ce

public function getName()
{
return $this->name === '' || $this->name === null ? static::class : $this->name;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

That seems like an annoying gotcha for users that extend this class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤷‍♂️ Maybe something like?

class ValuesOfCorrectType extends ValidationRule
{
    // (1)
    /** @var string */
    protected $name = self::class;
    
    // (2) or this (probably a breaking change) 
    public function __construct() { 
        $this->name = self::class;
    }

@spawnia
Copy link
Collaborator

spawnia commented Dec 16, 2021

Thanks for this POC, I will make sure this way of overwriting rules keeps working - see

graphql-php/tests/Validator/CustomRuleTest.php

Lines 19 to 55 in a5d64c7

public function testAddRuleCanReplaceDefaultRules(): void
{
DocumentValidator::addRule(new class extends ExecutableDefinitions
{
public function getName(): string
{
return ExecutableDefinitions::class;
}
public static function nonExecutableDefinitionMessage(string $defName): string
{
return "Custom message including {$defName}";
}
});
$this->expectInvalid(
self::getTestSchema(),
null,
'
query Foo {
dog {
name
}
}
type Cow {
name: String
}
',
[
ErrorHelper::create(
'Custom message including Cow',
[new SourceLocation(8, 12)]
),
]
);
}

@spawnia spawnia closed this Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spawnia spawnia spawnia left review comments

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Validation messages translation
3 participants
@LastDragon-ru @coveralls @spawnia