Skip to content

Conversation

@chmst
Copy link
Contributor

@chmst chmst commented Sep 18, 2022

Pull Request for Issue # .

Summary of Changes

"Configure Options Only" makes me think .. why only?

grafik

Change the String to "Configure Options"

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.2-dev labels Sep 18, 2022
@ChristineWk
Copy link

I have tested this item ✅ successfully on 125143e


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38781.

@brianteeman
Copy link
Contributor

Not at my pc but please check template manager. Iirc this string comes from there where you could have access to options but not other restricted items. Unlike all other components

@chmst
Copy link
Contributor Author

chmst commented Sep 18, 2022

The string is used in permissions of the template manager. But still correct when the "only" is removed.

If core.options has another meaning in template management than in other components, we better should define an specified action.

grafik

@brianteeman
Copy link
Contributor

Now that I am at my pc I can see that what was originally just a template manager permission is now everywhere and that the problem is not so much the word "only" but that there is a column missing in the users permissions view. "Configure ACL and Options"

@brianteeman
Copy link
Contributor

Looking at the issue I just created #38830 I can see that the problem here arises from your pr #37807 which moved core.admin (Configure ACL and Options) out of the columns but did not also move core.options (Configure ACL Only)

In summary this is not the correct fix and it should be fixed together with #38830

@chmst
Copy link
Contributor Author

chmst commented Sep 25, 2022

Will check after my holidays. The problem occurs if no permissions are found on level >= 4.

@chmst
Copy link
Contributor Author

chmst commented Oct 16, 2022

Looking at the issue I just created #38830 I can see that the problem here arises from your pr #37807 which moved core.admin (Configure ACL and Options) out of the columns but did not also move core.options (Configure ACL Only)

In summary this is not the correct fix and it should be fixed together with #38830

#38830 is resolved with #38960. PR #37807 did not import a bug but made an old error visible.
In summary this issue has nothing to do with the above PRs.

@brianteeman
Copy link
Contributor

Sorry for mistaking that error.

While I agree that in the debug user permissions view it might not be needed removing it from the permissions table does change its meaning.

image

@chmst
Copy link
Contributor Author

chmst commented Oct 16, 2022

I don't insist in changing the label, but I see a logical error in the permission rules. As a user and only from labels I think:

Configure everything | Configure options only | result

     yes         |       yes               |   configure everything but for what do we need  options only?
     yes         |       no                |   configure eeryting but don't configure options only (!)
     no          |       yes               |   configure nothing but configure options only (!)
     no          |       no                |   configure nothing but for what do we need  options only?

The follwing would be correct but must be checkd in the whole core


Configure ACL | Configure options       | result
-----------------------------------------------------------------
         yes  |       yes               | configure everything
         yes  |       no                | configure ACL only 
         no   |       yes               | configure options only
         no   |       no                | configure nothing

@brianteeman
Copy link
Contributor

There isn't a configure acl only option

(maybe there should be)

@chmst
Copy link
Contributor Author

chmst commented Oct 16, 2022

This is what I wanted to demonstrate. Configure ACL and Configure Options would be better. But this is a bigger task and surely not for J4.

@brianteeman
Copy link
Contributor

Agreed.

Originally the "configure options only" was a hack for joomla.com in com_templates when it was at siteground iirc

@chmst chmst changed the title Less is more: Configure Options Inconsistency in permission rules - configure options only Oct 16, 2022
@chmst chmst added Feature and removed PR-4.2-dev labels Oct 16, 2022
@Quy
Copy link
Contributor

Quy commented Jan 19, 2023

I have tested this item ✅ successfully on 125143e


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38781.

@Quy Quy added PR-4.2-dev and removed Feature labels Jan 19, 2023
@Quy
Copy link
Contributor

Quy commented Jan 19, 2023

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38781.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 19, 2023
@roland-d roland-d merged commit 14a8e61 into joomla:4.2-dev Jan 20, 2023
@roland-d
Copy link
Contributor

Thank you

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 20, 2023
@roland-d roland-d added this to the Joomla! 4.2.7 milestone Jan 20, 2023
@tecpromotion
Copy link
Contributor

The change was unfortunately not made consistently in the API.

JACTION_OPTIONS="Configure Options Only"

PR #39682 will fix this.

Kostelano added a commit to JPathRu/localisation that referenced this pull request Feb 1, 2023
* Joomla 4.2.6
joomla/joomla-cms#39143 - (только для en-GB)
joomla/joomla-cms#39317 - (исправлено ранее)

* Joomla 4.2.7
joomla/joomla-cms#39376 +
joomla/joomla-cms#39336 - (только для en-GB)
joomla/joomla-cms#39629 - (только для en-GB)
joomla/joomla-cms#37574 +
joomla/joomla-cms#38781 - (только для en-GB)
joomla/joomla-cms#39677 - (исправлено ранее)
joomla/joomla-cms#39682 - (только для en-GB)
@chmst chmst deleted the permission-language-change branch August 4, 2024 08:08
@chmst chmst restored the permission-language-change branch August 4, 2024 08:08
@chmst chmst deleted the permission-language-change branch August 4, 2024 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Language Change This is for Translators

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants