-
Notifications
You must be signed in to change notification settings - Fork 17
Operations Status Search API Endpoint #12
Operations Status Search API Endpoint #12
Conversation
|
|
PR is ready |
|
@nuzil @vrann Place documentation files under here: https://github.com/magento/devdocs/tree/develop/guides/v2.3/rest If you have multiple |
| /** | ||
| * @api | ||
| */ | ||
| interface OperationSearchResultsInterface extends \Magento\Framework\Api\SearchResultsInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add description to all new classes/interfaces
| interface OperationSearchResultsInterface extends \Magento\Framework\Api\SearchResultsInterface | ||
| { | ||
| /** | ||
| * Get attributes list. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
attributes list => operations list ?
| $this->searchResultFactory = $searchResultFactory; | ||
| $this->joinProcessor = $joinProcessor; | ||
| $this->operationExtensionFactory = $operationExtension; | ||
| $this->collectionProcessor = $collectionProcessor ? : ObjectManager::getInstance() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a new class, what is the reason to use object manager (it is normally used for backward compatibility)?
| SearchResultFactory $searchResultFactory, | ||
| JoinProcessorInterface $joinProcessor, | ||
| OperationExtensionInterfaceFactory $operationExtension, | ||
| CollectionProcessorInterface $collectionProcessor = null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This argument should not be optional.
| if ($extensionAttributes == null) { | ||
| $extensionAttributes = $this->operationExtensionFactory->create(); | ||
| } | ||
| $extensionAttributes->setStartTime('dsadsa'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does 'dsadsa' mean?
| * @since 100.2.0 | ||
| */ | ||
| interface OperationInterface | ||
| interface OperationInterface extends \Magento\Framework\Api\ExtensibleDataInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory, OperationInterface must have getExtensionAttributes and setExtensionAttributes declared.
What is the need to move inheritance from ExtensibleDataInterface up to this interface?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other case when I call
$this->joinProcessor->process($collection, \Magento\AsynchronousOperations\Api\Data\OperationInterface::class);
in Magento\AsynchronousOperations\Model\OperationRepository::getList()
I get logic exception from Magento\Framework\Api\ExtensionAttributesFactory::getExtensibleInterfaceName($extensibleClassName) because Magento\AsynchronousOperations\Api\Data\OperationInterface and parent interfaces doesn't extend \Magento\Framework\Api\ExtensibleDataInterface
[Backport] #13899 Solve Canada Zip Code pattern
…erface, remove meaningless code
[Forwardport] Corrected function comment
Allow to receive all bulk operations by search criteria.
Description
https://github.com/magento-engcom/bulk-api/wiki/Operations-Status-Search-API-Endpoint
Contribution checklist