Skip to content
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

Improve the processor permission response error message #15402

Merged
merged 4 commits into from Feb 23, 2021
Merged

Improve the processor permission response error message #15402

merged 4 commits into from Feb 23, 2021

Conversation

Jako
Copy link
Collaborator

@Jako Jako commented Feb 20, 2021

What does it do?

  • Add a lexicon entry for the processor permission message.
  • Output the permission and the processor action in the message (Moving the permission class property to the modProcessor class was needed).
  • Show the new error message in the MODx.combo.ComboBox loadexception

Why is it needed?

It is not easy to detect, which permission is needed for a combobox processor permission error.

Before:

Bildschirmfoto 2021-02-20 um 21 13 52

After:

Bildschirmfoto 2021-02-20 um 21 14 42

How to test

Add the following code to i.e. core/model/modx/processors/workspace/namespace/getlist.class.php and open the MODX system settings.

    public function checkPermissions() {
        return false;
    }

In a grid, the message is shown in the empty grid content.

Related issue(s)/PR(s)

#13243, #12568, #15344

@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Feb 20, 2021
@Ruslan-Aleev
Copy link
Collaborator

Ruslan-Aleev commented Feb 20, 2021

Also related with - #15344
Powerful PR! Will there be no problem with encoding in json here? As in the issue above.

@Ruslan-Aleev Ruslan-Aleev added proposal Proposal about improvement aka RFC. Need to be discussed before start implementation. pr/port-to-3 Pull request has to be ported to 3.x. pr/review-needed Pull request requires review and testing. labels Feb 20, 2021
@Jako
Copy link
Collaborator Author

Jako commented Feb 20, 2021

Yes, the JSON will be decoded and the encoded unicode chars will be transformed to readable text.

@Jako Jako changed the title Improve the processor response error message Improve the processor permission response error message Feb 20, 2021
@Mark-H
Copy link
Collaborator

Mark-H commented Feb 21, 2021

I don't typically want to engage in nitpicking textual stuff, but would "Permission 'foo bar' is required for 'action'" be better English?

I think "permission denied 'foo' for 'bar'" may give users the impression they have permission 'foo' but that it's not a valid role of sorts.

Copy link
Collaborator

@Ruslan-Aleev Ruslan-Aleev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! Everything works visually.

res_changes
template_views

@Jako
Copy link
Collaborator Author

Jako commented Feb 21, 2021

@Mark-H You are right. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed CLA confirmed for contributors to this PR. pr/review-needed Pull request requires review and testing. proposal Proposal about improvement aka RFC. Need to be discussed before start implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants