Pages:factory overrides passed in status

Kirby 3 Solved ✅
1

Hi,
i’m currently following the https://getkirby.com/docs/guide/virtual-pages/content-from-csv guide and running into an issue when I’m trying to set the status of the page based on a boolean received from the spreadsheet.

The issue with the factory is that it always overrides a isDraft prop of a page with the value of the third parameter (false by default).

is there anything I can do prevent overriding? or do i have to loop through the pages afterwards again?

thx

2

I think we ran into a similar issue here: Database demo, add new page and change page url/slug does not work. I haven’t had a chance to look into this any further, but the third parameter in the factory is a good hint.

Could you post your code?

Edit: I think I’m getting there. Have you implemented all the changeStatus() methods in your single page model?

We have to set the third value like this in the parent model (this is based on the database example):

<?php

class CommentsPage extends Kirby\Cms\Page
{
    public function children()
    {
        $comments = [];

        foreach (Db::select('comments') as $comment) {
            $isDraft = is_null($comment->status())  ? true: false;
            $comments[] = [
                'slug'     => $comment->slug(),
                'num'      => $comment->status() === 'listed' ? 0 : null,
                'template' => 'comment',
                'model'    => 'comment',
                //'status'   => $comment->status(),
                //'isDraft'  => ($comment->status() === null || $comment->status === 'draft') ? true : false,
                'content'  => [
                    'text'   => $comment->text(),
                    'user'   => $comment->user(),
                    'status' => is_null($comment->status()) ? 'draft' : $comment->status()
                ]
            ];
        }

        return Pages::factory($comments, $this, $isDraft);
    }
}

And then define the following methods that update the status in the child model:

protected function changeStatusToListed()
{
       // update status field and return page
}

protected function changeStatusToUnlisted()
{
       // update status field and return page
}
protected function changeStatusToDraft()
{
       // update status field and return page
}

And the isDraft() method:

public function isDraft(): bool
{
    return in_array($this->content()->status(), ['listed', 'unlisted']) === false;
}

In this example, the possible status values are null, unlisted and listed.

3

I’ve posted my full code here: Database demo, add new page and change page url/slug does not work

4

hi @texnixe,
hm, that is a similar approach like in my tests yesterday but I did not go that far and tried to override the changeStatus* methods. I will give your approach a try but just from reviewing it i am wondering about the behaviour from $isDraft. It is (re)defined inside the loop, therefore it’s last state will be passed on to the factory right?

To be honest, i see no reason why a factory of multiple pages should (always) decide what status should be. Why not simply checking for an existing status of that page and then use the passed in parameter as a fallback?

5

Not quite sure what you mean, but you have to define the relation between the database field and what Kirby considers as the status somewhere?

But I’ve asked the team to have a look at my code and improve it where needed.

6

wouldn’t mine as well as your code just work as expected if https://github.com/getkirby/kirby/blob/3.2.5/src/Cms/Pages.php#L155 would not override a given status but check first like so:

if !isset($props['isDraft']) {
    $props['isDraft'] = $draft;
}

this is the snippet I was playing around with:

public function children() {
    $csv      = csv($this->root() . '/nodes.csv');

    $children = array_map(function ($node, $index) {

        $slug = Str::slug($node['Name']);
        $page = $this->subpages()->find($slug);

        return [
            'slug'     => $slug,
            'template' => 'node',
            'model'    => 'node',
            'num'      => $index,
            'isDraft'   => $node['Card'] != 'Yes',
            'content'  => [
                'title'       => $page ? $page->Title()->value() : $node['Name']
            ]
        ];
    }, $csv, array_keys($csv));

    return Pages::factory($children, $this);
7

I thought about it again and realized that setting the third param doesn’t make sense, and in fact doesn’t have any effect. The example works without setting this, I’ve changed it accordingly.

I think the important part here is not to set the isDraft property, but the isDraft() method in the model.