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

Non-superusers unable to front-end edit fields inside a repeater item #183

Open
Toutouwai opened this Issue Feb 9, 2017 · 11 comments

Comments

Projects
None yet
8 participants
@Toutouwai

Toutouwai commented Feb 9, 2017

Short description of the issue

Superusers may front-end edit a field inside a repeater item using methods B, C or D. Non-superusers cannot edit these fields (double-click has no effect), despite the user having access to the same fields in the back-end.

The only way a non-superuser is able to front-end edit a repeater is if the entire repeater foreach() is surrounded in edit tags (method C or D) and in this case the editing occurs in a modal dialog rather than inline.

If the user is granted edit access to the system repeater template then they may edit the repeater fields the same as a superuser, but this step should not be necessary.

Related forum threads:
https://processwire.com/talk/topic/15357-solved-frontend-edit-on-repeater-fields/

https://processwire.com/talk/topic/13150-front-end-editing-for-textarea-with-ckeditor-in-custom-role-not-possible/

Expected behavior

Users with back-end edit access to a field may edit that field in the front-end also.

Setup/Environment

  • ProcessWire version: 3.0.51
  • (Optional) PHP version: 7
@Toutouwai

This comment has been minimized.

Show comment
Hide comment
@Toutouwai

Toutouwai Feb 23, 2018

@ryancramerdesign, this issue still exists in v3.0.93, although based on comments you made in the forum I think you probably don't see it as a bug.

A repeater field is intended to be edited as one thing. Access is controlled for the repeater field itself, and a user doesn't have access to edit individual fields in a repeater outside the context of the repeater field (unless a superuser).

I understand this to mean that because non-superusers do not have edit access for the templates used for repeater items it is to be expected that they cannot individually edit fields within a repeater item on the front-end.

That prompts a question for me: what is the reason behind denying edit access for repeater templates for non-superusers? Couldn't the edit access to each repeater template be determined by the edit access for the associated repeater field? Is there some risk to doing it that way?

If having edit access to repeater templates enables front-end editing for individual fields inside repeaters I think that would be quite handy to have. Also I remember there was some other issue I had to solve by granting edit access to repeater templates but unfortunately I can't find the forum discussion for it now.

Toutouwai commented Feb 23, 2018

@ryancramerdesign, this issue still exists in v3.0.93, although based on comments you made in the forum I think you probably don't see it as a bug.

A repeater field is intended to be edited as one thing. Access is controlled for the repeater field itself, and a user doesn't have access to edit individual fields in a repeater outside the context of the repeater field (unless a superuser).

I understand this to mean that because non-superusers do not have edit access for the templates used for repeater items it is to be expected that they cannot individually edit fields within a repeater item on the front-end.

That prompts a question for me: what is the reason behind denying edit access for repeater templates for non-superusers? Couldn't the edit access to each repeater template be determined by the edit access for the associated repeater field? Is there some risk to doing it that way?

If having edit access to repeater templates enables front-end editing for individual fields inside repeaters I think that would be quite handy to have. Also I remember there was some other issue I had to solve by granting edit access to repeater templates but unfortunately I can't find the forum discussion for it now.

@teppokoivula

This comment has been minimized.

Show comment
Hide comment
@teppokoivula

teppokoivula Feb 24, 2018

At one point I ran into similar issue with the Thumbnails module and had to give edit access to users directly to the Repeater template. Not sure if that issue still exists, but it could be related.

I haven't been running into such issues lately, but that's probably because – and it's a bit disheartening to say this, to be honest – I've tried my best to steer away from Repeaters. Had way too many issues with them in the past. Permission issues, incompatibilities with third party inputfields, performance bottlenecks, and so on and so forth.

I wasn't aware of this particular issue, mainly because we haven't yet enabled front-end editing on our client sites, but I'm definitely with @Toutouwai here: would be great if this could be fixed. It might actually solve some of the older issues as well as fix front-end editing, so that'd be a nice bonus :)

teppokoivula commented Feb 24, 2018

At one point I ran into similar issue with the Thumbnails module and had to give edit access to users directly to the Repeater template. Not sure if that issue still exists, but it could be related.

I haven't been running into such issues lately, but that's probably because – and it's a bit disheartening to say this, to be honest – I've tried my best to steer away from Repeaters. Had way too many issues with them in the past. Permission issues, incompatibilities with third party inputfields, performance bottlenecks, and so on and so forth.

