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.

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

Validation messages #13208

Closed
JABirchall opened this issue Dec 12, 2017 · 10 comments

Comments

@JABirchall
Copy link

commented Dec 12, 2017

Im not sure as to the reason behind this decision, but I feel that the validation messages for the included validators should be within the validators them selfs not the base class.

https://github.com/phalcon/cphalcon/blob/master/phalcon/validation.zep#L334-L360

This is a very minor issue and more of my ODD kicking in, just feel its better design to have each validator define its default message to allow for easier extension and reliable removal of validators.

@virgofx

This comment has been minimized.

Copy link
Member

commented Dec 12, 2017

Agreed. PRs are welcome. This should be a pretty straightforward refactor.

@JABirchall

This comment has been minimized.

Copy link
Author

commented Dec 12, 2017

I would if I knew how to work with zephir :P

@Jurigag

This comment has been minimized.

Copy link
Member

commented Dec 14, 2017

Just clone repo, install zephir extension into phpstorm and code it and do pr. Nothing fancy really.

@stale stale bot added the stale label Apr 16, 2018

@sergeyklay sergeyklay closed this Apr 16, 2018

@JABirchall

This comment has been minimized.

Copy link
Author

commented Apr 16, 2018

I feel this issue should stay open. @sergeyklay

@sergeyklay sergeyklay reopened this Jun 26, 2018

@stale stale bot removed the stale label Jun 26, 2018

@stale stale bot added the stale label Sep 24, 2018

@stale stale bot closed this Sep 25, 2018

@niden niden reopened this Sep 25, 2018

@stale stale bot removed the stale label Sep 25, 2018

@phalcon phalcon deleted a comment from stale bot Dec 22, 2018

@phalcon phalcon deleted a comment from stale bot Dec 22, 2018

@niden niden added this to To do in 4.0 Release via automation Dec 23, 2018

@niden niden self-assigned this Dec 23, 2018

@niden

This comment has been minimized.

Copy link
Member

commented Feb 23, 2019

Closing in favor of #13855. Will revisit if the community votes for it, or in later versions.

@niden niden closed this Feb 23, 2019

4.0 Release automation moved this from To do to Done Feb 23, 2019

@emiliodeg

This comment has been minimized.

Copy link
Member

commented Apr 27, 2019

@niden can you assign me this?

@niden niden reopened this Apr 27, 2019

4.0 Release automation moved this from Done to In progress Apr 27, 2019

@niden

This comment has been minimized.

Copy link
Member

commented Apr 27, 2019

@emiliodeg all yours

@niden niden added the Enhancement label May 16, 2019

@emiliodeg

This comment has been minimized.

Copy link
Member

commented May 16, 2019

I've been working on this and I want to share with you my design to receive opinions before doing all the work

I use an advice property to warning message, to avoid confusion with Phalcon\Messages\Message

Some validators like Phalcon\Validation\Validator\File starts to work like a container and split the inside validations in new validatos like Phalcon\Validation\Validator\File\Size, Phalcon\Validation\Validator\File\MimeType, Phalcon\Validation\Validator\File\Resolution, etc

The main idea is to have a transparent transition. All current definitions will continue to work as they do now

@niden niden added the Breaks BC label May 16, 2019

@emiliodeg emiliodeg referenced this issue Jun 20, 2019

Merged

T13208 validation messages #14193

4 of 5 tasks complete

niden added a commit to emiliodeg/cphalcon that referenced this issue Jun 20, 2019

niden added a commit to emiliodeg/cphalcon that referenced this issue Jun 20, 2019

niden added a commit to emiliodeg/cphalcon that referenced this issue Jun 20, 2019

niden added a commit to emiliodeg/cphalcon that referenced this issue Jun 20, 2019

niden added a commit to emiliodeg/cphalcon that referenced this issue Jun 20, 2019

niden added a commit to emiliodeg/cphalcon that referenced this issue Jun 20, 2019

niden added a commit to emiliodeg/cphalcon that referenced this issue Jun 21, 2019

niden added a commit to emiliodeg/cphalcon that referenced this issue Jun 21, 2019

niden added a commit to emiliodeg/cphalcon that referenced this issue Jun 21, 2019

niden added a commit to emiliodeg/cphalcon that referenced this issue Jun 21, 2019

niden added a commit to emiliodeg/cphalcon that referenced this issue Jun 21, 2019

niden added a commit that referenced this issue Jun 21, 2019

Merge branch 'T13208-validation-messages' of https://github.com/emili…
…odeg/cphalcon into emiliodeg-T13208-validation-messages

* 'T13208-validation-messages' of https://github.com/emiliodeg/cphalcon: (27 commits)
  [#13208] - Corrected more tests
  [#13208] - More test corrections
  [#13208] - Corrected test
  [#13208] - Corrected references
  [#13208] - Trying to fix the observed var
  [#13208] - Trying different scope
  [#13208] - Trying interface corrections
  Revert "[#13208] - Interface corrections"
  [#13208] - Interface corrections
  Revert "[#13208] - Interface corrections/docblocks"
  [#13208] - Interface corrections/docblocks
  Updated docblock
  Updated validators
  Fixed array fields
  Fixed some tests
  Changed advice to template
  Changed advice for template
  More string length tests
  String length tests
  Added a message factory
  ...

niden added a commit that referenced this issue Jun 21, 2019

Merge branch 'emiliodeg-T13208-validation-messages' into 4.0.x
* emiliodeg-T13208-validation-messages: (27 commits)
  [#13208] - Corrected more tests
  [#13208] - More test corrections
  [#13208] - Corrected test
  [#13208] - Corrected references
  [#13208] - Trying to fix the observed var
  [#13208] - Trying different scope
  [#13208] - Trying interface corrections
  Revert "[#13208] - Interface corrections"
  [#13208] - Interface corrections
  Revert "[#13208] - Interface corrections/docblocks"
  [#13208] - Interface corrections/docblocks
  Updated docblock
  Updated validators
  Fixed array fields
  Fixed some tests
  Changed advice to template
  Changed advice for template
  More string length tests
  String length tests
  Added a message factory
  ...
@niden

This comment has been minimized.

Copy link
Member

commented Jun 21, 2019

Resolved in #14193

@niden niden closed this Jun 21, 2019

4.0 Release automation moved this from In progress to Done Jun 21, 2019

@JABirchall

This comment has been minimized.

Copy link
Author

commented Jun 21, 2019

Thank you guys :)

@niden niden added the 4.0 label Jun 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
6 participants
You can’t perform that action at this time.