Server validation handling

[esizer]

There are multiple different ways to handle validation in laravel or lighthouse-php we currently use a mix of them.

I feel like this is very subjective so I want to see how we feel about possibly trying to consolidate it so we can go to one place to check validation.

Validation methods

ValidationException (control flow)

We sometimes just have control flow right in the mutations and throw exceptions manually.

gc-digital-talent/api/app/GraphQL/Mutations/ArchivePool.php

Lines 20 to 22 in 6f6acac

	if ($pool->getStatusAttribute() !== PoolStatus::CLOSED->name) {
	throw ValidationException::withMessages(['ArchivePoolInvalidStatus']);
	}

Validator::make

We also make validators right in the mutations and manually add errors to the bag when it fails and then throw the exception.

gc-digital-talent/api/app/GraphQL/Mutations/CreateApplication.php

Lines 21 to 33 in 6f6acac

	$createValidator = new CreateApplicationValidator($args['poolId'], $args['userId']);
	$validator = Validator::make(
	$args,
	$createValidator->rules(),
	$createValidator->messages(),
	);
	if ($validator->fails()) {
	throw new ValidationException(
	$validator->errors()->first(),
	$validator
	);
	}

@validator directive

We also use the validator directive with custom rules in some places.

https://github.com/GCTC-NTGC/gc-digital-talent/blob/main/api/app/GraphQL/Validators/AssessmentPlanIsCompleteValidator.php

Throwing exceptions in validators

This is one that probably should not be done regardless of what we decide but we have validators with no rules and manually throw an exception which seems very odd to me. But, if people like it, we can keep it 😅

gc-digital-talent/api/app/GraphQL/Validators/RevertFinalDecisionValidator.php

Lines 32 to 34 in 9188e3b

	if (! (in_array($candidate->pool_candidate_status, $statusesArray))) {
	throw ValidationException::withMessages([ApiErrorEnums::INVALID_STATUS_REVERT_FINAL_DECISION]);
	}

[esizer]

My opinion on this, I don't mind having different ways since creating a class to handle a single rule seems heavy handed. However, the second instance where we make a class and then wrap it in a Validator::make then throw the exception is basically re-implementing how the @validator directive works so I'd be in favour of removing those and just using the directive instead.