I wasn't aware of this particular issue, mainly because we haven't yet enabled front-end editing on our client sites, but I'm definitely with @Toutouwai here: would be great if this could be fixed. It might actually solve some of the older issues as well as fix front-end editing, so that'd be a nice bonus :)

@ryancramerdesign

This comment has been minimized.

Show comment
Hide comment
@ryancramerdesign

ryancramerdesign Feb 26, 2018

Contributor

The intention with repeater pages is that they are editable through the field, and not as independent pages separate from the field. The pages are protected under the admin structure, and the Repeater fieldtype manages things like status, names, parent, template. When edited on their own, separately from the field, those things are no longer able to be managed by the fieldtype. In a RepeaterMatrix those factors increase as well. That's why access is provided through the field/fieldtype, and that's intended. That's also why you have to edit it as a field (rather than separate pages and fields) when using front-end editing. Maybe that's not ideal for every case of front-end editing, and maybe there's more than can be done to help with the front-end editing side of it in the core, but probably not before the next master version. If you'd like to experiment with it on your own, you can assign access to the template like you've done, but I'd be more inclined to use a hook, maybe something like this in ready.php:

if(!$user->isSuperuser()) $wire->addHookAfter('Page::editable', function($e) {
  if($e->return) return;
  $page = $e->object;
  if($page instanceof RepeaterPage) {
    $args = $e->arguments(); 
    $event->return = $page->getForPage()->editable($args[0], $args[1]);
  }
}, [ 'priority' => 300 ]); 
Contributor

ryancramerdesign commented Feb 26, 2018

The intention with repeater pages is that they are editable through the field, and not as independent pages separate from the field. The pages are protected under the admin structure, and the Repeater fieldtype manages things like status, names, parent, template. When edited on their own, separately from the field, those things are no longer able to be managed by the fieldtype. In a RepeaterMatrix those factors increase as well. That's why access is provided through the field/fieldtype, and that's intended. That's also why you have to edit it as a field (rather than separate pages and fields) when using front-end editing. Maybe that's not ideal for every case of front-end editing, and maybe there's more than can be done to help with the front-end editing side of it in the core, but probably not before the next master version. If you'd like to experiment with it on your own, you can assign access to the template like you've done, but I'd be more inclined to use a hook, maybe something like this in ready.php:

if(!$user->isSuperuser()) $wire->addHookAfter('Page::editable', function($e) {
  if($e->return) return;
  $page = $e->object;
  if($page instanceof RepeaterPage) {
    $args = $e->arguments(); 
    $event->return = $page->getForPage()->editable($args[0], $args[1]);
  }
}, [ 'priority' => 300 ]); 
@teppokoivula

This comment has been minimized.

Show comment
Hide comment
@teppokoivula

teppokoivula Feb 27, 2018

Just for the record, here's that Thumbnails thing I mentioned earlier: https://github.com/apeisa/Thumbnails/blob/19d4b95ecbbf7e2e249acce4f0b5c3f9776bea51/ProcessCropImage/ProcessCropImage.module#L107:L108. It's roughly the same thing that Ryan mentions above, except in this case we have to figure out the Repeater Page ID from a GET param and then check if it's "for page" is editable, and it's all done within a Process module.

Not exactly optimal, but it gets the job done. I'm repeating myself here (d'oh) but it'd be awesome if Repeaters could handle things like these on their own. @ryancramerdesign, I'd be really happy if you could keep this in mind / on the to-do list / whatever. I quite enjoy the Repeater GUI, but these little things tend to pile up, and currently Repeaters are a bit unpredictable, so to speak :)

teppokoivula commented Feb 27, 2018

Just for the record, here's that Thumbnails thing I mentioned earlier: https://github.com/apeisa/Thumbnails/blob/19d4b95ecbbf7e2e249acce4f0b5c3f9776bea51/ProcessCropImage/ProcessCropImage.module#L107:L108. It's roughly the same thing that Ryan mentions above, except in this case we have to figure out the Repeater Page ID from a GET param and then check if it's "for page" is editable, and it's all done within a Process module.

