Skip to content

Commit 858f3a7

Browse files
sreichelCopilotkiatng
authored
Improved error message for CMS page delete (#4974)
* Improved error message for CMS page delete * Added basic cypress test * Improved config sort order - group by Mage_Core and Mage_Cms * rename * update tests * doc * [skip-ci] typo * locale fix * fix disable page * cypress screenshots * bug fix * Update app/code/core/Mage/Cms/Model/Resource/Page.php Co-authored-by: Copilot <[email protected]> * Update cypress/support/openmage/config/paths.js Co-authored-by: Copilot <[email protected]> * Update tests * template fix * Update app/code/core/Mage/Cms/Model/Resource/Page.php * Update app/code/core/Mage/Cms/Model/Resource/Page.php * Update app/code/core/Mage/Cms/Helper/Page.php * Update store and website names in data provider * Refactor getScopeInfoFromConfigScope method * removed od files * updated css * translation * rector * Apply suggestions from code review * wording * tests * wording * typo * test * message for ENV config --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Ng Kiat Siong <[email protected]>
1 parent 99e2a6f commit 858f3a7

File tree

9 files changed

+183
-11
lines changed

9 files changed

+183
-11
lines changed

‎app/code/core/Mage/Cms/Helper/Page.php‎

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
* @package Mage_Cms
88
*/
99

10+
use Mage_Adminhtml_Block_System_Config_Form as Form;
11+
1012
/**
1113
* CMS Page Helper
1214
*
@@ -29,6 +31,7 @@ class Mage_Cms_Helper_Page extends Mage_Core_Helper_Abstract
2931
*
3032
* @param string $pageId
3133
* @return bool
34+
* @throws Mage_Core_Exception
3235
*/
3336
public function renderPage(Mage_Core_Controller_Front_Action $action, $pageId = null)
3437
{
@@ -41,8 +44,9 @@ public function renderPage(Mage_Core_Controller_Front_Action $action, $pageId =
4144
* @param string $pageId
4245
* @param bool $renderLayout
4346
* @return bool
47+
* @throws Mage_Core_Exception
4448
*/
45-
protected function _renderPage(Mage_Core_Controller_Varien_Action $action, $pageId = null, $renderLayout = true)
49+
protected function _renderPage(Mage_Core_Controller_Varien_Action $action, $pageId = null, $renderLayout = true)
4650
{
4751
$page = Mage::getSingleton('cms/page');
4852
if (!is_null($pageId) && $pageId !== $page->getId()) {
@@ -129,6 +133,7 @@ protected function _renderPage(Mage_Core_Controller_Varien_Action $action, $pag
129133
* @param string $pageId
130134
* @param bool $renderLayout
131135
* @return bool
136+
* @throws Mage_Core_Exception
132137
*/
133138
public function renderPageExtended(Mage_Core_Controller_Varien_Action $action, $pageId = null, $renderLayout = true)
134139
{
@@ -140,6 +145,7 @@ public function renderPageExtended(Mage_Core_Controller_Varien_Action $action, $
140145
*
141146
* @param string $pageId
142147
* @return string|null
148+
* @throws Mage_Core_Model_Store_Exception
143149
*/
144150
public function getPageUrl($pageId = null)
145151
{
@@ -176,4 +182,68 @@ public static function getUsedInStoreConfigPaths(?array $paths = []): array
176182

177183
return $searchPaths;
178184
}
185+
186+
/**
187+
* @param self::XML_PATH_* $path
188+
*/
189+
public static function getConfigLabelFromConfigPath(string $path): string
190+
{
191+
return match ($path) {
192+
self::XML_PATH_NO_ROUTE_PAGE => Mage::helper('cms')->__('No Route Page'),
193+
self::XML_PATH_NO_COOKIES_PAGE => Mage::helper('cms')->__('No Cookies Page'),
194+
self::XML_PATH_HOME_PAGE => Mage::helper('cms')->__('Home Page'),
195+
default => $path,
196+
};
197+
}
198+
199+
/**
200+
* @param Form::SCOPE_* $scope
201+
* @throws Mage_Core_Exception
202+
*/
203+
public static function getScopeInfoFromConfigScope(string $scope, string $scopeId): string
204+
{
205+
return match ($scope) {
206+
Form::SCOPE_ENV => Mage::helper('cms')->__('Environment Config'),
207+
Form::SCOPE_DEFAULT => Mage::helper('cms')->__('Default Config'),
208+
Form::SCOPE_WEBSITES => Mage::app()->getWebsite($scopeId)->getName(),
209+
Form::SCOPE_STORES => sprintf(
210+
'%s - %s',
211+
Mage::app()->getStore($scopeId)->getGroup()->getName(),
212+
Mage::app()->getStore($scopeId)->getName(),
213+
),
214+
};
215+
}
216+
217+
/**
218+
* @throws Mage_Core_Exception
219+
*/
220+
public static function getValidateConfigErrorMessage(Mage_Core_Model_Resource_Db_Collection_Abstract $isUsedInConfig): string
221+
{
222+
$messages = [];
223+
224+
$data = $isUsedInConfig->getData();
225+
foreach ($data as $key => $item) {
226+
$path = $item['path'];
227+
unset($item['config_id'], $item['path'], $item['updated_at'], $item['value']);
228+
$data[$path][] = $item;
229+
unset($data[$key], $key, $path);
230+
}
231+
232+
foreach ($data as $path => $items) {
233+
$scopes = [];
234+
foreach ($items as $item) {
235+
$scopes[] = self::getScopeInfoFromConfigScope($item['scope'], $item['scope_id']);
236+
}
237+
238+
$messages[] = sprintf(
239+
'%s (%s)',
240+
self::getConfigLabelFromConfigPath($path),
241+
implode(', ', $scopes),
242+
);
243+
}
244+
245+
unset($data, $path, $items, $item, $scopes);
246+
247+
return implode(', ', $messages);
248+
}
179249
}

‎app/code/core/Mage/Cms/Model/Resource/Page.php‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,9 @@ protected function _beforeDelete(Mage_Core_Model_Abstract $object)
3939
$object->setId(null);
4040
Mage::throwException(
4141
Mage::helper('cms')->__(
42-
'Cannot delete page, it is used in "%s".',
43-
implode(', ', $isUsedInConfig->getColumnValues('path')),
42+
'You cannot delete this page as it is used to <a href="%s">configure</a> %s.',
43+
Mage::helper('adminhtml')::getUrl('adminhtml/system_config/edit', ['section' => 'web']),
44+
Mage_Cms_Helper_Page::getValidateConfigErrorMessage($isUsedInConfig),
4445
),
4546
);
4647
}
@@ -79,8 +80,9 @@ protected function _beforeSave(Mage_Core_Model_Abstract $object)
7980
$object->setIsActive(true);
8081
Mage::getSingleton('adminhtml/session')->addWarning(
8182
Mage::helper('cms')->__(
82-
'Cannot disable page, it is used in configuration "%s".',
83-
implode(', ', $isUsedInConfig->getColumnValues('path')),
83+
'You cannot disable this page as it is used to <a href="%s">configure</a> %s.',
84+
Mage::helper('adminhtml')::getUrl('adminhtml/system_config/edit', ['section' => 'web']),
85+
Mage_Cms_Helper_Page::getValidateConfigErrorMessage($isUsedInConfig),
8486
),
8587
);
8688
}
@@ -111,6 +113,7 @@ protected function _beforeSave(Mage_Core_Model_Abstract $object)
111113
/**
112114
* @param Mage_Cms_Model_Page $object
113115
* @inheritDoc
116+
* @throws Zend_Db_Exception
114117
*/
115118
protected function _afterSave(Mage_Core_Model_Abstract $object)
116119
{
@@ -185,6 +188,7 @@ protected function _afterLoad(Mage_Core_Model_Abstract $object)
185188
* @param mixed $value
186189
* @param Mage_Cms_Model_Page $object
187190
* @return Zend_Db_Select
191+
* @throws Exception
188192
*/
189193
protected function _getLoadSelect($field, $value, $object)
190194
{
@@ -236,6 +240,7 @@ protected function _getLoadByIdentifierSelect($identifier, $store, $isActive = n
236240
/**
237241
* Check for unique of identifier of page to selected store(s).
238242
*
243+
* @param Mage_Cms_Model_Page $object
239244
* @return bool
240245
*/
241246
public function getIsUniquePageToStores(Mage_Core_Model_Abstract $object)
@@ -282,7 +287,9 @@ protected function isValidPageIdentifier(Mage_Core_Model_Abstract $object)
282287

283288
public function getUsedInStoreConfigCollection(Mage_Cms_Model_Page $page, ?array $paths = []): Mage_Core_Model_Resource_Db_Collection_Abstract
284289
{
285-
$storeIds = (array) $page->getStoreId();
290+
$storeId = (array) $page->getStoreId(); # null on save
291+
$stores = (array) $page->getStores(); # null on delete
292+
$storeIds = array_merge($storeId, $stores);
286293
$storeIds[] = Mage_Core_Model_App::ADMIN_STORE_ID;
287294
$config = Mage::getResourceModel('core/config_data_collection')
288295
->addFieldToFilter('value', $page->getIdentifier())
@@ -326,6 +333,7 @@ public function checkIdentifier($identifier, $storeId)
326333
*
327334
* @param string|int $identifier
328335
* @return string
336+
* @throws Mage_Core_Model_Store_Exception
329337
*/
330338
public function getCmsPageTitleByIdentifier($identifier)
331339
{
@@ -418,6 +426,7 @@ public function setStore($store)
418426
* Retrieve store model
419427
*
420428
* @return Mage_Core_Model_Store
429+
* @throws Mage_Core_Model_Store_Exception
421430
*/
422431
public function getStore()
423432
{

‎app/locale/en_US/Mage_Cms.csv‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,8 @@
2727
"CMS Static Block","CMS Static Block"
2828
"CMS Static Block Default Template","CMS Static Block Default Template"
2929
"Cannot create new directory.","Cannot create new directory."
30-
"Cannot delete page, it is used in ""%s"".","Cannot delete page, it is used in ""%s""."
3130
"Cannot delete directory %s.","Cannot delete directory %s."
3231
"Cannot delete root directory %s.","Cannot delete root directory %s."
33-
"Cannot disable page, it is used in configuration ""%s"".","Cannot disable page, it is used in configuration ""%s""."
3432
"Cannot upload file.","Cannot upload file."
3533
"Collapse All","Collapse All"
3634
"Content","Content"
@@ -124,4 +122,6 @@
124122
"Unable to find a block to delete.","Unable to find a block to delete."
125123
"Unable to find a page to delete.","Unable to find a page to delete."
126124
"WYSIWYG Options","WYSIWYG Options"
125+
"You cannot delete this page as it is used to <a href=""%s"">configure</a> %s.","You cannot delete this page as it is used to <a href=""%s"">configure</a> %s."
126+
"You cannot disable this page as it is used to <a href=""%s"">configure</a> %s.","You cannot disable this page as it is used to <a href=""%s"">configure</a> %s."
127127
"px.","px."

‎cypress/e2e/openmage/backend/cms/page.cy.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ describe(`Checks admin system "${test.index.title}"`, () => {
6161
test.edit.__buttons.saveAndContinue.click();
6262

6363
const success = 'The page has been saved.';
64-
const warning = 'Cannot disable page, it is used in configuration';
64+
const warning = 'You cannot disable this page as it is used to configure';
6565
validation.hasWarningMessage(warning);
6666
validation.hasSuccessMessage(success);
6767
utils.screenshot(cy.openmage.validation._messagesContainer, 'message.cms.page.disableActivePage');
@@ -71,7 +71,7 @@ describe(`Checks admin system "${test.index.title}"`, () => {
7171
test.index.clickGridRow('no-route');
7272
test.edit.__buttons.delete.click();
7373

74-
const message = 'Cannot delete page';
74+
const message = 'You cannot delete this page as it is used to configure';
7575
const screenshot = 'message.cms.page.deleteActivePage';
7676
validation.hasErrorMessage(message, { screenshot: true, filename: screenshot });
7777
});

‎skin/adminhtml/default/openmage/override.css‎

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)