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

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

Open
Toutouwai opened this issue Feb 9, 2017 · 18 comments
Open

Comments

@Toutouwai
Copy link

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
Copy link
Author

@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
Copy link

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
Copy link
Member

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
Copy link

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
Copy link
Author

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
Copy link

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
Copy link

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
Copy link

+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
Copy link

@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
Copy link
Collaborator

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

@arjenblokzijl
Copy link

And now one of mine as wel.

@netcarver
Copy link
Collaborator

@ryancramerdesign How would you like this issue handling? Is it worth reconsidering or is it still a won't fix?

@reminders reminders bot added the reminder label Feb 28, 2019
@reminders
Copy link

reminders bot commented Feb 28, 2019

@netcarver set a reminder for Mar 14th 2019

ryancramerdesign added a commit to processwire/processwire that referenced this issue Mar 1, 2019
…ing of fields in repeater items, individually per-field by non-superusers with edit access to the page the owns the repeater field.
@ryancramerdesign
Copy link
Member

@Toutouwai @netcarver I've pushed a fix for this issue, though since there's likely a few scenarios where this may come into play, it might need some testing. I'm testing it like this (with non-superuser) and all seems to be working after this latest commit:

foreach($page->repeater_field as $item) {
  echo "<h3><edit field='title' page='$item'>$item->title</edit></h3>";
  echo "<edit field='body' page='$item'>$item->body</edit>";
}

@Toutouwai
Copy link
Author

Thanks for the update @ryancramerdesign.

I can confirm that Option C is now working for non-superusers (and superusers).

Option A and Option B are working for superusers but not working for non-superusers (they cannot edit the fields within the repeater). I'm happy to use Option C if this is the only option that can support inline editing of fields within a repeater, but if it's possible to add support for Option A and Option B then that would be good as it would avoid any confusion going forward for people using those options.

ryancramerdesign added a commit to processwire/processwire that referenced this issue Mar 4, 2019
…peaters editable to non-superusers with front-end editing options A and B.
@ryancramerdesign
Copy link
Member

@Toutouwai It looks like I can make them editable with options A and B relatively easily, so I have added that as well. Note that if using option A, the "editor scope" setting in the module settings must be set to "fields editable regardless of page", since repeater items are independent pages.

@Toutouwai
Copy link
Author

Great, thanks!

@reminders reminders bot reopened this Mar 14, 2019
@reminders reminders bot removed the reminder label Mar 14, 2019
@processwire processwire deleted a comment from reminders bot Mar 14, 2019
@gebeer
Copy link

gebeer commented Jun 4, 2024

Sorry for reviving this issue.

On PW 3.0.210 I cannot get frontend editing to work properly for non superusers in nested repeaters.
Option B doesn't work at all.
Option C brings up editors for text fields, but changes are not being applied on save for non superusers.

When editing as superuser, everything works.

My scenario: Repeater Matrix field with a repeater field. Other fields work fine.

@netcarver netcarver reopened this Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants