Improved error message for CMS page delete

[sreichel]

Description (*)

@addison74 thanks for working on all the CMS page related issue, but we should do it step by step ....

What are the issues we have?

• it is incomplete
  • disable page (works)
  • change url key (needs to be checked)
  • un-asign stores(needs to be checked)
  • asign disabled page in config(needs to be checked)
  • asign page with invalid store setting (test!?)
• error message should be improved (confirmed)

This PR ...

• improves error message
• fixes sort order in admin config
• adds some cypress/unit tests

Related Pull Requests

• see Fix and improve the cms page warning message when deleting #4968

Manual testing scenarios (*)

1. get to Admin - CMS - Page
2. try delete "no-route" page
3. ...

Delete

Disable

Note

To run cypress ... use DDEV, install cypress plugin, run ddev cypress-open -C .cypress.config.js

[Copilot]

Pull Request Overview

This PR improves error messaging when attempting to delete or disable CMS pages that are used in configuration, and includes related improvements to the admin interface.

• Enhanced error messages to provide clearer context about which configurations are preventing page deletion/disabling
• Fixed sort order in admin config system.xml for CMS pages
• Added comprehensive test coverage for new functionality

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
app/code/core/Mage/Cms/Helper/Page.php	Added helper methods to generate user-friendly error messages with configuration labels and scope information
app/code/core/Mage/Cms/Model/Resource/Page.php	Updated error message formatting and fixed store ID handling logic
app/locale/en_US/Mage_Cms.csv	Improved error message templates for better readability
app/code/core/Mage/Cms/etc/system.xml	Fixed sort order values for CMS page configuration options
tests/unit/Mage/Cms/Helper/PageTest.php	Added unit tests for new helper methods
tests/unit/Traits/DataProvider/Mage/Cms/CmsTrait.php	Added test data providers for the new functionality
cypress/e2e/openmage/backend/cms/page.cy.js	Added end-to-end tests for page deletion and disabling scenarios
cypress/support/openmage/config/paths.js	Added UI selectors for CMS page management
cypress/support/openmage.js	Fixed save action behavior to prevent multiple clicks

[addison74]

@sreichel -Thank you for your work on this PR and for your kind words. However, I have some concerns and suggestions based on our previous implementations (PR #4239 and #4969) and on practical usability:

1. The current message is incomplete, as it does not specify exactly where the page is used. Using "Store View" in the message is not informative (and takes precious space), especially since multiple store views can have the same name. My approach displays the logical combination with a defined lord, such as "Default Config, Madison Website, Madison Website > French", making it much easier for users to quickly identify and change the relevant scope immediately (e.g. Store View: French vs Madison Island > French)

2. When changing strictly the URL key, Status, or Delete, there should always be two mandatory messages displayed. First, a warning stating that the action cannot be performed and showing the reason (a short phrase). Second, an informational message explaining concretely where is the place the user can make the change (including the link to Web > Default Pages). If other allowed data was changed, a third success message should be displayed to confirm that those changes were saved (not the one existing which creates confusion and says the page was saved - only the unrestricted fields were salved).

3. Based on our previous discussions and the changes already implemented, this PR should not only address the message shown when deleting a page, but also comprehensively cover all three scenarios: changing the URL Key, updating the Status, and deleting the page.

For the info message displaying where the page is used, I can drop the attempt to build a grammatically correct English sentence (method _joinWithCommaAnd in PR #4969).

I believe these adjustments are crucial for a better user experience, especially in multi-store setups but this PR forces me to close my previous work, even though it could be easily adapted to incorporate these suggestions and best practices.

[sreichel]

The current message is incomplete

All is done here, can you make suggestions to the code?

    public static function getScopeInfoFromConfigScope(string $scope, string $scopeId): string
    {
        return match ($scope) {
            Mage_Adminhtml_Block_System_Config_Form::SCOPE_DEFAULT => Mage::helper('cms')->__('Default Config'),
            Mage_Adminhtml_Block_System_Config_Form::SCOPE_WEBSITES => sprintf(
                '%s "%s"',
                Mage::helper('cms')->__('Website'),
                Mage::app()->getWebsite($scopeId)->getName(),
            ),
            Mage_Adminhtml_Block_System_Config_Form::SCOPE_STORES => sprintf(
                '%s "%s"',
                Mage::helper('cms')->__('Store View'),
                Mage::app()->getStore($scopeId)->getName(),
            ),
        };
    }

this PR should not only address the message shown

This PR is ONLY for the message :P

Changes in logic please in another PR.

[addison74]

This is the current implementation in OpenMage. We can immediately notice that the message does not indicate where exactly the page is being used. The entire scope configuration list must be searched until it is found.

This is my implementation. The message clearly indicates where the page is being used. Furthermore, since it could be used in many places, I avoided using “Website” and “Store View” labels to save space. Moreover, the list of locations in parentheses follows a logical order — it fully traverses the tree, for example: Main Website, Main Website > French. If there are multiple stores, they will be listed in their respective order, making it even easier to find. In addition, I point the administrator to the exact location in the Backend where he can edit the scopes.

This is your implementation. It can be seen that you borrowed my idea of showing the scopes where the page is used, but the display is far from optimal — the usage of the words “Store View” and “Website” takes space, and I cannot tell at a glance where I should go (Stores and Websites are not listed in a logical order). In addition, there is no link to direct the administrator to edit the configuration; in my opinion, this is a useful feature.

In conclusion, thank you for taking the time to implement this PR. As I had already been working on a PR for the same functionality and shared my approach earlier, which includes more features, it would have been more constructive to coordinate rather than opening a separate PR and moving in a different direction.

Out of respect for you, I will refrain from reviewing further implementations and, more broadly, from contributing to anything related to the usage of pages in Default Pages. You are free to implement your approach. I will focus on improving my own version as a separate extension, ensuring it does not interfere with the core code and avoiding any conflicting discussions on this topic.

I hope you take all my feedback regarding this issue into consideration, with the request that you see it through to the end. I will focus my attention on other issues in OpenMage. Thank you for your understanding.

[sreichel]

In a few words, lets change the wording. I asked for suggestions. Link to config is fine too.

This PR was to avoid working on duplicated functionality for getting usage in config.

?

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud