8000 [Validator] [DateTime] Add `format` to error messages by saulius334 · Pull Request #58559 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Validator] [DateTime] Add format to error messages #58559

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
Merged
merged 1 commit into from
Jan 5, 2025
Merged

[Validator] [DateTime] Add format to error messages #58559

merged 1 commit into from
Jan 5, 2025

Conversation

saulius334
Copy link
@saulius334 saulius334 commented Oct 14, 2024 •
edited
Loading
Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
Issues -
License MIT

Problem

Currently there is no way to dynamically add the format value into error messages when using the DateTime constraint. It has to be hardcoded on every DateTime constraint.
For example:

#[Assert\DateTime(format: 'Y-m-d', message: 'Value {{ value }} is invalid. Expected format: "Y-m-d"')]
public string $from;

#[Assert\DateTime(format: 'Y-m-d', message: 'Value {{ value }} is invalid. Expected format: "Y-m-d"')]
public string $to;

Goal

The goal is to add the possibility to use a parameter instead of hardcoding the selected format.
For example:

#[Assert\DateTime(format: 'Y-m-d', message: 'Value {{ value }} is invalid. Expected format:{{ format }}')]
public string $from;

If using the symfony translator we would even not need to set the message when creating the constraint.
For example:

$translator = new Translator('en');
$translator->addLoader('array', new ArrayLoader());
$translator->addResource('array', [
    'This value is not a valid datetime.' => 'Value {{ value }} is invalid. Expected format:{{ format }}',
], 'en');

Solution

When building the violations set a new parameter called format which represents the given format when the DateTime constraint is created.

@carsonbot

This comment was marked as outdated.

Sign in to view
@saulius334 saulius334 marked this pull request as ready for review October 14, 2024 09:04
@carsonbot carsonbot added this to the 5.4 milestone Oct 14, 2024
@carsonbot
Copy link

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

  • Always add tests
  • Keep backward compatibility (see https://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
  • Features and deprecations must be submitted against the 7.2 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@OskarStark OskarStark modified the milestones: 5.4, 7.2 Oct 14, 2024
@OskarStark
Copy link
Contributor

Hi, thanks for this new feature. New features need to target the latest branch, which is 7.2 right now.

@saulius334 saulius334 changed the base branch from 5.4 to 7.2 October 14, 2024 10:42
@saulius334
Copy link
Author

Hi, thanks for this new feature. New features need to target the latest branch, which is 7.2 right now.

@OskarStark Thank you for your quick review. Changed the target branch to latest (7.2)

@OskarStark OskarStark changed the title [Validator] [DateTime] Add "format" to error messages [Validator] [DateTime] Add format to error messages Oct 14, 2024
@nicolas-grekas
Copy link
Member

Duplicate of #58602, which was rejected.
Please open an issue if you want to discuss this.

@nicolas-grekas
Copy link
Member

Aaah sorry I was too fast, that's different.

@nicolas-grekas nicolas-grekas reopened this Nov 5, 2024
@nicolas-grekas
Copy link
Member

Please add a line to the changelog of the component.

@saulius334 saulius334 force-pushed the validator_add_datetime_format_to_violation_message branch from 9e2f5ad to d5beef3 Compare November 8, 2024 07:20
@saulius334
Copy link
Author

@nicolas-grekas Thank you for your comment. Added changes to changelog

@fabpot fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024
@saulius334
Copy link
Author

Bump, anything else I need to add/change to move this forward ?

@fabpot fabpot force-pushed the validator_add_datetime_format_to_violation_message branch from 996e398 to a695074 Compare January 5, 2025 14:43
@fabpot fabpot force-pushed the validator_add_datetime_format_to_violation_message branch from a695074 to 20e83b9 Compare January 5, 2025 14:44
@fabpot
Copy link
Member
fabpot commented Jan 5, 2025

Thank you @saulius334.

@fabpot fabpot merged commit fc65c8f into symfony:7.3 Jan 5, 2025
3 checks passed
@fabpot fabpot mentioned this pull request Jan 5, 2025
Closed
@xabbuh xabbuh mentioned this pull request Jan 6, 2025
Merged
nicolas-grekas added a commit that referenced this pull request Jan 7, 2025
@nicolas-grekas
minor #59379 [MonologBridge] revert test changes (xabbuh)
697a15e 
This PR was merged into the 7.3 branch.

Discussion
----------

[MonologBridge] revert test changes

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        |
| License       | MIT

I fail to see why these changes have been done in #58559.

Commits
-------

71d8ec0 revert test changes
@fabpot fabpot mentioned this pull request May 2, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
63FD
Reviewers

@fabpot fabpot fabpot approved these changes

Assignees
No one assigned
Labels
Feature Status: Reviewed Validator
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.
5 participants
@saulius334 @carsonbot @OskarStark @nicolas-grekas @fabpot
0