Skip to content

Resolve Console error when clicking select all at "Newsletter Problems Report" (issue 24102) #24104

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

Conversation

edenduong
Copy link
Contributor

@edenduong edenduong commented Aug 11, 2019

Description (*)

  1. Resolve Console error when clicking checkbox at "Newsletter Problems Report" #24102: Console error when clicking select all at "Newsletter Problems Report"

=> Can not select all

Solution:

  • The selector is wrong , so need to change selector

Fixed Issues (if relevant)

  1. Console error when clicking checkbox at "Newsletter Problems Report" #24102: Console error when clicking select all at "Newsletter Problems Report"

Manual testing scenarios (*)

  1. Go to Backend
  2. Report -> Newsletter Problems Report
  3. Make sure no record in the grid. Check the select all

=> No console error

image

Questions or comments

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 Aug 11, 2019

Hi @edenduong. 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

In case you'd like to rerun tests use the following comments:

  • @magento run all tests - run all tests
  • @magento run { check name } - run a single test job (check name example: Static Tests build)

For more details, please, review the Magento Contributor Guide documentation.

Copy link
Contributor

@Stepa4man Stepa4man left a comment

Choose a reason for hiding this comment

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

Hi, @edenduong
Thanks for your contribution!

@magento-engcom-team
Copy link
Contributor

Hi @Stepa4man, thank you for the review.
ENGCOM-5590 has been created to process this Pull Request
✳️ @Stepa4man, 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
Copy link
Contributor

Hi @edenduong !

During testing we faced an issue.

I don't think that checkbox is useless, because when you have some problem reports in the grid, "Unsubscribe Selected" and "Delete Selected Problems" buttons appears. I think this buttons related with checkbox in the grid.
after

@edenduong Could you take a look, please?

Thanks!

@edenduong
Copy link
Contributor Author

@engcom-Alfa : Thank you for your reply. I see. I will check it again. Because it has the Console Error when you click the checkbox (when no record).

@edenduong edenduong changed the title Resolve Useless "Checkbox" at "Newsletter Problems Report", console error when clicking (issue 24102) Resolve Console error when clicking checkbox at "Newsletter Problems Report" (issue 24102) Aug 14, 2019
@edenduong edenduong force-pushed the 2.3-bugfix/useless_checkbox_error_report_issue24102 branch from f6935d3 to d7606f4 Compare August 14, 2019 15:51
@edenduong
Copy link
Contributor Author

@engcom-Alfa: I have changed the purpose of this PR. I changed the source code to fix the problem about "Console error when clicking checkbox at "Newsletter Problems Report" (If grid has no record)".
@Stepa4man : Please review my new changes . Thank you very much :)

@magento-engcom-team
Copy link
Contributor

Hi @sidolov, thank you for the review.
ENGCOM-5590 has been created to process this Pull Request
✳️ @sidolov, 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
Copy link
Contributor

Hi @edenduong

When we have some records, console errors occurs again

Actual Result:
after2

@edenduong Could you take a look, please?

Thanks!

@edenduong
Copy link
Contributor Author

@engcom-Alfa: I fixed it. Please check it again. Now the "select all" can run normally (it can not run before because of the Javascript errors)
@Stepa4man : Please review my new changes . Thank you very much :)

@Stepa4man
Copy link
Contributor

@magento run all tests

@engcom-Alfa engcom-Alfa requested a review from Stepa4man August 23, 2019 12:46
@edenduong
Copy link
Contributor Author

@Stepa4man , @sidolov : Could you please remove the "Need Update" so it can move to the "Testing" step? Thanks!

@magento-engcom-team
Copy link
Contributor

Hi @sidolov, thank you for the review.
ENGCOM-5590 has been created to process this Pull Request
✳️ @sidolov, 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

@edenduong edenduong changed the title Resolve Console error when clicking checkbox at "Newsletter Problems Report" (issue 24102) Resolve Console error when clicking select all at "Newsletter Problems Report" (issue 24102) Aug 27, 2019
@engcom-Foxtrot engcom-Foxtrot self-assigned this Aug 27, 2019
magento-engcom-team pushed a commit that referenced this pull request Aug 27, 2019
@magento-engcom-team magento-engcom-team merged commit 86d5c22 into magento:2.3-develop Aug 27, 2019
@m2-assistant
Copy link

m2-assistant bot commented Aug 27, 2019

Hi @edenduong, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@magento-engcom-team magento-engcom-team added this to the Release: 2.3.4 milestone Aug 27, 2019
@sidolov sidolov added the Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests label Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Console error when clicking checkbox at "Newsletter Problems Report"
6 participants