Skip to content

Add private property annotations from inherited classes #675

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

Merged
merged 3 commits into from
Jun 27, 2020

Conversation

IceShack
Copy link
Contributor

@IceShack IceShack commented May 4, 2020

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Documented? no
Fixed tickets -
License MIT

Annotations from private properties from inherited classes got ignored, because they didn't get checked.

Copy link
Collaborator

@Vincz Vincz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good to me 👍

@akomm
Copy link
Contributor

akomm commented May 13, 2020

@IceShack Out of curiosity: I can't quite follow, why you want the properties private, if they are publicly accessed later? GQL/Type means, the value injected into field resolver of this type will be an instance of the annotated PHP class and the GLQ/Field makes the field something you would be publicly able to access via graphql, but it will be invisible in PHP. Accessing via graphql also means the field being read when serializing into graphql response.

@IceShack
Copy link
Contributor Author

IceShack commented May 14, 2020

@akomm I use getter/setter methods to access properties. Which looks like this:

<?php
use Overblog\GraphQLBundle\Annotation as GQL;

class Product
{

    /**
     * @var string
     *
     * @GQL\Field(type="String!")
     */
    private string $name;

    public function getName(): string
    {
        return $this->name;
    }

    public function setName(string $name): Product
    {
        $this->name = $name;

        return $this;
    }
}

@Vincz Vincz merged commit 6433464 into overblog:0.13 Jun 27, 2020
mcg-web added a commit that referenced this pull request Jul 8, 2020
…roperties0.12"

This reverts commit 6433464, reversing
changes made to 7155466.
@mcg-web
Copy link
Contributor

mcg-web commented Jul 8, 2020

I revert this merge on 0.13 since it seem to be more a feature than a bug fix and 0.13 is feature freeze. I'm I wrong @Vincz @IceShack ?

@Vincz
Copy link
Collaborator

Vincz commented Jul 8, 2020

Hey @mcg-web ! It's not a new feature, it's a fix. Previously, annotations on private properties on a parent class were not found (they were supposed to be). This PR fixed this issue.

@mcg-web
Copy link
Contributor

mcg-web commented Jul 8, 2020

Hey @Vincz thanks for your answer so maybe we should make the change from 0.12 if it is a fix related to annotations ?

@Vincz
Copy link
Collaborator

Vincz commented Jul 8, 2020

@mcg-web Yes, definitely 👍

mcg-web added a commit that referenced this pull request Jul 8, 2020
cherry-pick commits from original #675 fix proposed for 0.13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants