Skip to content
This repository has been archived by the owner on Dec 19, 2019. It is now read-only.

GraphQl-93: Implement support for variables in query #259

Merged
merged 6 commits into from
Jan 23, 2019

Conversation

Aquarvin
Copy link
Contributor

@Aquarvin Aquarvin commented Nov 20, 2018

-- Variables may be Input type
-- Query now accepts variables
-- added type property for Output/Input Type element
-- functional test was added

Description (*)

Currently running the following Graph Ql query

query GetProductsQuery($page: Int, $filterInput: ProductFilterInput){
  products(
    pageSize: 10
    currentPage: $page
    filter: $filterInput
    sort: {}
  ) {
    items {
    	name
    }
  }
}

with variables:

{
  "page": 1,
  "filterInput": {
    "price": {
    	"gt": "10"
    }
  }
}

Will return filtered products list, since ProductFilterInput is an input type, according to scheme

Fixed Issues (if relevant)

  1. Implement support for variables in query #93: Implement support for variables in query

Manual testing scenarios (*)

  1. ...
  2. ...

Contribution checklist (*)

  • [+ ] Pull request has a meaningful description of its purpose
  • [ +] All commits are accompanied by meaningful commit messages
  • [ +] All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

-- Variables may be Input type
-- Query now accepts variables
-- added type property for Output/Input Type element
-- functional test was added
@Aquarvin
Copy link
Contributor Author

Aquarvin commented Nov 20, 2018

Request Sample:

query GetProductsQuery($page: Int, $filterInput: ProductFilterInput){
  products(
    pageSize: 10
    currentPage: $page
    filter: $filterInput
    sort: {}
  ) {
    items {
    	name
    }
  }
}

With variables:

{
  "page": 1,
  "filterInput": {
    "price": {
    	"gt": "10"
    }
  }
}


$query
= <<<'QUERY'
query GetProductsQuery($page: Int, $filterInput: ProductFilterInput){
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have enum used here?

*
* @return string[]
* @return array $types
* name string
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this common style for documenting array structure? Usually I see
['name' => 'example value', 'type' = 'example value']

* @return string[]
* @return array $types
* name string
* type string
*/
public function getDeclaredTypeNames() : array
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we rename this method, to getDeclaredTypes, for example

}

/**
* Create type object based off array of configured GraphQL InputType data.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Create type object based off array of configured GraphQL InputType data.
* Create input type object based off array of configured GraphQL InputType data.

@@ -8,7 +8,7 @@
namespace Magento\Framework\GraphQl\Config\Element;

/**
* Describes all the configured data of an Output or Input type in GraphQL.
* Class representing 'type' GraphQL config element.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Class representing 'type' GraphQL config element.
* Class representing 'type' GraphQL config elements.

*
* @return string[]
*/
private function getVariables(array $variables): array
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private function getVariables(array $variables): array
private function formatVariables(array $variables): array

or maybe extractVariables

$typesImplementors = [];
foreach ($this->config->getDeclaredTypeNames() as $name) {
$typesImplementors [] = $this->outputMapper->getOutputType($name);
foreach ($this->config->getDeclaredTypeNames() as $type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please create a ticket not to load all types, but just the ones which do not have references from Query/Mutation types

$configElementClass = get_class($configElement);
if (!isset($this->configToTypeMap[$configElementClass])) {
throw new GraphQlInputException(
new Phrase("Type for '{$configElementClass}' has not declared.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
new Phrase("Type for '{$configElementClass}' has not declared.")
new Phrase("No mapping to Webonyx type is declared for '{$configElementClass}' config element type.")

@magento-engcom-team
Copy link
Contributor

@Aquarvin thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@ghost
Copy link

ghost commented Jan 23, 2019

Hi @Aquarvin, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants