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

[Customer] Removing the delete buttons for default customer groups #26251

Merged
merged 4 commits into from
Jan 9, 2020
Merged

[Customer] Removing the delete buttons for default customer groups #26251

merged 4 commits into from
Jan 9, 2020

Conversation

eduard13
Copy link
Contributor

@eduard13 eduard13 commented Jan 3, 2020 •
edited
Loading

Description (*)

This PR removes the listing Delete button for Default Customer Group, as far as we aren't able to remove it.

Fixed Issues (if relevant)

  1. Navigate to Customers / Customer Groups
  2. Try to delete the default General group
  3. You'll get Cannot delete group error message

Manual testing scenarios (*)

  1. Navigate to Stores / Configuration / Customer Configuration / Create New Account Options
  2. Check what is the Default Group -> General
  3. Navigate to Customers / Customer Groups
  4. The Delete action shouldn't be available for General group.
    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)

@eduard13 eduard13 requested a review from paliarush as a code owner January 3, 2020 13:10
@m2-assistant
Copy link

m2-assistant bot commented Jan 3, 2020

Hi @eduard13. 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.4-develop instance - deploy vanilla Magento instance

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

@magento-engcom-team magento-engcom-team added Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Jan 3, 2020
@dmytro-ch dmytro-ch self-assigned this Jan 3, 2020
@dmytro-ch dmytro-ch added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Jan 3, 2020
lbajsarowicz
lbajsarowicz suggested changes Jan 3, 2020
View reviewed changes
Copy link
Contributor

@lbajsarowicz lbajsarowicz left a comment

Choose a reason for hiding this comment

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

Wow! Really great job done. Could you only replace magic numbers with more meaningful const?

app/code/Magento/Customer/Test/Unit/Ui/Component/Listing/Column/GroupActionsTest.php Outdated
->method('getUrl')
->willReturnMap(
[
['customer/group/edit', ['id' => 1], 'http://magento.com/customer/group/edit'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace magic numbers with const named for example - STUB_LOGGEDIN_CUSTOMER_GROUP_ID
Instead of literal with sample domain - try to also replace it with const - eg. STUB_GROUP_EDIT_URL

app/code/Magento/Customer/Test/Unit/Ui/Component/Listing/Column/GroupActionsTest.php Outdated
->method('getUrl')
->willReturnMap(
[
['customer/group/edit', ['id' => 1], 'http://magento.com/customer/group/edit'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace magic numbers with const named for example - STUB_LOGGEDIN_CUSTOMER_GROUP_ID
Instead of literal with sample domain - try to also replace it with const - eg. STUB_GROUP_EDIT_URL

app/code/Magento/Customer/Test/Unit/Ui/Component/Listing/Column/GroupActionsTest.php Outdated
->willReturnMap(
[
['customer/group/edit', ['id' => 1], 'http://magento.com/customer/group/edit'],
['customer/group/edit', ['id' => 0], 'http://magento.com/customer/group/edit']
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace magic numbers with const named for example - STUB_GUEST_CUSTOMER_GROUP_ID

app/code/Magento/Customer/Test/Unit/Ui/Component/Listing/Column/GroupActionsTest.php Outdated
[
[
[
'customer_group_id' => 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

Give the magic number a meaning - like const named for example - STUB_LOGGEDIN_CUSTOMER_GROUP_ID

app/code/Magento/Customer/Test/Unit/Ui/Component/Listing/Column/GroupActionsTest.php Outdated
false,
[
[
'customer_group_id' => 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

Give the magic number a meaning - like const named for example - STUB_LOGGEDIN_CUSTOMER_GROUP_ID

@ghost ghost assigned lbajsarowicz Jan 3, 2020
@eduard13
Copy link
Contributor Author

eduard13 commented Jan 8, 2020

Hi @lbajsarowicz, thank you for reviewing this one, could you please check the latest changes?
Thank you.

lbajsarowicz
lbajsarowicz approved these changes Jan 8, 2020
View reviewed changes
app/code/Magento/Customer/Test/Unit/Ui/Component/Listing/Column/GroupActionsTest.php
['customer/group/edit', ['id' => 0], 'http://magento.com/customer/group/edit']
[
'customer/group/edit',
[
Copy link
Contributor

Choose a reason for hiding this comment

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

This one could be just onliner:

['id' => static::STUB_GENERAL_CUSTOMER_GROUP_ID]

But it's minor stuff 👍

@magento-engcom-team
Copy link
Contributor

Hi @lbajsarowicz, thank you for the review.
ENGCOM-6553 has been created to process this Pull Request

@engcom-Alfa engcom-Alfa self-assigned this Jan 8, 2020
@engcom-Alfa
Copy link
Contributor

✔️ QA Passed

@engcom-Golf engcom-Golf self-assigned this Jan 8, 2020
magento-engcom-team pushed a commit that referenced this pull request Jan 9, 2020
@VladimirZaets
ENGCOM-6553: [Customer] Removing the delete buttons for default custo…
d23f88c 
…mer groups #26251
@magento-engcom-team magento-engcom-team merged commit 5d0e12b into magento:2.4-develop Jan 9, 2020
@m2-assistant
Copy link

m2-assistant bot commented Jan 9, 2020

Hi @eduard13, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lbajsarowicz lbajsarowicz lbajsarowicz approved these changes

@paliarush paliarush Awaiting requested review from paliarush

Assignees

@lbajsarowicz lbajsarowicz

@dmytro-ch dmytro-ch

@engcom-Alfa engcom-Alfa

@engcom-Golf engcom-Golf

Labels
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Component: Customer Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner Progress: accept Release Line: 2.4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@eduard13 @magento-engcom-team @engcom-Alfa @lbajsarowicz @dmytro-ch @engcom-Golf