Skip to content

Fixed issue regarding widget form validation #23499

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
Closed
wants to merge 23 commits into from
Closed

Fixed issue regarding widget form validation #23499

wants to merge 23 commits into from

Conversation

yash7690
Copy link

@yash7690 yash7690 commented Jun 29, 2019 •
edited by sdzhepa
Loading

Description (*)

-> Have added attribute to provide additional_classes in xml file, so that field in widget can have validation classes like integer, positive-integer, validate-zero-or-greater etc
-> Have created a new validation rule for "Not negative integer" validation

Fixed Issues (if relevant)

Widget form was not having any validation apart from required. Have added additional_classes attribute to field so that any class/validation can apply.

  1. Fixes Usage of non-numeric value for "Number of Products to Display" setting in "Catalog Products List" widget leads to an exception #23422: Usage of non-numeric value for "Number of Products to Display" setting in "Catalog Products List" widget leads to an exception

Manual testing scenarios (*)

  1. Manually Tested

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

@m2-assistant
Copy link

m2-assistant bot commented Jun 29, 2019

Hi @yash7690. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Jul 1, 2019 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@yash7690
Copy link
Author

yash7690 commented Jul 3, 2019

@magento-cicd2 , This commit shows me error of sematic version failure and asks me to update minor version of module. But the confusion is, the widget module does not have version in either composer.json or module.xml, so how can i solve this error ?

@yash7690
Copy link
Author

@magento-engcom-team Please let me know solution for my query.

sidolov
sidolov requested changes Aug 15, 2019
View reviewed changes
app/code/Magento/Widget/Block/Adminhtml/Widget/Options.php Outdated
* @param \Magento\Framework\DataObject $parameter
* @return string
*/
protected function getCssClasses($parameter)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, make method private and SVC build failures will be resolved

Copy link
Author

Choose a reason for hiding this comment

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

I have made the changes suggested by you, but still it shows build failed. This time, it is showing that array_merge is being used in a forloop, which is right but actually it is not my change, default extension is having the same code. Can you please advice here what can be done ?

@ghost ghost assigned sidolov Aug 15, 2019
sidolov
sidolov requested changes Sep 4, 2019
View reviewed changes
@ghost ghost added the Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround. label May 11, 2020
VladimirZaets
VladimirZaets previously approved these changes Jul 28, 2020
View reviewed changes
@ghost ghost assigned VladimirZaets Jul 28, 2020
@magento-engcom-team magento-engcom-team added Partner: Krish TechnoLabs partners-contribution Pull Request is created by Magento Partner labels Jul 28, 2020
@magento-engcom-team
Copy link
Contributor

Hi @VladimirZaets, thank you for the review.
ENGCOM-7905 has been created to process this Pull Request
✳️ @VladimirZaets, could you please add one of the following labels to the Pull Request?

Label Description
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests

@engcom-Alfa engcom-Alfa self-assigned this Jul 30, 2020
engcom-Alfa
engcom-Alfa requested changes Jul 30, 2020
View reviewed changes
Copy link
Contributor

@engcom-Alfa engcom-Alfa left a comment

Choose a reason for hiding this comment

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

Hi @yash7690 . We have some problems with checkout to your branch. Could you update it?

@ghost ghost dismissed VladimirZaets’s stale review July 30, 2020 11:18

Pull Request state was updated. Re-review required.

@VladimirZaets
Copy link
Contributor

@magento run all tests

@sinhaparul sinhaparul removed the request for review from DrewML March 28, 2023 18:37
@engcom-Dash
Copy link
Contributor

Hello @yash7690,

Thanks for your contributions!

Please resolve the conflict so we can proceed with this PR.

Thanks!

@engcom-Dash
Copy link
Contributor

Closing this PR since it has not been updated in a while. Please feel free to reopen if needed.

@engcom-Dash engcom-Dash closed this Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@engcom-Alfa engcom-Alfa engcom-Alfa requested changes

@VladimirZaets VladimirZaets VladimirZaets left review comments

@kokoc kokoc Awaiting requested review from kokoc

@melnikovi melnikovi Awaiting requested review from melnikovi

@sidolov sidolov Awaiting requested review from sidolov

Assignees

@VladimirZaets VladimirZaets

@sidolov sidolov

@engcom-Alfa engcom-Alfa

@engcom-Echo engcom-Echo

Labels
Area: Lib/Frontend Component: CatalogWidget Component: Widget Partner: Krish TechnoLabs partners-contribution Pull Request is created by Magento Partner Priority: P3 May be fixed according to the position in the backlog. Release Line: 2.4 Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Usage of non-numeric value for "Number of Products to Display" setting in "Catalog Products List" widget leads to an exception
10 participants
@yash7690 @magento-cicd2 @sidolov @engcom-Echo @magento-engcom-team @VladimirZaets @engcom-Dash @engcom-Alfa @lenaorobei @yash-magedelight