[11.14] GraphQL API #772
[11.14] GraphQL API #772
Conversation
| @@ -36,7 +36,7 @@ abstract class AbstractApi | |||
| * | |||
| * @var string | |||
| */ | |||
| private const URI_PREFIX = '/api/v4/'; | |||
| protected const URI_PREFIX = '/api/v4/'; | |||
There was a problem hiding this comment.
Do you really need to change this? Since you're using static now, it should automatically use the one from the child class, no? 🤔
There was a problem hiding this comment.
In PHP private means a sub-class cannot extend/override it.
There was a problem hiding this comment.
why does it need to be overwritten? the changes you want don't need to re-use this?
There was a problem hiding this comment.
@GrahamCampbell, GraphQL is versionless. The URL prefix is /api instead of /api/v4, so AbstractApi::prepareUriP(), needs to get /api when subclassed as \Gitlab\Api\GraphQL
| private const URI_PREFIX = '/api/v4/'; | ||
| protected const URI_PREFIX = '/api/v4/'; |
There was a problem hiding this comment.
Maybe we should split version out of URI_PREFIX, into a protected class? Then we override the version in GraphQL as GitLab GraphQL is versionless
|
|
||
| namespace Gitlab\Api; | ||
|
|
||
| class GraphQL extends AbstractApi |
There was a problem hiding this comment.
We should. But I have no time to work on
| @@ -264,7 +264,7 @@ private static function prepareUri(string $uri, array $query = []): string | |||
| return null !== $value; | |||
| }); | |||
|
|
|||
| return \sprintf('%s%s%s', self::URI_PREFIX, $uri, QueryStringBuilder::build($query)); | |||
| return \sprintf('%s%s%s', static::URI_PREFIX, $uri, QueryStringBuilder::build($query)); | |||
There was a problem hiding this comment.
Please revert these changes.
There was a problem hiding this comment.
I don't think will work. GraphQL needs a different URL prefix
Maybe but I wanted to reuse |
|
We should just take the hit on a little duplication. Traits are horrible for this type of thing (most things imo |
).
|
Closing due to inactivity. |
Fixes #766