Graphql-laravel: Computed properties issue

[ZaidBarghouthi]

Consider the following:

type User {
   id
   name
}
type Post {
   id
   title
   body
   isPublished
   publishedAt
}

Where isPublished is defined as a computed property as

class Post extends Model
{
    ...
    public function getIsPublishedAttribute(): bool
    {
        $publishedAt = $this->published_at;
        return $publishedAt !== null;
    }
}

where is_published published_at is a dateTime field in the database. Edited

So, isPublished is not selectable. The problem is the value of isPublished depends on
published_at and the following query will always give isPublished : false

{
  users {
    id
    name
    posts {
      id
      isPublished #always false
    }
  }
}

and

{
  users {
    id
    name
    posts {
      id
      publishedAt
      isPublished #correct result
    }
  }
}

I suppose we need something like

            'isPublished' => [
                'type' => Type::nonNull(Type::boolean()),
                'selectable' => false,
                'must_select' => 'publishedAt'
            ],

because one wouldn't want to select published_at always, yet want it to always be selected when isPublished is selected

[mfn]

Philosophical note: I'm wondering if it's smart to add more and more custom attributes to such a config, mixed with the ones from GraphQL. I thinking about something funneling them in a single attribute, e.g.:

            'isPublished' => [
                'type' => Type::nonNull(Type::boolean()),
                'select_fields' => [
                    'selectable' => false,
                    'must_select' => 'publishedAt',
                ],  
            ],

Just thinking out loud and, sorry, unrelated to the issue itself: but I acknowledge a new directive is probably required.

Suggestions:

• naming: requires?
• it should probably be an array because who knows how many other columns it requires
• (and it should also support selecting relations because that's the next thing coming up)
• …

[mfn]

After one more thought, I'm not sure there is a problem, please see https://github.com/rebing/graphql-laravel/pull/378#issuecomment-508571375

[ZaidBarghouthi]

I know always fixes the issue, but its a workaround,

because one wouldn't want to select published_at always, yet want it to always be selected when isPublished is selected

[ZaidBarghouthi]

This is why i came across #369. I have a lot if computed properties in my project, Some of them depend on morph relations :P

[mfn]

You're right, acknowledged!

[ZaidBarghouthi]

Suggestions:

• naming: requires?
• it should probably be an array because who knows how many other columns it requires
• (and it should also support selecting relations because that's the next thing coming up)
• …

Yes 'required' sound better. But I guess we can name it always because its doing basically the same thing. It appears to be a generalization on always. What do you think @mfn?

[mfn]

But won't this clash in some way? 🤔

I guess I need to judge this on a technical implementation rather 🤷‍♀️

[mfn]

because one wouldn't want to select published_at always, yet want it to always be selected when isPublished is selected

@ZaidBarghouthi I would like to revisit this, because every time I re-think about this it seems to me the 'always' is the correct technical solution. Or I'm missing something.

always in fact acts like the suggested requires: only on the fields being _selected_ it's evaluated and thus adds the additional fields "always" to the SQL.

But it does not add the fields mentioned there _always_ when the particular type is fetched.

Do you know what I mean?

When in \Rebing\GraphQL\Tests\Database\SelectFields\ComputedPropertiesTests\PostType I change this:

            'isPublished' => [
                'type' => Type::nonNull(Type::boolean()),
                'selectable' => false,
                'always' => [
                    'published_at',
                ],
            ],

Only when isPublished is actually requested in the query, is published_at also fetched. But it's not fetched when I don't specify isPublished.

[mfn]

ping @ZaidBarghouthi

[ZaidBarghouthi]

Ah!, it seems to solve the problem!