Skip to content

[Validator] Missing information about payload in callback #18791

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

Merged
merged 1 commit into from
Aug 28, 2023
Merged

[Validator] Missing information about payload in callback #18791

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions reference/constraints/Callback.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,11 +304,12 @@ callback method:
* A closure.

Concrete callbacks receive an :class:`Symfony\\Component\\Validator\\Context\\ExecutionContextInterface`
instance as only argument.
instance as the first argument and the :ref:`payload option <payload>` as the second argument.

Static or closure callbacks receive the validated object as the first argument
and the :class:`Symfony\\Component\\Validator\\Context\\ExecutionContextInterface`
instance as the second argument.
, the :class:`Symfony\\Component\\Validator\\Context\\ExecutionContextInterface`
instance as the second argument and the :ref:`payload option <payload>` as the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about the :ref: I used. When I build locally it's broken: href is Sequentially.html#payload

What syntax should I use?

It's in .. include:: /reference/constraints/_payload-option.rst.inc

image

Copy link
Member

Choose a reason for hiding this comment

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

This is tricky:

  • RST creates the anchors based on the section titles
  • We're including a RST doc with a section title called Payload
  • So, the payload cross reference exists and it's valid

But, I don't like this automatic references much because if you change the title, then the reference breaks. To avoid this, while merging I've added a explicit reference name in the included file and used it in the links. See 61fcb40

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, the payload cross reference exists and it's valid

This behavior is very strange, and can confuse user because it redirect to another page. https://symfony.com/doc/current/reference/constraints/Ip.html#reference-constraints-payload for example

Do you know if we can improve this, in which repository ?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if it's fixable in symfony-tools repository or in the underlying RST parser 😐

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created issue doctrine/rst-parser#275 with a reproducer and fix proposal (I hope 😄 )

third argument.

.. include:: /reference/constraints/_groups-option.rst.inc

Expand Down