The user passed for async api referenced correct WebAPI user #22

nuzil · 2018-05-18T21:12:44Z

https://app.zenhub.com/workspace/o/magento/bulk-api-ce/issues/14

Drop foreign key referencing admin_user table from bulk operations. Pass the Web API user id taken from the context object to MassScheduler

…ass the Web API user id taken from the context object to MassScheduler

magento-cicd2 · 2018-05-18T21:12:47Z

All committers have signed the CLA.

magento-cicd2 · 2018-05-18T21:12:47Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

vrann · 2018-05-19T11:18:36Z

@nuzil @anvasiliev can you please fix travis and sign CLA

paliarush · 2018-05-23T15:12:03Z

app/code/Magento/AsynchronousOperations/Model/MassSchedule.php

@@ -95,6 +103,10 @@ public function publishMass($topicName, array $entitiesArray, $groupId = null, $
    {
        $bulkDescription = __('Topic %1', $topicName);

+        if ($userId == null) {
+            $userId = $this->userContext->getUserId();


User ID is not unique identifier and should be used in conjunction with user type.

There could be integration, admin and customer with the same ID. I believe bulk operation could at least be scheduled by the admin or integration.

you are right the user ID is not unique.

@anvasiliev @nuzil How do you think if we save the user type?

Hi @paliarush , @vrann
We though about 2 possible solutions, maybe we have to discuss it separately:

We will add prefixes to users, like "admin_ID, customer_ID" .....

We will add new column to this table with user_type.

What do you think?

@munkhulzii @nuzil it would be a cleaner solution if we add another column, with the default value set to the admin. Otherwise it will be backward incompatible with the rest of the system

vrann · 2018-06-15T16:35:39Z

app/code/Magento/AsynchronousOperations/Model/MassSchedule.php

@@ -95,6 +103,10 @@ public function publishMass($topicName, array $entitiesArray, $groupId = null, $
    {
        $bulkDescription = __('Topic %1', $topicName);

+        if ($userId == null) {
+            $userId = $this->userContext->getUserId();


@munkhulzii @nuzil it would be a cleaner solution if we add another column, with the default value set to the admin. Otherwise it will be backward incompatible with the rest of the system

paliarush · 2018-08-31T18:13:58Z

app/code/Magento/AsynchronousOperations/Api/Data/BulkSummaryInterface.php

+
+    /**
+     * Set user type
+     * @param int $operationCount


Looks like copy-paste

paliarush · 2018-08-31T18:16:36Z

app/code/Magento/AsynchronousOperations/Test/Unit/Model/BulkManagementTest.php

@@ -108,6 +108,7 @@ public function testScheduleBulk()
        $bulkUuid = 'bulk-001';
        $description = 'Bulk summary description...';
        $userId = 1;
+        $userType = 2;


User types are defined in \Magento\Authorization\Model\UserContextInterface as constants. Please use one of those constants to make code cleaner.

vkublytskyi · 2018-09-13T14:14:47Z

app/code/Magento/AsynchronousOperations/Model/BulkManagement.php

@@ -73,6 +79,7 @@ public function __construct(
        BulkPublisherInterface $publisher,
        MetadataPool $metadataPool,
        ResourceConnection $resourceConnection,
+        UserContextInterface $userContext,


This constructor exists in Magento Commerce 2.2 so new parameters should be added corresponding to backward compatibility policies (at the end as optional)

vkublytskyi · 2018-09-13T14:15:34Z

app/code/Magento/AsynchronousOperations/Model/MassSchedule.php

@@ -70,14 +76,16 @@ public function __construct(
        AsyncResponseInterfaceFactory $asyncResponseFactory,
        BulkManagementInterface $bulkManagement,
        LoggerInterface $logger,
-        OperationRepository $operationRepository
+        OperationRepository $operationRepository,
+        UserContextInterface $userContext


This constructor exists in Magento Commerce 2.2 so new parameters should be added corresponding to backward compatibility policies (as optional)

Requested changes are provided

- applied backward compatibility policy to modified constructors

vkublytskyi

Before merge following issues should be resolved:

The class MassSchedule has a coupling between objects value of 13. Consider to reduce the number of dependencies under 13.
During upgrade foreign key MAGENTO_BULK_USER_ID_ADMIN_USER_USER_ID is not dropped for Magento Commerce installation.

rganin · 2018-10-05T20:32:57Z

app/code/Magento/AsynchronousOperations/etc/db_schema_whitelist.json

@@ -1,47 +1,50 @@
 {
-    "magento_bulk": {


Please keep 4 spaces intendation for whitelists, changes are very hard to read.

rganin · 2018-10-05T20:48:17Z

lib/internal/Magento/Framework/Setup/Declaration/Schema/Diff/TableDiff.php

@@ -177,7 +182,9 @@ private function calculateDiff(ElementInterface $declaredTable, ElementInterface
            $declaredElements = $declaredTable->getElementsByType($elementType);

            if ($elementType === self::INDEX_DIFF_TYPE) {
-                $generatedElements = $this->excludeAutoIndexes($generatedTable, $generatedElements);
+                // We should exclude from generated indexes only these auto indexes that correspond to
+                // declared constraint. If constraint is not declared we should clean up index created for it.


What is the purpose of this change?
It causes a case when autogenerated index will be implicitly removed from declaration so removing FK with preserving index for performance purposes won't work.

If you need to remove index use declaration with 'disable="1"' attribute.

…user #22

Munkh-Ulzii Balidar added 2 commits May 12, 2018 13:28

Drop foreign key referencing admin_user table from bulk operations. P…

0748ada

…ass the Web API user id taken from the context object to MassScheduler

Drop foreign key referencing admin_user table from bulk operations. P…

55fb636

…ass the Web API user id taken from the context object to MassScheduler

nuzil mentioned this pull request May 18, 2018

The user passed for async api should reference WebAPI user #14

Closed

paliarush suggested changes May 23, 2018

View reviewed changes

vrann previously requested changes Jun 15, 2018

View reviewed changes

Munkh-Ulzii Balidar added 4 commits August 29, 2018 09:54

add user type for asynchronous operations

7bf456a

add user type of bulk operation

9d112a6

merge 2.3-develop

9e56563

remove empty line from interface

9724202

paliarush reviewed Aug 31, 2018

View reviewed changes

Munkh-Ulzii Balidar added 2 commits September 5, 2018 11:48

add user type for asynchronous operations

73d1e38

remove redundant dependency

614ed8a

vkublytskyi self-assigned this Sep 12, 2018

vkublytskyi suggested changes Sep 13, 2018

View reviewed changes

#14: The user passed for async api should reference WebAPI user

1acda9b

- applied backward compatibility policy to modified constructors

magento-engcom-team added this to the Release: 2.3.0 milestone Sep 14, 2018

magento-engcom-team added Release Line: 2.3 Partner: Comwrap partners-contribution Component: AsynchronousOperations labels Sep 14, 2018

Merge mainline

3d1e389

vkublytskyi suggested changes Sep 14, 2018

View reviewed changes

vkublytskyi changed the base branch from 2.3-develop to 2.3.0-release October 2, 2018 10:56

magento-engcom-team modified the milestones: Release: 2.3.0, Release: 2.3.1 Oct 4, 2018

magento-engcom-team added the Component: Framework/Setup label Oct 4, 2018

rganin reviewed Oct 5, 2018

View reviewed changes

vkublytskyi force-pushed the comwrap-contribution-day branch 4 times, most recently from 5d2de54 to 918f8ae Compare October 8, 2018 15:41

Volodymyr Kublytskyi added 2 commits October 8, 2018 23:49

Declare index for schema consistency

3aa2955

Fixed code style

f708896

vkublytskyi force-pushed the comwrap-contribution-day branch from 918f8ae to f708896 Compare October 8, 2018 20:49

vkublytskyi approved these changes Oct 10, 2018

View reviewed changes

vkublytskyi merged commit f708896 into 2.3.0-release Oct 10, 2018

vkublytskyi pushed a commit that referenced this pull request Oct 10, 2018

ENGCOM-2976: The user passed for async api referenced correct WebAPI …

f20a58c

…user #22

magento-engcom-team added the Award: complex label Oct 10, 2018

vkublytskyi mentioned this pull request Oct 10, 2018

Add topic description feature #30

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The user passed for async api referenced correct WebAPI user #22

The user passed for async api referenced correct WebAPI user #22

nuzil commented May 18, 2018

magento-cicd2 commented May 18, 2018 •

edited

Loading

magento-cicd2 commented May 18, 2018

vrann commented May 19, 2018

paliarush May 23, 2018

munkhulzii May 29, 2018

nuzil May 29, 2018

vrann Jun 15, 2018

vrann Jun 15, 2018

paliarush Aug 31, 2018

paliarush Aug 31, 2018

vkublytskyi Sep 13, 2018

vkublytskyi Sep 13, 2018

vkublytskyi left a comment

rganin Oct 5, 2018

rganin Oct 5, 2018

rganin Oct 5, 2018

The user passed for async api referenced correct WebAPI user #22

The user passed for async api referenced correct WebAPI user #22

Conversation

nuzil commented May 18, 2018

magento-cicd2 commented May 18, 2018 • edited Loading

magento-cicd2 commented May 18, 2018

vrann commented May 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vkublytskyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magento-cicd2 commented May 18, 2018 •

edited

Loading