-
Notifications
You must be signed in to change notification settings - Fork 1
FRW-10501 Improve publish product logic based on timestamps. #96
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.
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
Changes from all commits
a654c77
f9e72b6
71c368e
e66dd55
fa2508c
3e6709c
fed1563
24df6a4
988d590
6cc5382
c3ba41f
13e66fb
1ad357c
ec32d56
c4e2738
f456a0b
7d3cfdd
012dd38
f576ba3
260a202
068dbd9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,10 @@ | |
|
|
||
| namespace Spryker\Zed\EventBehavior\Business\Model; | ||
|
|
||
| use Generated\Shared\Transfer\EventEntityTransfer; | ||
| use Generated\Shared\Transfer\HydrateEventsRequestTransfer; | ||
| use Generated\Shared\Transfer\HydrateEventsResponseTransfer; | ||
|
|
||
| class EventEntityTransferFilter implements EventEntityTransferFilterInterface | ||
| { | ||
| /** | ||
|
|
@@ -145,6 +149,93 @@ public function getEventTransfersAdditionalValues(array $eventTransfers, string | |
| return array_unique($additionalValues); | ||
| } | ||
|
|
||
| /** | ||
| * @param \Generated\Shared\Transfer\HydrateEventsRequestTransfer $hydrateEventsRequestTransfer | ||
| * | ||
| * @return \Generated\Shared\Transfer\HydrateEventsResponseTransfer | ||
| */ | ||
| public function hydrateEventDataTransfer(HydrateEventsRequestTransfer $hydrateEventsRequestTransfer): HydrateEventsResponseTransfer | ||
gechetspr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| $hydrateEventsResponseTransfer = new HydrateEventsResponseTransfer(); | ||
| $eventEntityTransfers = $hydrateEventsRequestTransfer->getEventEntities()->getArrayCopy(); | ||
|
|
||
| if (count($eventEntityTransfers) === 0) { | ||
| return $hydrateEventsResponseTransfer; | ||
| } | ||
| usort($eventEntityTransfers, fn (EventEntityTransfer $a, EventEntityTransfer $b) => $a->getTimestamp() <=> $b->getTimestamp()); | ||
|
|
||
| $hydrateEventsResponseTransfer->setIdTimestampMap($this->getIdsTimestampMap($eventEntityTransfers)); | ||
|
|
||
| if ($hydrateEventsRequestTransfer->getAdditionalValueName()) { | ||
| $hydrateEventsResponseTransfer->setAdditionalValueTimestampMap($this->getAdditionalValueTimestampMap($eventEntityTransfers, $hydrateEventsRequestTransfer->getAdditionalValueName())); | ||
| } | ||
|
|
||
| if ($hydrateEventsRequestTransfer->getForeignKeyName()) { | ||
| $hydrateEventsResponseTransfer->setForeignKeyTimestampMap($this->getForeignKeyTimestampMap($eventEntityTransfers, $hydrateEventsRequestTransfer->getForeignKeyName())); | ||
| } | ||
|
|
||
| return $hydrateEventsResponseTransfer; | ||
| } | ||
|
|
||
| /** | ||
| * @param array<\Generated\Shared\Transfer\EventEntityTransfer> $eventTransfers | ||
| * | ||
| * @return array<int, int> | ||
| */ | ||
| protected function getIdsTimestampMap(array $eventTransfers): array | ||
| { | ||
| $ids = []; | ||
| foreach ($eventTransfers as $eventTransfer) { | ||
| $ids[(int)$eventTransfer->getId()] = (int)$eventTransfer->getTimestamp(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so, if we go with the easiest flow that we can check. E.g. I've updating some big form on backoffice that have more than 1 entity in it. I put new data in it and it will trigger n+1 different events. There are not a 0 chance that those events will have the same id. With this approach we are going to loose some of the events. The same goes for other similar methods
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As you see, we use the last one for a specific (unique) entity. It's part of the improvement
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok. I still have my doubts, but please double check it on QA |
||
| } | ||
|
|
||
| return $ids; | ||
| } | ||
|
|
||
| /** | ||
| * @param array<\Generated\Shared\Transfer\EventEntityTransfer> $eventTransfers | ||
| * @param string $columnName | ||
| * | ||
| * @return array<int|string, int> | ||
| */ | ||
| protected function getAdditionalValueTimestampMap(array $eventTransfers, string $columnName): array | ||
| { | ||
| $additionalValues = []; | ||
| foreach ($eventTransfers as $eventTransfer) { | ||
stereomon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| $additionalValuesOfEvent = $eventTransfer->getAdditionalValues(); | ||
| if (!isset($additionalValuesOfEvent[$columnName])) { | ||
| continue; | ||
| } | ||
|
|
||
| $additionalValues[$additionalValuesOfEvent[$columnName]] = (int)$eventTransfer->getTimestamp(); | ||
| } | ||
|
|
||
| return $additionalValues; | ||
| } | ||
|
|
||
| /** | ||
| * @param array<\Generated\Shared\Transfer\EventEntityTransfer> $eventTransfers | ||
| * @param string $foreignKeyColumnName | ||
| * | ||
| * @return array<int, int> | ||
| */ | ||
| protected function getForeignKeyTimestampMap(array $eventTransfers, string $foreignKeyColumnName): array | ||
| { | ||
| $foreignKeys = []; | ||
| foreach ($eventTransfers as $eventTransfer) { | ||
| if (!isset($eventTransfer->getForeignKeys()[$foreignKeyColumnName])) { | ||
| continue; | ||
| } | ||
|
|
||
| $value = $eventTransfer->getForeignKeys()[$foreignKeyColumnName] ?? null; | ||
| if ($value !== null) { | ||
| $foreignKeys[(int)$value] = (int)$eventTransfer->getTimestamp(); | ||
| } | ||
| } | ||
|
|
||
| return $foreignKeys; | ||
| } | ||
|
|
||
| /** | ||
| * @param array<string> $columns | ||
| * @param array<string> $modifiedColumns | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,7 +118,9 @@ protected function createEventResourceQueryContainerPluginIterator(EventResource | |
| protected function triggerBulk(EventResourceQueryContainerPluginInterface $plugin, array $ids): void | ||
| { | ||
| $eventEntityTransfers = array_map(function ($id) { | ||
| return (new EventEntityTransfer())->setId($id); | ||
| return (new EventEntityTransfer()) | ||
| ->setId($id) | ||
| ->setTimestamp(time()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so we just set time from now, but we need to compare it with the time from the message. This is not according AC
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are those changes mentioned in the changelogs?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, it's enough here because we publish specific entities, we don't modify them. |
||
| }, $ids); | ||
|
|
||
| $this->eventFacade->triggerBulk($plugin->getEventName(), $eventEntityTransfers); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.