-
Notifications
You must be signed in to change notification settings - Fork 174
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
SUP-20392:make syndication feed with dynamic playlist work the same as playlist executeFromFilter #9149
Conversation
… playlist executeFromFilter
… playlist executeFromFilter
… playlist executeFromFilter
… playlist executeFromFilter
|
||
if ($this->dynamicPlaylist) | ||
{ | ||
return $this->getEntriesCountFromDynamic(); |
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.
Only this code should be executed if there is a filter
{ | ||
list($mediaEntryFilterForPlaylist, $playlistService) = self::prepareParameters($entryFilter); | ||
$entriesFromFilter = $this->getEntriesFromPlaylist($playlistService, $mediaEntryFilterForPlaylist); | ||
$kalturaEntries = array_merge($kalturaEntries, $entriesFromFilter->toArray()); |
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.
Keep only the Id's
Use associative array to have the uniq,
return count($kalturaEntries); | ||
} | ||
|
||
protected static function getEntryListUnique($kalturaEntries) |
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.
remove
$this->lastEntryCreatedAt = 0; | ||
} | ||
|
||
++$this->returnedEntriesCount; |
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->returnedEntriesCount++;
$this->fetchNextEntriesAccordingToFilter(); | ||
} | ||
|
||
if (current($this->entriesCurrentPage)) |
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.
put current into local var
validate local var against !== false;
if (current($this->entriesCurrentPage)) | ||
{ | ||
$entry = entryPeer::retrieveByPK(current($this->entriesCurrentPage)->id); | ||
if($entry && !array_key_exists($entry->getId(), $this->entryIdsHandled)) |
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.
Check what happen in the current implementation when using multi filters and they return the same object twice.
In any case the return should be false only when no more entries
$pager = new KalturaPager(); | ||
$pager->pageSize = $this->syndicationFeed->pageSize; | ||
} | ||
if (!$currentFilter) |
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.
Move up , immediate after getting the value.
@@ -15,6 +15,8 @@ class KalturaSyndicationFeedRenderer | |||
const CACHE_EXPIRY = 2592000; // 30 days | |||
|
|||
const PAGE_SIZE_MAX_VALUE = 500; | |||
|
|||
const TOTAL_RESULTS = 200; |
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.
Rename to - DYNAMIC_PLAYLIST_TOTAL...
$entriesFromFilter = $this->getEntriesFromPlaylist($playlistService, $mediaEntryFilterForPlaylist); | ||
foreach ($entriesFromFilter as $entry) | ||
{ | ||
if (!array_key_exists($entry->id,$kalturaEntries)) |
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.
remove the if
$currentEntry = current($this->entriesCurrentPage); | ||
while ($currentEntry !== false) | ||
{ | ||
$entry = entryPeer::retrieveByPK(current($this->entriesCurrentPage)->id); |
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.
Use currentEntry
No description provided.