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.

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

T14203 fix form validator get value #15752

Merged
merged 12 commits into from
Nov 7, 2021
Merged

T14203 fix form validator get value #15752

merged 12 commits into from
Nov 7, 2021

Conversation

BeMySlaveDarlin
Copy link
Contributor

@BeMySlaveDarlin BeMySlaveDarlin commented Nov 2, 2021 •
edited
Loading

Hello!

In raising this pull request, I confirm the following:

  • I have read and understood the Contributing Guidelines
  • I have checked that another pull request for this purpose does not exist
  • I wrote some tests for this PR
  • I have updated the relevant CHANGELOG
  • I have created a PR for the documentation about this change

Small description of change:

  • Phalcon\Validation::getValue() seeks through this->data if not found in this->entity;
  • Phalcon\Forms\Form::isValid() signature was changed: added 3rd argument whitelist;
  • Added setter and getter for whitelist in Phalcon\Forms\Form.

Thanks

Jeckerson
Jeckerson reviewed Nov 3, 2021
View reviewed changes
phalcon/Validation.zep Outdated Show resolved Hide resolved
@BeMySlaveDarlin BeMySlaveDarlin self-assigned this Nov 3, 2021
@BeMySlaveDarlin BeMySlaveDarlin added 5.0 The issues we want to solve in the 5.0 release breaks bc Functionality that breaks Backwards Compatibility labels Nov 3, 2021
@BeMySlaveDarlin BeMySlaveDarlin changed the title Issue/14203 fix form validator get value T14203 fix form validator get value Nov 3, 2021
@BeMySlaveDarlin BeMySlaveDarlin force-pushed the issue/14203-fix-form-validator-get-value branch from dc41377 to d6fd0a5 Compare November 4, 2021 02:55
@BeMySlaveDarlin
Copy link
Contributor Author

Rebased due to fix conflicts

Jeckerson
Jeckerson reviewed Nov 4, 2021
View reviewed changes
phalcon/Validation.zep Outdated Show resolved Hide resolved
Jeckerson
Jeckerson requested changes Nov 4, 2021
View reviewed changes
phalcon/Validation.zep Outdated Show resolved Hide resolved
phalcon/Forms/Form.zep Outdated Show resolved Hide resolved
phalcon/Validation.zep Outdated Show resolved Hide resolved
phalcon/Validation.zep Outdated Show resolved Hide resolved
@niden niden linked an issue Nov 5, 2021 that may be closed by this pull request
Form Validation and Entity are not playing well together #14203
Closed
niden
niden requested changes Nov 5, 2021
View reviewed changes
phalcon/Forms/Form.zep Outdated Show resolved Hide resolved
phalcon/Forms/Form.zep Outdated Show resolved Hide resolved
phalcon/Forms/Form.zep Outdated Show resolved Hide resolved
phalcon/Forms/Form.zep Show resolved Hide resolved
phalcon/Validation.zep Outdated Show resolved Hide resolved
@niden
Copy link
Member

niden commented Nov 5, 2021

Other than the above changes, this is looking good. If you can please add a test or two.

@BeMySlaveDarlin BeMySlaveDarlin force-pushed the issue/14203-fix-form-validator-get-value branch from f2c80dd to e975947 Compare November 7, 2021 07:18
@BeMySlaveDarlin
Copy link
Contributor Author

Fixed all comments. Rebased to 5.0.x resolving conflicts

@BeMySlaveDarlin BeMySlaveDarlin mentioned this pull request Nov 7, 2021
Merged
5 tasks
@BeMySlaveDarlin BeMySlaveDarlin removed a link to an issue Nov 7, 2021
Form Validation and Entity are not playing well together #14203
Closed
@BeMySlaveDarlin BeMySlaveDarlin linked an issue Nov 7, 2021 that may be closed by this pull request
Form Validation and Entity are not playing well together #14203
Closed
@BeMySlaveDarlin BeMySlaveDarlin force-pushed the issue/14203-fix-form-validator-get-value branch from fa8d741 to 6062138 Compare November 7, 2021 15:26
@BeMySlaveDarlin BeMySlaveDarlin marked this pull request as draft November 7, 2021 15:26
@BeMySlaveDarlin
Copy link
Contributor Author

BeMySlaveDarlin commented Nov 7, 2021 •
edited
Loading

Rebased due to fix conflicts
Draft until all tests passes

UPD: Success

@BeMySlaveDarlin BeMySlaveDarlin marked this pull request as ready for review November 7, 2021 15:53
@BeMySlaveDarlin BeMySlaveDarlin marked this pull request as draft November 7, 2021 16:27
@BeMySlaveDarlin BeMySlaveDarlin force-pushed the issue/14203-fix-form-validator-get-value branch from 39fd4b5 to f347536 Compare November 7, 2021 16:49
@BeMySlaveDarlin BeMySlaveDarlin force-pushed the issue/14203-fix-form-validator-get-value branch from f347536 to d03319f Compare November 7, 2021 17:13
@BeMySlaveDarlin BeMySlaveDarlin marked this pull request as ready for review November 7, 2021 17:51
@niden niden added bug A bug report status: medium Medium labels Nov 7, 2021
@niden niden merged commit bfccf72 into phalcon:5.0.x Nov 7, 2021
@niden
Copy link
Member

niden commented Nov 7, 2021

Thank you @BeMySlaveDarlin

@niden niden mentioned this pull request Nov 7, 2021
Closed
@BeMySlaveDarlin BeMySlaveDarlin deleted the issue/14203-fix-form-validator-get-value branch November 7, 2021 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@niden niden niden approved these changes

@Jeckerson Jeckerson Awaiting requested review from Jeckerson

Assignees

@BeMySlaveDarlin BeMySlaveDarlin

Labels
5.0 The issues we want to solve in the 5.0 release breaks bc Functionality that breaks Backwards Compatibility bug A bug report status: medium Medium
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Form Validation and Entity are not playing well together
3 participants
@BeMySlaveDarlin @niden @Jeckerson