Jump to content

delete($page, true) doesn't delete repeated fields on 2.3.0


joe_g
 Share

Recommended Posts

Hi,

delete($page, true) doesn't seem to delete my repeater fields.

It's version 2.3.0 so this fix is included: 

https://github.com/ryancramerdesign/ProcessWire/commit/b2780236a2643d703c586a23991d80e2e6b171bf

I'm deleting a large amount of pages and inserting them again (working on a CMS conversion). The weird thing is that unless I delete all pages, and then all repeater pages, I can't insert any repeaters again. I have to clear out the DB from both pages and repeaters before I can insert anything again - otherwise no repeaters are stored

More weird: Unless I do the delete in a separate GET, it doesn't work either.

I have to first delete all stuff

$events = $pages->get('/events');
$events_children = $events->children();

foreach($events_children as $c) {
    $pages->delete($c, true);
}

$times = $pages->get('/processwire/repeaters/for-field-112/for-page-0/')->children();

foreach($times as $t) {
    $pages->delete($t, true);
}
 

and then fill in the new values:

for($n=1; $n<10; $n++) {
    $event = new Page();
    $event->parent= $pages->get('/events');
    $event->template = $templates->get('event');

    $event->title = 'Event hello #' . $n;
    

    //insert two times (repeater fields) per event
    for($t=0; $t<2; $t++) {
        $time = $event->times->getNew();
        echo $time->id . '</br>';
        
        $time->title = 'Time #' . $t;
        echo $time;
        $time->startdate = rand_date('2011-01-01', '2013-12-01');
        $time->save();
    }

    $event->save();
}
 

If I don't do it in two steps the times values are not inserted.

thanks!

rgds,

J

Link to comment
Share on other sites

I disabled the automatic creation of entries in the repeater, that seem to solve the problem with not being to insert the repeater entries without clearing out the database first.

I still would like to

- be able to delete everything and insert in the same script (now i need to do it in two GET's)

- Not have cruft left behind when i delete pages (repeater items under /processwire/repeaters/for-field-112/for-page-0/)

thanks in advance for any tips or hints. I can get by like this, also in the worst case I can clean up the database manually – but maybe the process could be cleaner :)

J

Link to comment
Share on other sites

I haven't been able to reproduce it either, but it's come up a couple times here, so there must be something to it. Though I'm not sure if it has to do with possibly leftovers from a previous version of ProcessWire. I think we need a test case/code to reproduce it consistently before it's something we can reproduce and solve. 

Link to comment
Share on other sites

I don't really know about that but have little intuition that it could have to do with the fact if one has already filled some fields of a repeater record but hasn't saved the page. Maybe these 'predefined' and than half 'prefilled' repeater records could have to do with it?

I disabled the automatic creation of entries in the repeater, that seem to solve the problem with not being to insert the repeater entries without clearing out the database first.

Link to comment
Share on other sites

  • 4 months later...

Reviving an old topic, as I'm seeing something similar here at the moment. Sadly my current setup is far from clean test case, so I won't be going into too much detail, except that it seems to have something to do with Pages delete() method.

Simply put it doesn't find actual repeater items, so they never get deleted. For an example page /processwire/repeaters/for-field-359/for-page-1646/ gets deleted, while one below it at /processwire/repeaters/for-field-359/for-page-1646/1391714225-7472-1/ remains.

This is the original code that won't work for me:

    public function ___delete(Page $page, $recursive = false) {

        if(!$this->isDeleteable($page)) throw new WireException("This page may not be deleted");

        if($page->numChildren) {
            if(!$recursive) throw new WireException("Can't delete Page $page because it has one or more children.");
            foreach($page->children("status<" . Page::statusMax) as $child) {

If I change that last line to this, repeater items are found and properly deleted:

            foreach($page->children("include=all") as $child) {

I'm probably missing something here, especially since I've no idea why status selector is used here or why it doesn't seem to find the repeater page in question, but at least for me this fixes the problem. Might cause some new ones, though, don't know about that yet.. :)

  • Like 1
Link to comment
Share on other sites

Thanks for the debugging work you did with this. You are right that the status<Page::statusMax should be replaced with an include=all. That status<Page::statusMax is the old version of include=all ... what we used before we had include=all. It still works just fine, except it doesn't bypass access control the way include=all does, because access control used to be handled differently. So it looks like the behavior you saw there could occur if it was executed from a non-superuser account. Just to confirm, replacing it with "include=all" fixed the issue from what you can tell? I will make the change here, as it should be include=all regardless. 

  • Like 1
Link to comment
Share on other sites

@ryan: the issue I was having was (once again) related to API use, in which case access control affecting things makes sense.. and yes, since then I've been running modified core code (include=all) and at least that particular issue seems to be fixed :)

By the way, I only just noticed your post and had already written a slightly more extensive test script to see what exactly happens when I work with repeaters over API without aforementioned fix. I posted it as a public gist here, if you want to take a look: https://gist.github.com/teppokoivula/8889040. Still far from a proper test case and a bit disorganised, but at least it helped me figure out what works and what (possibly) doesn't..

It's very much possible that I'm doing something wrong here and/or should really do even more manually, but so far it seems to me that repeaters could use some extra garbage cleaning here and there -- and perhaps some simplification especially when it comes to creating and/or removing repeater fields. It should also be noted that I couldn't find any "official" documentation either, so the way I'm doing this at the moment is partly copied from core code, some if it came from examples adrian (I think) posted somewhere etc.

  • Like 1
Link to comment
Share on other sites

  • 1 month later...

Just wanted to add that I am experiencing this issue too. Seems like it is the same as reported here:

http://processwire.com/talk/topic/2518-repeater-fields-via-api-it-has-no-parent-assigned/

Only difference I notice is that unlike teppo's report on his gist linked above, what I am finding left over is actually the "for-field" page, rather than the "for-page" page. As soon as I delete that page entry from the DB, everything works again as expected. 

Obviously this needs to be automated as part of the repeater cleanup.

Thanks for looking into it Ryan.

EDIT: Actually I am getting even more confused with this at the moment - a delete($parent_page, true) actually deletes a lot more repeater information than an admin delete of the same $parent_page, even after emptying the trash - which leaves behind the pages containing the actual repeater field content).

I guess in reality, the for_field page should stay there unless you're actually wanting to delete the repeater field. All I want to do is delete all the pages and the pages that contain the repeater field content on those pages. 

I'll keep experimenting :)

Link to comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
 Share

×
×
  • Create New...