Skip to content

[#32717] Front End Module Editing #2521

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

Closed
wants to merge 58 commits into from

Conversation

Buddhima
Copy link
Contributor

Hi All,
With latest Joomla! release, super users can access module settings(back end) directly from front end.
But that can be done as front-end com_config does.

Along with this PR, I've created a system which can do almost all the essential settings for a module without switching to back end. Because of that, super user can see the changes easily.

Feature Tracker: http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=32717&start=0

Thank You !

message and redirect added to save controller
JHtml::_('behavior.combobox');
JHtml::_('formbehavior.chosen', 'select');

$hasContent = empty($this->item['module']) || $this->item['module'] == 'custom' || $this->item['module'] == 'mod_custom';
Copy link
Contributor

Choose a reason for hiding this comment

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

Why a check against a specific module? How would that work with similar 3rd party modules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, This check is to add 'content' field (text editor). Not every module needs that. This is a similar implementation to B/E view. 3rd party modules can contain 'content' fields and they'll be shown in respective sections.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, thanks

@zero-24
Copy link
Contributor

zero-24 commented Mar 10, 2014

@test again:
only a smal thing that i would change is:

screen shot 2014-03-10 at 09 49 10

I think we should use here a green message as all is ok :)
You may blame the J!Tracker Application for transmitting this comment.

@zero-24
Copy link
Contributor

zero-24 commented Mar 10, 2014

Please ignore my last post. The implementation here is correct see here:
https://github.com/Buddhima/joomla-cms/blob/config_modules/components/com_config/controller/modules/save.php#L90
and here:
http://docs.joomla.org/Display_error_messages_and_notices
so:
'message' (or empty) should green
but it don't work. So it is a issue with the alert implementation in the frontend not with this PR :)
Sorry :)
You may blame the J!Tracker Application for transmitting this comment.

EDIT: I will send a new Tracker for this.

@Bakual
Copy link
Contributor

Bakual commented Mar 10, 2014

So it is a issue with the alert implementation in the frontend not with this PR

Just a note: It's actually the Isis template which has some funny overrides for the messages. Protostar does not have any overrides here. This is why coloring is different in front- and backend.

@zero-24
Copy link
Contributor

zero-24 commented Mar 10, 2014

Issue message for the alert issue can be finde here:

http://issues.joomla.org/tracker/joomla-cms/3273
You may blame the J!Tracker Application for transmitting this comment.

@phproberto
Copy link
Contributor

I have some questions:

  • Why are you using com_config instead of com_modules?
  • Why are you sending calls directly to backend?

If this is frontend editing I think frontend has to contain the required code to be able to customise each behavior correctly for frontend /backend

About using com_config I think each component has to contain its own stuff. Otherwise we will face the chaos when we want to create the lite core.

Also it seems that we have some trailing whitespaces/codestyle errors. This is what I get when applying the PR as patch:

<stdin>:247: trailing whitespace.
<?php 
<stdin>:376: trailing whitespace.
<!DOCTYPE html><title></title>
<stdin>:891: trailing whitespace.
<!DOCTYPE html><title></title>
<stdin>:1057: trailing whitespace.

<stdin>:1150: trailing whitespace.
                echo $field->input; 
warning: 5 lines add whitespace errors.

@Buddhima
Copy link
Contributor Author

Hi Roberto,
With this concept frontend administration, I tried to make backend as a service provider for this, rather than creating a huge amount of duplicated code.
com_config, is actually a general container for frontend administration.
Through this way I tried to build a simple way to flexibly hold what people want.
I'll look at errors you mentioned.
Thank You!

@phproberto
Copy link
Contributor

Sorry but I don't agree. For the same reason we could merge all the components into a single one.

You are doing 2 things here:

  • Mixing frontend & backend (which I can accept but requires to be strongly discussed before putting it into the core)
  • Mixing different components (which I think is definitely wrong)

@brianteeman
Copy link
Contributor

2 questions

  1. Where is the ACL configuration for this feature
  2. Why dont I get full module editing. EG no menu assignment an limited module positions

@Buddhima
Copy link
Contributor Author

Hi Brian,
Thanks for testing this PR.
Answers for 2 questions are;

  1. Should be done through backend. Still there's no way to configure ACL through frontend and should be implemented in a way that can be adopted by upcoming frontend components. If a user has both "Access Administration Interface" & "Edit" levels allowed for Modules Component, that user has been granted the permission for Frontend module configuration. Special thing about here is "Access Administration Interface" is seeing as an access to frontend administration. This combination has never been used so far in any other case, so we can introduce that without making existing ACL more complex. Discussions on this has been taken in place at joomlacode-tracker.
  2. Module positions are shown according to current frontend template, logic can be find here:
    https://github.com/Buddhima/joomla-cms/blob/config_modules/components/com_config/model/modules.php#L127
    Similar to Permissions, Module assignments also need a way that can be used with other components that will come. It also has been discussed in the joomlacode-tracker.

Again, thank you very much for having experience with this.

@brianteeman
Copy link
Contributor

  1. So to give someone front end module edit I have to give them admin access as well? Doesnt sound right to me at all. This should have its own ACL settings and not "borrow" them from something else
  2. What about menu assigment?

Sorry but for me at least this isnt ready yet as its incomplete

@parthlawate
Copy link
Contributor

