[Con 4873] Enable "Export All" button to export lots of products #399
Conversation
…oducts in batches
…oducts in batches
1 similar comment
.psh.yml
Outdated
| DB_USER: "shopware" | ||
| DB_PASSWORD: "shopware" | ||
| DB_HOST: "localhost" | ||
| DB_USER: "app" |
There was a problem hiding this comment.
i think this is special for your setup in docker?
i guess it wouldn't work for everybody else
Components/Helper.php
Outdated
| Shopware()->Session()->connectReservation = null; | ||
| } | ||
|
|
||
| public function getAllArticleSourceIds($offset, $batchSize) |
There was a problem hiding this comment.
why you need this?
i would suggest to put the repo as a dependency where this function is used
if this is used in a controller please add a proper service function
Imho helper classes are an antipattern
dataflow should be controller->service->repo
Also dochints are missing
Components/Helper.php
Outdated
| * @return array | ||
| */ | ||
| public function getAllNonConnectArticleIds() | ||
| public function getBatchOfArticles($offset, $batchSize) |
There was a problem hiding this comment.
same as above put this in aproper service function don't overload the helper
and also no dochints
| return array_map(function ($row) { | ||
| return $row['article_id']; | ||
| }, $result); | ||
| public function getLocalArticleCount() |
| */ | ||
| public function getArticleCountAction() | ||
| { | ||
| try { |
There was a problem hiding this comment.
call a service method here
| offset = parseInt(offset); | ||
| batchSize = parseInt(batchSize); | ||
| count = parseInt(count); | ||
| // if ((offset+batchSize) > count ){ |
There was a problem hiding this comment.
please remove unused outcommented code
|
|
||
| /** | ||
| * Contains the amount of products export all will export as batches | ||
| */ |
There was a problem hiding this comment.
why a global variable == global state? can you refactor it so that this is passed as argument
|
|
||
| /** | ||
| * Contains the amount of products export all will export as batches | ||
| */ |
There was a problem hiding this comment.
same here and above exportAll too
|
|
||
| Ext.create('Shopware.apps.Connect.view.export.product.manyProductsDialog', { | ||
| sourceIds: operation.sourceIds | ||
| exportAll: true, |
There was a problem hiding this comment.
Imho these bool switches are also bad practice
maybe we can find a cleaner solution here
| me.cancelButton.setDisabled(true); | ||
|
|
||
| me.fireEvent('startExport', me.sourceIds, me.batchSize, me); | ||
| if (me.exportAll) { |
There was a problem hiding this comment.
i think the progress bar shouldn't fire any event at all
it should just show some progress that will be updated but shouldn't know anything more
i would suggest firing this event after creating this progess bar
…ssbar window firing the export event
Implemented the "export all action" as a batch process to handle big amounts without gui tmeouts