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

Programmatically deleting drafts in hook throws error #1232

Closed
robinscholz opened this issue Dec 3, 2018 · 2 comments

Comments

@robinscholz
Copy link

@robinscholz robinscholz commented Dec 3, 2018

Describe the bug
Trying to programmatically delete subpage drafts within a page.changeStatus:after hook, throws the following error: SyntaxError: Unexpected token T in JSON at position 0

3/5 pages get deleted, 2/5 (identical) don’t.

Here is the config.php

'page.changeStatus:after' => function ($newPage, $oldPage) {
            if($newPage->intendedTemplate() == 'project' && $newPage->status() == 'unlisted') {
                foreach($newPage->drafts() as $phase) {
                    try {
                        $phase->delete();
                        echo 'The page has been deleted';
                    } catch(Exception $e) {
                        echo $e->getMessage();
                    }
                }   
            }
        }

To Reproduce
Steps to reproduce the behavior:

  1. Add subpages to parent page
  2. Change status from listed to unlisted for parent page through the panel
  3. See error

Expected behavior
All pages should be deleted.

Kirby Version
3.0.0-beta-6.22

Desktop (please complete the following information):

  • OS: OSX
  • Browser Chrome
  • Version 70
@bastianallgeier

This comment has been minimized.

Copy link
Contributor

@bastianallgeier bastianallgeier commented Dec 19, 2018

There are two issues here. First you shouldn't be echoing anything as this will end up in the response and the json will be broken.

Second: we have a bit of a problem here with a weird behavior of PHP's iterators. The $page->delete() method unsets the element from the parent collection while you are still looping through it. That way the loop will skip some elements.

You can avoid this by looping through the original data array instead.

'page.changeStatus:after' => function ($newPage, $oldPage) {
    if($newPage->intendedTemplate() == 'project' && $newPage->status() == 'unlisted') {
        foreach($newPage->drafts()->data() as $phase) {
                $phase->delete();
            }
        }   
    }
}
@bastianallgeier bastianallgeier removed this from the 3.0.0 RC 2 milestone Dec 19, 2018
@bastianallgeier bastianallgeier added this to the 3.1.1 milestone Mar 12, 2019
@bastianallgeier bastianallgeier removed this from the 3.1.2 milestone Apr 1, 2019
@distantnative distantnative added this to the 3.1.4 milestone Apr 1, 2019
@distantnative distantnative modified the milestones: 3.1.4, 3.2.0 Apr 16, 2019
@distantnative distantnative modified the milestones: 3.2.0, 3.2.2 May 10, 2019
@bastianallgeier bastianallgeier modified the milestones: 3.2.2, 3.2.3 Jul 10, 2019
@distantnative distantnative removed this from the 3.2.3 milestone Jul 18, 2019
afbora added a commit that referenced this issue Oct 15, 2019
Closes #1232
@afbora afbora referenced this issue Oct 15, 2019
0 of 4 tasks complete
bastianallgeier added a commit that referenced this issue Oct 15, 2019
bastianallgeier added a commit that referenced this issue Oct 15, 2019
bastianallgeier added a commit that referenced this issue Oct 15, 2019
@bastianallgeier

This comment has been minimized.

Copy link
Contributor

@bastianallgeier bastianallgeier commented Oct 15, 2019

@bastianallgeier bastianallgeier added this to the 3.3.0 milestone Oct 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.