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

Allow to copy ACL and config path #2774

Closed
wants to merge 5 commits into from
Closed

Allow to copy ACL and config path #2774

wants to merge 5 commits into from

Conversation

luigifab
Copy link
Contributor

@luigifab luigifab commented Dec 2, 2022

Description

A new feature!

Bellow click on the link, and paste the config path where you want!
You can disable it from System > Configuration > Admin > Admin theme (admin/design/copy_path).

Tested with Firefox 108, Chrome 108, Opera 94.
navigator.clipboard.writeText require Firefox 63+, Chrome 66+, Edge 79+, Opera 53+, Safari 13.1+ (mdn docs).

1/ Go to System / Configuration:

before/after
image
image

2/ Go to System / Web Services / SOAP Roles, add or edit any role:

before/after
image
image

3/ Go to System / Permissions / Roles, add or edit any role:

before/after
image
image

Warning

Not complete for System / Web Services / REST Roles, I'm not sure how them works.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added Component: Adminhtml Relates to Mage_Adminhtml Component: Api2 Relates to Mage_Api2 labels Dec 2, 2022
@github-actions github-actions bot added the Template : admin Relates to admin template label Dec 3, 2022
@sreichel
Copy link
Contributor

sreichel commented Dec 3, 2022

I like it, but do we need this in production mode?

Change

Mage::getIsDeveloperMode() || Mage::helper('admin/variable')->isPathAllowed($element->getPath())

to

Mage::getIsDeveloperMode() && Mage::helper('admin/variable')->isPathAllowed($element->getPath())

What do think?

@sreichel
Copy link
Contributor

sreichel commented Dec 4, 2022

Do we have to adjust openmage-theme (SCSS, CSS) too?

@github-actions github-actions bot added the Component: PayPal Relates to Mage_Paypal label Dec 8, 2022
@luigifab
Copy link
Contributor Author

luigifab commented Dec 8, 2022

I didn't test with openmage theme.

For production mode, normal admin can use only isPathAllowed in cms blocks, cms pages, wysiwyg products and categories attributes, transactional emails. In dev mode, developers can use all path in PHP files.

@kyrena
Copy link
Contributor

kyrena commented Dec 23, 2022

There was a module for Magento 1.4-9? that add a red icon image (for default configuration) when configuration is modified by store view. Is someone know where I can find it?

@sreichel
Copy link
Contributor

sreichel commented Dec 23, 2022

There was a module for Magento 1.4-9? that add a red icon image image (for default configuration) when configuration is modified by store view. Is someone know where I can find it?

I know that extension, but cant find it (was it a hackathon project?) There is a similar one: https://github.com/ericthehacker/magento-configscopehints (and https://github.com/avstudnitz/AvS_ScopeHint).

@Flyingmana
Copy link
Contributor

good idea.
Can we put it behind a config value? Iam worried not everyone might like the potentially long extra text.
also please change it to the branch for a later version (its a new feature)
And I wonder if we should make it more flexible, but thats something for a later iteration

@sreichel
Copy link
Contributor

sreichel commented Dec 27, 2022

good idea.

  • we could also use an icin instead of text
  • duplicated js code javascript:navigator.clipboard.writeText could move to core/js helper
  • replace inline style style="font-style:italic with css

@github-actions github-actions bot added Component: Core Relates to Mage_Core documentation JavaScript Relates to js/* translations Relates to app/locale labels Jan 1, 2023
@luigifab luigifab marked this pull request as ready for review January 1, 2023 13:00
@luigifab
Copy link
Contributor Author

luigifab commented Jan 5, 2023

I think that I have added all comments to the PR (except the red icon). It's not necessary to update openmage theme.
Do I need to change the branch to 20?

@sreichel
Copy link
Contributor

sreichel commented Jan 5, 2023

Think v19 is correct. Will test it later.

@luigifab luigifab changed the base branch from 1.9.4.x to main April 9, 2023 08:21
@addison74 addison74 self-assigned this Jul 14, 2023
@luigifab
Copy link
Contributor Author

Yes? No? To know if I try to add the red mark or not.

@kiatng
Copy link
Contributor

kiatng commented Nov 13, 2023

So I installed this PR:

But I don't see effects:

Any idea what I miss.

[edit] OOOPs, only for developer mode.

<copy_path translate="label">
<label>Allow to copy/paste roles and configuration path</label>
<frontend_type>select</frontend_type>
<source_model>adminhtml/system_config_source_yesno</source_model>
Copy link
Contributor

@kiatng kiatng Nov 13, 2023

Choose a reason for hiding this comment

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

Instead of yes/no:

  1. Only in Developer Mode (default)
  2. Yes
  3. No

@fballiano
Copy link
Contributor

why is this feature behind "developer mode"? I don't see why normal admin shouldn't be able to copy ACLs from a role to another no?

@luigifab
Copy link
Contributor Author

luigifab commented Apr 9, 2024

When you are a normal admin user, you can use only allowed config path in email, cms and pages, so this PR display copy links only for allowed config paths.

But it's true that if you are a normal admin user with access to all admin features, you can add any config paths to allowed list.

This PR display all copy links only in dev mode because I supposed that there are useless when you are using the backend as a normal user, not as a dev user.

@empiricompany
Copy link
Contributor

I've checked out the PR, but the new feature doesn't show up in every section of system/config and in permissions.
Also, when clicking, the path is not copied (Firefox)

demo
if i click on config nothing is copied

Nothing appears here:
demo2
roles

I've disabled all caches and also re-logged in to the backend

@fballiano
Copy link
Contributor

I think #3960 will be merged before this one and this one will have to be slightly adjusted, for example using the copy icon and the js functions ;-)

@@ -77,6 +77,10 @@ protected function _getNodeJson($node, $level = 0)
$item['sort_order'] = isset($node->sort_order) ? (string)$node->sort_order : 0;
$item['id'] = (string)$node->attributes()->aclpath;

if (Mage::getIsDeveloperMode() && Mage::getStoreConfigFlag('admin/design/copy_path')) {
$item['text'] .= '</a> <a href="javascript:copyText(\'' . $item['id'] . '\');" class="copycnf">(' . $item['id'] . ')';
Copy link
Contributor

Choose a reason for hiding this comment

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

Function name conflict, see PR #3960

@@ -11,6 +11,11 @@
* @copyright Copyright (c) 2019-2023 The OpenMage Contributors (https://www.openmage.org)
* @license https://opensource.org/licenses/afl-3.0.php Academic Free License (AFL 3.0)
*/

function copyText(txt) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Function name conflict, see PR #3960

@kiatng
Copy link
Contributor

kiatng commented Jun 5, 2024

I agree with @fballiano, this PR needs to incorporate what was done in PR #3960.

@luigifab
Copy link
Contributor Author

luigifab commented Jun 5, 2024

Yes, but I can't do it for now.

@luigifab luigifab marked this pull request as draft July 17, 2024 09:44
@luigifab luigifab closed this by deleting the head repository Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Adminhtml Relates to Mage_Adminhtml Component: Api2 Relates to Mage_Api2 Component: Core Relates to Mage_Core Component: PayPal Relates to Mage_Paypal documentation Don't forget this PR help wanted JavaScript Relates to js/* new feature Template : admin Relates to admin template translations Relates to app/locale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants