[Widgets] Mass-action delete for Widgets #20765

vasilii-b · 2019-01-29T10:35:23Z

Description (*)

This PR adds massaction delete to Widget Instances

Fixed Issues (if relevant)

[Widgets] Bulk delete is not available #20764: [Widgets] Bulk delete is not available

Manual testing scenarios (*)

Provided in issue description

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 on Travis CI are green)

magento-engcom-team · 2019-01-29T10:35:27Z

Hi @vasilii-b. 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-engcom-team give me test instance - deploy test instance based on PR changes
@magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

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

rogyar · 2019-01-29T14:50:22Z

app/code/Magento/Widget/Controller/Adminhtml/Widget/Instance/MassDelete.php

+        if (count($notDeletedInstances)) {
+            $this->messageManager->addErrorMessage(__(
+                'Widget(s) with ID(s) %1 were not found',
+                implode(',', $notDeletedInstances)


Please add an additional space after the comma in the implode functions. So we have in result 1, 2, 30 instead of 1,2,30

rogyar · 2019-01-29T14:53:03Z

app/code/Magento/Widget/Model/DeleteWidgetInstanceById.php

+    public function execute(int $id)
+    {
+        $model = $this->getById($id);
+        var_dump($model->getData(), $id);


I believe it should not be here :)

rogyar · 2019-01-29T14:54:09Z

app/code/Magento/Widget/Model/DeleteWidgetInstanceById.php

+     * @return WidgetInstance
+     * @throws NoSuchEntityException
+     */
+    private function getById(int $id)


Could you rename it to getWidgetById, please?

rogyar · 2019-01-29T15:02:40Z

app/code/Magento/Widget/Api/DeleteWidgetInstanceByIdInterface.php

+ * Command to delete a widget instance by specified widget instance ID
+ * @api
+ */
+interface DeleteWidgetInstanceByIdInterface


I have doubts regarding a necessity to use an API interface in this particular case. It might be useful if we create an additional web API endpoint or we need to define a strategy in an interface for multiple implementations. But it's not the case.
The main downside of using an interface here - it does not make much sense. In the future, the interface restricts us in possible architectural changes (if the implementation needs to be changed). So if we declare an interface we are responsible for its support. On the other hand, if we don't have an interface we can change the implementation in more flexible ways.
So the main idea is to use API interfaces only in cases when they are useful but not harmful.

Got it! I'll adjust it. Thank you!

vasilii-b · 2019-01-30T15:34:50Z

Hi @rogyar, can you please have a look on added changes? Thank you!

rogyar · 2019-02-05T14:06:07Z

app/code/Magento/Widget/Model/DeleteWidgetInstanceById.php

+/**
+ * Class DeleteWidgetInstanceById
+ */
+class DeleteWidgetInstanceById


Please, rename it to DeleteWidgetById

rogyar · 2019-02-05T14:06:48Z

app/code/Magento/Widget/Model/DeleteWidgetInstanceById.php

@@ -0,0 +1,73 @@
+<?php


Please, use strict_types declaration in new implementations

app/code/Magento/Widget/Controller/Adminhtml/Widget/Instance/MassDelete.php

rogyar · 2019-02-05T14:07:42Z

app/code/Magento/Widget/etc/di.xml

@@ -34,4 +34,6 @@
        <plugin name="widget-layout-update-plugin"
                type="Magento\Widget\Model\ResourceModel\Layout\Plugin" sortOrder="10"/>
    </type>
+    <preference for="Magento\Widget\Api\DeleteWidgetInstanceByIdInterface"


I believe we don't need this config anymore

sidolov · 2019-02-19T17:58:58Z

@vasilii-b , I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for the collaboration!

ghost · 2019-02-19T17:59:51Z

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

…lk-delete * origin/2.3-develop: (4954 commits) magento#23427 Fix static magento#23532 Fix-static-tests magento/magento#23500 fix funk tests fix validation class for max-words MC-17922: Date in filter area doesn't match with Active filters bar Fixed coding standards violations Updated unit test MC-17922: Date in filter area doesn't match with Active filters bar Update Framework/Mail::Message to send all emails as MIME, not just HTML emails. MC-17922: Date in filter area doesn't match with Active filters bar Removed editors from all dependable customer billing address fields Correct array type hints Fix mass detection issue Testing configurable option update based on product and attribute_id move breakpoint by -1px to make nav work correctly at viweport width of 768px MC-17275: Admin Down after changing store domain/cookie Setting right shipper street1 in addressline1 MC-17275: Admin Down after changing store domain/cookie Replaced urldecode with rawurldecode that not converting + into space Fixed issue magento#23383 ...

# Fixed codestyle and added improvements requested by maintainer

m2-assistant · 2019-07-10T13:19:13Z

Hi @vasilii-b. 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-engcom-team · 2019-08-13T19:36:48Z

Hi @sidolov, thank you for the review.
ENGCOM-5614 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-Delta · 2019-08-20T11:12:59Z

✔️ QA passed

…bulk-delete

m2-assistant · 2019-08-30T17:35:07Z

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

Vasilii added 2 commits January 29, 2019 10:51

Added massaction and delete option to admin widget instances page

073121a

Added possibility to mass-delete widget instances

4376b91

vasilii-b assigned rogyar Jan 29, 2019

vasilii-b requested a review from rogyar January 29, 2019 10:35

magento-engcom-team added Area: Frontend Component: Widget Release Line: 2.3 Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Jan 29, 2019

rogyar requested changes Jan 29, 2019

View reviewed changes

rogyar added the Progress: needs update label Jan 29, 2019

Added adjustments as per maintainer's code review

7d92ade

vasilii-b removed the Progress: needs update label Feb 1, 2019

rogyar requested changes Feb 5, 2019

View reviewed changes

rogyar added the Progress: needs update label Feb 5, 2019

sidolov closed this Feb 19, 2019

Vasilii Burlacu added 2 commits July 10, 2019 15:35

magento/magento:20764 - Mass-action delete for Widgets

2fd6ef1

# Fixed codestyle and added improvements requested by maintainer

vasilii-b reopened this Jul 10, 2019

ghost unassigned rogyar Jul 10, 2019

Add void strict type to new model

8225526

sidolov approved these changes Aug 13, 2019

View reviewed changes

ghost assigned sidolov Aug 13, 2019

ghost added Progress: ready for testing and removed Progress: needs update labels Aug 13, 2019

engcom-Delta self-assigned this Aug 20, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels Aug 20, 2019

ghost added Progress: extended testing and removed Progress: testing in progress labels Aug 20, 2019

engcom-Foxtrot self-assigned this Aug 20, 2019

engcom-Foxtrot and others added 2 commits August 20, 2019 16:45

magento#20765: Static test fix.

8746079

Merge remote-tracking branch 'mainline/2.3-develop' into 2.3-widgets-…

b56c3c4

…bulk-delete

ghost added Progress: accept and removed Progress: extended testing labels Aug 21, 2019

magento-engcom-team mentioned this pull request Aug 30, 2019

[Widgets] Bulk delete is not available #20764

Closed

magento-engcom-team pushed a commit that referenced this pull request Aug 30, 2019

ENGCOM-5614: [Widgets] Mass-action delete for Widgets #20765

a5a07b8

magento-engcom-team merged commit b56c3c4 into magento:2.3-develop Aug 30, 2019

magento-engcom-team added this to the Release: 2.3.4 milestone Aug 30, 2019

sidolov added the Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests label Sep 14, 2019

[Widgets] Mass-action delete for Widgets #20765

[Widgets] Mass-action delete for Widgets #20765

Uh oh!

Conversation

vasilii-b commented Jan 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Contribution checklist (*)

Uh oh!

magento-engcom-team commented Jan 29, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vasilii-b commented Jan 30, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sidolov commented Feb 19, 2019

Uh oh!

ghost commented Feb 19, 2019

Uh oh!

m2-assistant bot commented Jul 10, 2019

Uh oh!

magento-engcom-team commented Aug 13, 2019

Uh oh!

engcom-Delta commented Aug 20, 2019

Uh oh!

m2-assistant bot commented Aug 30, 2019

Uh oh!

Uh oh!

vasilii-b commented Jan 29, 2019 •

edited

Loading