tested working as Described by the dev.

@Buddhima
Copy link
Contributor Author

Thank you @parthlawate for testing this.

@brianteeman , I think still you didn't get the idea about what I was explaining.

  1. No, I mean "Access Administration Interface" & "Edit" ACL allows particular user group to login from frontend (even can't login to backend) and modify modules.
  2. Actually, I was talking about Menu Assignment , accidentally , I've put "Module Assignment". That should be corrected as "Menu Assignment". Regret for the inconvenience caused you.

Thank You!

@brianteeman
Copy link
Contributor

It just seems wrong to me to use a generic ACL as you have described for a specific function.

@sovainfo
Copy link
Contributor

Agree with @brianteeman on frontend editting should not be connected to 'Access Administration interface'. Adding something like 'Access Frontend Administration' is not complicating ACL, abusing it is!
ACL does work in frontend provided it is implemented properly. Unfortunately the way it is currently implemented it needs code. I am not sure the frontend administration components should be the same as backend.

@sanderpotjer
Copy link
Member

I have been looking into this pull request, and in general it looks good. But the ACL implementation is indeed missing.

I expect the ACL for the frontend module editing is working similar to the Joomla Articles frontend editing. So in line with the articles concept I did the following test which works for the Joomla articles, but then for this module feature:

Article Manager Test

  • Create a new user group, with "Public" as parent
  • Go to the global configuration permissions tab and allow only the "Site Login"
  • Go to the Article Manager, open the Options, tab permissions and set "Edit" to allowed
  • Try to edit an article in the frontend

Result: able to see the edit icon, edit the article and save it

Module Manager Test (A)

  • Create a new user group, with "Public" as parent
  • Go to the global configuration permissions tab and allow only the "Site Login"
  • Go to the Module Manager, open the Options, tab permissions and set "Edit" to allowed
  • Try to edit a module in the frontend

Result: edit buttons are not visible, can't edit modules

I also tested the Module Manager by allowing the "Access Administration Interface" action, in that case the test is as follow:

Module Manager Test (B)

  • Create a new user group, with "Public" as parent
  • Go to the global configuration permissions tab and allow only the "Site Login"
  • Go to the Module Manager, open the Options, tab permissions and set "Edit" and "Access Administration Interface" to allowed
  • Try to edit a module in the frontend

Result: able to see the edit icon, edit the module but saving results in "You are not authorized to view this resource." error.

General ACL suggestions

I strongly recommend to follow the frontend ACL concept of the article manager for now, or have at least the same behavior for the frontend articles and module editing around ACL settings.

This means that a group with the "Site Login" and "Edit" action allowed should be able to edit the modules on the frontend. If a site owner doesn't want the modules editable on the frontend they shouldn't allow the edit action for the module manager or for the specific module(s).

I'm not sure there is a big need for difference in what a user can edit based on the ACL settings between te frontend and backend. So if you allow modules to be editable and a group has backend and frontend access they can edit it on both places. I can only think of situations where people would allow people to edit articles/modules in the backend, but not in the frontend. A general "Disable frontend editing" option would be enough for that. But that would be another pull request.

Anyway, thanks for your work on the frontend module editing, if the module ACL is following the article manager concept it looks good to me around ACL.

@infograf768
Copy link
Member

"This means that a group with the "Site Login" and "Edit" action allowed should be able to edit the modules on the frontend. If a site owner doesn't want the modules editable on the frontend they shouldn't allow the edit action for the module manager or for the specific module(s). "

The issue here is that we would not be B/C: A default "Editor" or "Publisher" (used only as of today for articles) would be able to edit modules in the frontend. A big surprise for a site admin...

@Bakual
Copy link
Contributor

Bakual commented Apr 5, 2014

The issue here is that we would not be B/C: A default "Editor" or "Publisher" (used only as of today for articles) would be able to edit modules in the frontend. A big surprise for a site admin...

That's indeed true. However Sander is right in how the ACL should be used. The fact that we use stupid default rules here doesn't change that.

A general "Disable frontend editing" option would be enough for that. But that would be another pull request.

In my own extension I have such a parameter to enable frontend editing (disabled by default). I think that would be a good parameter to add to com_modules. Then it doesn't matter anymore that we have messed up default ACL rules.
One could even add a postinstall message to tell the admins to enable that feature if they want to use it.
For new installations we could also fix the ACL and have it enabled by defautl.

@Buddhima
Copy link
Contributor Author

Buddhima commented Apr 5, 2014

Hi Sander,
There was a mistake in Save controller and I fixed it. Now "Module Manager Test (B)" should be okay.
Thank you for testing.

@bayareajenn
Copy link

tested fine on bluehost (i figure enough people are testing on rochen), PHP 5.4.27.

@Bakual
Copy link
Contributor

Bakual commented Jul 14, 2014

Changed ACL works for me. Thanks for that!

@dbhurley
Copy link

dbhurley commented Sep 3, 2014

Merged to 3.4-dev, thanks!

@dbhurley dbhurley closed this Sep 3, 2014
@kira99
Copy link

kira99 commented Nov 14, 2014

Joomla tooltip says: The template has to support this feature. Beez3 obviously does not support this. How can I implement this in a template?

@wilsonge wilsonge added this to the Joomla! 3.4.0 milestone Feb 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.