Not exactly optimal, but it gets the job done. I'm repeating myself here (d'oh) but it'd be awesome if Repeaters could handle things like these on their own. @ryancramerdesign, I'd be really happy if you could keep this in mind / on the to-do list / whatever. I quite enjoy the Repeater GUI, but these little things tend to pile up, and currently Repeaters are a bit unpredictable, so to speak :)

@Toutouwai

This comment has been minimized.

Show comment
Hide comment
@Toutouwai

Toutouwai Feb 28, 2018

For front-end editing of fields inside a repeater, I found that besides the hook to Page::editable that @ryancramerdesign suggested another hook was also needed so that the repeater page does not fail the test of $user->hasPermission('page-edit-front', $page).

This is what I ended up with:

if(!$user->isSuperuser()) {

    $wire->addHookAfter('Page::editable', function(HookEvent $event) {
        if($event->return) return;
        $page = $event->object;
        if(!$page instanceof RepeaterPage) return;
        $n = 0;
        while(wireInstanceOf($page, 'RepeaterPage') && ++$n < 10) {
            /* @var RepeaterPage $page */
            $page = $page->getForPage();
        }
        $event->return = $page->editable(implode(',', $event->arguments()));
    }, [ 'priority' => 300 ]);

    $wire->addHookBefore('User::hasPagePermission', function(HookEvent $event) {
        $name = $event->arguments(0);
        $page = $event->arguments(1);
        if($name !== 'page-edit-front' || !$page instanceof RepeaterPage) return;
        $n = 0;
        while(wireInstanceOf($page, 'RepeaterPage') && ++$n < 10) {
            /* @var RepeaterPage $page */
            $page = $page->getForPage();
        }
        $event->arguments(1, $page);
    });

}

But rather than this approach I was thinking of actually granting edit/create access to the repeater template according to the user's access to the field.

$wire->addHookAfter('Fields::save', function(HookEvent $event) {
    $field = $event->arguments(0);
    // Only for Repeater fields
    if(!$field->type instanceof FieldtypeRepeater) return;
    if($field->hasFlag(Field::flagAccess)) {
        // Field is access controlled
        $edit_roles = $field->editRoles;
    } else {
        // Field is not access controlled
        $edit_roles = $this->roles->find("name!=superuser|guest")->explode('id');
    }
    // Get the template of the repeater field
    /* @var Template $template */
    $template = $this->templates->get($field->template_id);
    // Set view, edit and create access
    $template->useRoles = 1;
    $template->setRoles([37], 'view'); // Everyone can view
    $template->setRoles($edit_roles, 'edit');
    $template->setRoles($edit_roles, 'create');
    $template->save();
});

Toutouwai commented Feb 28, 2018

For front-end editing of fields inside a repeater, I found that besides the hook to Page::editable that @ryancramerdesign suggested another hook was also needed so that the repeater page does not fail the test of $user->hasPermission('page-edit-front', $page).

This is what I ended up with:

if(!$user->isSuperuser()) {

    $wire->addHookAfter('Page::editable', function(HookEvent $event) {
        if($event->return) return;
        $page = $event->object;
        if(!$page instanceof RepeaterPage) return;
        $n = 0;
        while(wireInstanceOf($page, 'RepeaterPage') && ++$n < 10) {
            /* @var RepeaterPage $page */
            $page = $page->getForPage();
        }
        $event->return = $page->editable(implode(',', $event->arguments()));
    }, [ 'priority' => 300 ]);

    $wire->addHookBefore('User::hasPagePermission', function(HookEvent $event) {
        $name = $event->arguments(0);
        $page = $event->arguments(1);
        if($name !== 'page-edit-front' || !$page instanceof RepeaterPage) return;
        $n = 0;
        while(wireInstanceOf($page, 'RepeaterPage') && ++$n < 10) {
            /* @var RepeaterPage $page */
            $page = $page->getForPage();
        }
        $event->arguments(1, $page);
    });

}

But rather than this approach I was thinking of actually granting edit/create access to the repeater template according to the user's access to the field.

$wire->addHookAfter('Fields::save', function(HookEvent $event) {
    $field = $event->arguments(0);
    // Only for Repeater fields
    if(!$field->type instanceof FieldtypeRepeater) return;
    if($field->hasFlag(Field::flagAccess)) {
        // Field is access controlled
        $edit_roles = $field->editRoles;
    } else {
        // Field is not access controlled
        $edit_roles = $this->roles->find("name!=superuser|guest")->explode('id');
    }
    // Get the template of the repeater field
    /* @var Template $template */
    $template = $this->templates->get($field->template_id);
    // Set view, edit and create access
    $template->useRoles = 1;
    $template->setRoles([37], 'view'); // Everyone can view
    $template->setRoles($edit_roles, 'edit');
    $template->setRoles($edit_roles, 'create');
    $template->save();
});
@BernhardBaumrock

This comment has been minimized.

Show comment
Hide comment
@BernhardBaumrock

BernhardBaumrock Mar 17, 2018

thank you for this workaround @Toutouwai

@ryancramerdesign I think Robin is totally right and this should be visited. Frontend-editing is an awesome tool you built! There are more and more people building page-builder like setups with the repeater matrix field ( see https://processwire.com/talk/topic/18735-processwire-repeatermatrix-css-grid-page-builder-concept/ for example). Having the repeater item fields editable on their own is just way better UI for the user than having it open in a modal.

Btw: I cannot use the modal edit functionality at all:
2018-03-17 15_39_51-energieguru at - home
That's a different topic, but I just wanted to mention it, so you can see this is a real issue for me and modal editing is no solution at all.

BernhardBaumrock commented Mar 17, 2018

thank you for this workaround @Toutouwai

@ryancramerdesign I think Robin is totally right and this should be visited. Frontend-editing is an awesome tool you built! There are more and more people building page-builder like setups with the repeater matrix field ( see https://processwire.com/talk/topic/18735-processwire-repeatermatrix-css-grid-page-builder-concept/ for example). Having the repeater item fields editable on their own is just way better UI for the user than having it open in a modal.

Btw: I cannot use the modal edit functionality at all:
2018-03-17 15_39_51-energieguru at - home
That's a different topic, but I just wanted to mention it, so you can see this is a real issue for me and modal editing is no solution at all.

@AndZyk

This comment has been minimized.

Show comment
Hide comment
@AndZyk

AndZyk Apr 26, 2018

@ryancramerdesign I also would like this decision to be reconsidered, because Front-End Editing is now a major selling point for our clients. ;-)

AndZyk commented Apr 26, 2018

@ryancramerdesign I also would like this decision to be reconsidered, because Front-End Editing is now a major selling point for our clients. ;-)

@Notanotherdotcom

This comment has been minimized.

Show comment
Hide comment
@Notanotherdotcom

Notanotherdotcom Jul 17, 2018

+1 for this - using repeaters for a massive documentation page (one long page) where repeater item levels are subsections and it opens and closes the nav as you scroll (kinda neat, and ideal that it's built with repeaters as the backend structure matches the navigation menu without having to force the client to use separate pages in the PW admin).

The client needs the ability to just edit one section of a document at a time, so would be ideal to frontend-edit just one repeater item at a time.

Notanotherdotcom commented Jul 17, 2018

+1 for this - using repeaters for a massive documentation page (one long page) where repeater item levels are subsections and it opens and closes the nav as you scroll (kinda neat, and ideal that it's built with repeaters as the backend structure matches the navigation menu without having to force the client to use separate pages in the PW admin).

The client needs the ability to just edit one section of a document at a time, so would be ideal to frontend-edit just one repeater item at a time.

@BernhardBaumrock

This comment has been minimized.

Show comment
Hide comment
@BernhardBaumrock

BernhardBaumrock Jul 17, 2018

@ryancramerdesign any news on this? I still think it's a very common scenario to have a page with repeater(matrix) blocks and you want it to be editable by clients from the frontend just like any other fields.

BernhardBaumrock commented Jul 17, 2018

@ryancramerdesign any news on this? I still think it's a very common scenario to have a page with repeater(matrix) blocks and you want it to be editable by clients from the frontend just like any other fields.

@netcarver

This comment has been minimized.

Show comment
Hide comment
@netcarver

netcarver Jul 29, 2018

Collaborator

@ryancramerdesign This also impacts one of my client's sites.

Collaborator

netcarver commented Jul 29, 2018

@ryancramerdesign This also impacts one of my client's sites.

@arjenblokzijl

This comment has been minimized.

Show comment
Hide comment
@arjenblokzijl

arjenblokzijl Sep 16, 2018

And now one of mine as wel.

arjenblokzijl commented Sep 16, 2018

And now one of mine as wel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment