-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Fix admin got auto logged out when displaying large number of product reviews #40227
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
base: 2.4-develop
Are you sure you want to change the base?
Changes from 3 commits
6e8aa57
5af53da
1840736
7d96d70
4ab76c2
ed08e62
73e7edc
39502f5
ab55653
1e122b1
922e107
441bf69
49b25bc
e1220b3
c9ef532
831cdcc
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 |
|---|---|---|
|
|
@@ -93,21 +93,6 @@ protected function _construct() | |
| $this->setDefaultSort('created_at'); | ||
| } | ||
|
|
||
| /** | ||
| * Save search results | ||
| * | ||
| * @return \Magento\Backend\Block\Widget\Grid | ||
| */ | ||
| protected function _afterLoadCollection() | ||
|
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. Removing this method could backward incompatible change, third-party extensions may override this method. |
||
| { | ||
| /** @var $actionPager \Magento\Review\Helper\Action\Pager */ | ||
| $actionPager = $this->_reviewActionPager; | ||
| $actionPager->setStorageId('reviews'); | ||
| $actionPager->setItems($this->getCollection()->getResultingIds()); | ||
|
Comment on lines
104
to
106
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. Removing these lines might breaks prev/next functionality unless replaced with alternative.
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. @engcom-Hotel I have made the other changes, for prev/next functionality, I have handled it in the Pager.php, please look at getPreviousItemId and getNextItemId methods. |
||
|
|
||
| return parent::_afterLoadCollection(); | ||
| } | ||
|
|
||
| /** | ||
| * @inheritDoc | ||
| */ | ||
|
|
||
|
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. Various methods has been removed from here, this could be backward incompatible change. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,144 +6,77 @@ | |
|
|
||
| namespace Magento\Review\Helper\Action; | ||
|
|
||
| use Magento\Framework\Exception\LocalizedException; | ||
| use Magento\Framework\App\Helper\AbstractHelper; | ||
| use Magento\Review\Model\ResourceModel\Review\CollectionFactory; | ||
|
|
||
| /** | ||
| * Action pager helper for iterating over search results | ||
| * | ||
| * @api | ||
| * @since 100.0.2 | ||
| */ | ||
| class Pager extends \Magento\Framework\App\Helper\AbstractHelper | ||
| class Pager extends AbstractHelper | ||
| { | ||
| const STORAGE_PREFIX = 'search_result_ids'; | ||
|
|
||
| /** | ||
| * Storage id | ||
| * | ||
| * @var int | ||
| */ | ||
| protected $_storageId = null; | ||
|
|
||
| /** | ||
| * Array of items | ||
| * Review collection factory | ||
| * | ||
| * @var array | ||
| * @var CollectionFactory | ||
| */ | ||
| protected $_items = null; | ||
| protected $reviewCollectionFactory; | ||
|
|
||
| /** | ||
| * Backend session model | ||
| * Pager constructor. | ||
| * | ||
| * @var \Magento\Backend\Model\Session | ||
| */ | ||
| protected $_backendSession; | ||
|
|
||
| /** | ||
| * @param \Magento\Framework\App\Helper\Context $context | ||
| * @param \Magento\Backend\Model\Session $backendSession | ||
| * @param CollectionFactory $reviewCollectionFactory | ||
| */ | ||
| public function __construct( | ||
| \Magento\Framework\App\Helper\Context $context, | ||
| \Magento\Backend\Model\Session $backendSession | ||
| CollectionFactory $reviewCollectionFactory | ||
|
||
| ) { | ||
| $this->_backendSession = $backendSession; | ||
| parent::__construct($context); | ||
| $this->reviewCollectionFactory = $reviewCollectionFactory; | ||
| } | ||
|
|
||
| /** | ||
| * Set storage id | ||
| * | ||
| * @param int $storageId | ||
| * @return void | ||
| */ | ||
| public function setStorageId($storageId) | ||
| { | ||
| $this->_storageId = $storageId; | ||
| } | ||
|
|
||
| /** | ||
| * Set items to storage | ||
| * | ||
| * @param array $items | ||
| * @return $this | ||
| */ | ||
| public function setItems(array $items) | ||
| { | ||
| $this->_items = $items; | ||
| $this->_backendSession->setData($this->_getStorageKey(), $this->_items); | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| /** | ||
| * Load stored items | ||
| * | ||
| * @return void | ||
| */ | ||
| protected function _loadItems() | ||
| { | ||
| if ($this->_items === null) { | ||
| $this->_items = (array)$this->_backendSession->getData($this->_getStorageKey()); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Get next item id | ||
| * Get the next review id. | ||
| * | ||
| * @param int $id | ||
| * @return int|bool | ||
| * @return int|false | ||
| */ | ||
| public function getNextItemId($id) | ||
| { | ||
| $position = $this->_findItemPositionByValue($id); | ||
| if ($position === false || $position == count($this->_items) - 1) { | ||
| return false; | ||
| } | ||
|
|
||
| return $this->_items[$position + 1]; | ||
| return $this->getRelativeReviewId($id, 'gt', 'ASC'); | ||
| } | ||
|
|
||
| /** | ||
| * Get previous item id | ||
| * Get the previous review id. | ||
| * | ||
| * @param int $id | ||
| * @return int|bool | ||
| * @return int|false | ||
| */ | ||
| public function getPreviousItemId($id) | ||
| { | ||
| $position = $this->_findItemPositionByValue($id); | ||
| if ($position === false || $position == 0) { | ||
| return false; | ||
| } | ||
|
|
||
| return $this->_items[$position - 1]; | ||
| return $this->getRelativeReviewId($id, 'lt', 'DESC'); | ||
| } | ||
|
|
||
| /** | ||
| * Return item position based on passed in value | ||
| * Get the review id based on comparison and order. | ||
| * | ||
| * @param mixed $value | ||
| * @return int|bool | ||
| * @param int $id | ||
| * @param string $operator | ||
| * @param string $order | ||
| * @return int|false | ||
| */ | ||
| protected function _findItemPositionByValue($value) | ||
| private function getRelativeReviewId($id, $operator, $order) | ||
|
||
| { | ||
| $this->_loadItems(); | ||
| return array_search($value, $this->_items); | ||
| } | ||
|
|
||
| /** | ||
| * Get storage key | ||
| * | ||
| * @return string | ||
| * @throws \Magento\Framework\Exception\LocalizedException | ||
| */ | ||
| protected function _getStorageKey() | ||
| { | ||
| if (!$this->_storageId) { | ||
| throw new LocalizedException(__("The storage key wasn't set. Add the storage key and try again.")); | ||
| } | ||
|
|
||
| return self::STORAGE_PREFIX . $this->_storageId; | ||
| $collection = $this->reviewCollectionFactory->create(); | ||
| $collection->addFieldToFilter('main_table.review_id', [$operator => $id]) | ||
| ->setOrder('main_table.review_id', $order) | ||
| ->setPageSize(1) | ||
| ->setCurPage(1); | ||
|
|
||
| $item = $collection->getFirstItem(); | ||
| return $item->getId() ? (int)$item->getId() : false; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -401,30 +401,6 @@ public function getAllIds($limit = null, $offset = null) | |
| return $this->getConnection()->fetchCol($idsSelect); | ||
| } | ||
|
|
||
| /** | ||
| * Get result sorted ids | ||
| * | ||
| * @return array | ||
| */ | ||
| public function getResultingIds() | ||
|
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. Removing this method could be backward incompatible change. |
||
| { | ||
| $idsSelect = clone $this->getSelect(); | ||
| $data = $this->getConnection() | ||
| ->fetchAll( | ||
| $idsSelect | ||
| ->reset(Select::LIMIT_COUNT) | ||
| ->reset(Select::LIMIT_OFFSET) | ||
| ->columns('rt.review_id') | ||
| ); | ||
|
|
||
| return array_map( | ||
| function ($value) { | ||
| return $value['review_id']; | ||
| }, | ||
| $data | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Render SQL for retrieve product count | ||
| * | ||
|
|
||
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.
Removing
$actionPager->setStorageId('reviews')breaks the pagination API contract.If
setStorageId()is never called, then_getStorageKey()in Pager.php throws exception: