fix: report correct errors for json schema validation

[CaptainStandby]

• Implemented the translation of jsonschema.ValidationError to errors codes documented here
• Added missing error codes for relevant schema errors

  Validation	Name	ID
  maxLength	ErrorValidationMaxLength	4000017
  minimum	ErrorValidationMinimum.	4000018
  exclusiveMinimum	ErrorValidationExclusiveMinimum	4000019
  maximum	ErrorValidationMaximum	4000020
  exclusiveMaximum	ErrorValidationExclusiveMaximum	4000021
  multipleOf	ErrorValidationMultipleOf	4000022
  maxItems	ErrorValidationMaxItems	4000023
  minItems	ErrorValidationMinItems	4000024
  uniqueItems	ErrorValidationUniqueItems	4000025
  type	ErrorValidationWrongType	4000026

• Updated e2e tests to check these IDs explicitly

Related issue(s)

#2974

Checklist

• I have read the contributing guidelines.
• I have referenced an issue containing the design document if my change
  introduces a new feature.
• I am following the
  contributing code guidelines.
• I have read the security policy.
• I confirm that this pull request does not address a security
  vulnerability. If this pull request addresses a security vulnerability, I
  confirm that I got the approval (please contact
  [email protected]) from the maintainers to push
  the changes.
• I have added tests that prove my fix is effective or that my feature
  works.
• I have added or changed the documentation.

Further Comments

[codecov]

Codecov Report

Merging #3085 (e443928) into master (74ae852) will increase coverage by 0.17%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3085      +/-   ##
==========================================
+ Coverage   77.19%   77.36%   +0.17%     
==========================================
  Files         315      315              
  Lines       19772    19817      +45     
==========================================
+ Hits        15263    15332      +69     
+ Misses       3315     3295      -20     
+ Partials     1194     1190       -4     

Impacted Files	Coverage Δ
schema/errors.go	88.77% <ø> (+6.70%)	⬆️
cmd/clidoc/main.go	67.90% <100.00%> (+1.56%)	⬆️
text/message_validation.go	100.00% <100.00%> (ø)
ui/container/container.go	88.46% <100.00%> (+2.63%)	⬆️
selfservice/strategy/link/sender.go	70.10% <0.00%> (-1.07%)	⬇️
driver/config/config.go	82.77% <0.00%> (-0.11%)	⬇️
selfservice/strategy/code/code_sender.go	78.07% <0.00%> (-0.06%)	⬇️
courier/message.go	73.91% <0.00%> (+2.17%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[aeneasr]

fyi :) in the open source repositories we have a different changelog strategy - here you can use regular commit messages :)

[CaptainStandby]

fyi :) in the open source repositories we have a different changelog strategy - here you can use regular commit messages :)

good to know :-D

[jonas-jonas]

Nice work, just one question about the messages & variables.

[jonas-jonas]

LGTM.

[aeneasr]

Epic! Are there any backwards compatibility issues that could happen with this and if so how do we deal with them?

[CaptainStandby]

Epic! Are there any backwards compatibility issues that could happen with this and if so how do we deal with them?

I would argue that responding with ErrorValidationGeneric for all validations was a bug and code relying on this undocumented behaviour should be updated (like I did for some of the e2e tests.
Otherwise this only adds more options to gain insights into the validation.

[aeneasr]

I see these scenarios:

1. Tests rely on this behavior - this is ok if we break it
2. Custom ui implementation uses translation based on the IDs - it will mean that they have untranslated strings which is ok
3. Custom ui implementation uses the IDs to perform logic - this would break without warning

Did I miss something? What should we do to get (3) over the line smoothly in your opinion?

[CaptainStandby]

I don't have a good solution for this.
Maybe we could just issue two errors for the same validation, one with the old value and one with the new. I'm not entirely sure if this is possible and it doesn't sound like the best idea.
WDYT?

[aeneasr]

I'm fine with breaking it. I don't think anyone really uses it this way. We can probably check in the community and see if it's a problem before deploying it to production?

[lukaszxion]

I'm using it. Now there is no context information and the messages look like this:
Länge muss >= {{expected_length}} sein, hat aber {{actual_length}}.

The use of context was proposed by Ory:
#692 (comment)

How can I now get the params for the translations?