Reaction: flatRateFulfillmentMethods graphql query does not match the shipping schema

[alex-sh-li]

Issue Description

Possible Solution

change the schema of graphql ? or mongo ?

Versions

3.4.0

[alex-sh-li]

here is the mongo schema

[sheunglaili]

alter flatRateFulfillmentMethods in reaction-api using aggregation with following pipeline should work

[
      {
        $match: {
          shopId
        }
      },
      {
        $unwind: "$methods"
      },
      {
        $replaceRoot: {
          newRoot: "$methods"
        }
      },
      {
        $set: {
          "isEnabled":"$enabled"
        }
      }
    ]

also change getPaginatedResponse to getPaginatedResponseFromAggregate in flatRateFulfillment resolvers as well

[loan-laux]

@sheunglaili @alex-sh-li The fact that the GraphQL schema doesn't match the database schema is normal. Both of them have their own structure. The GraphQL one is aimed at being more efficient in terms of accessing data and using it in a UI, and the database one aims at being efficient for storing data in a way that makes the most sense.

That being said, are you getting any error when trying to run your aggregate? Do you have more details about what you're trying to do and where exactly it fails?

[sheunglaili]

@sheunglaili @alex-sh-li The fact that the GraphQL schema doesn't match the database schema is normal. Both of them have their own structure. The GraphQL one is aimed at being more efficient in terms of accessing data and using it in a UI, and the database one aims at being efficient for storing data in a way that makes the most sense.

That being said, are you getting any error when trying to run your aggregate? Do you have more details about what you're trying to do and where exactly it fails?

Yes. I understand. But In the flatRatefulfillmethods Query. the method just return the result of Shipping collection query which make the query not functional.

[sheunglaili]

@sheunglaili @alex-sh-li The fact that the GraphQL schema doesn't match the database schema is normal. Both of them have their own structure. The GraphQL one is aimed at being more efficient in terms of accessing data and using it in a UI, and the database one aims at being efficient for storing data in a way that makes the most sense.

That being said, are you getting any error when trying to run your aggregate? Do you have more details about what you're trying to do and where exactly it fails?

I make it work by altering the query into the aggregate I mentioned above.

[loan-laux]

@sheunglaili Doesn't src/plugins/shipping-rates/getFulfillmentMethodsWithQuotes.js do what you're trying to do? It filters shipping rates like this:

const shippingRateDocs = await Shipping.find({
    "shopId": commonOrder.shopId,
    "provider.enabled": true
}).toArray();

[sheunglaili]

@sheunglaili Doesn't src/plugins/shipping-rates/getFulfillmentMethodsWithQuotes.js do what you're trying to do? It filters shipping rates like this:

const shippingRateDocs = await Shipping.find({
    "shopId": commonOrder.shopId,
    "provider.enabled": true
}).toArray();

No. If you try calling the graphql method flatRateFulfillmentMethods with record in database , it will prompt you error that the fulfillType field is null while it's a non-null field. It's because the schema returned doesn't match

[loan-laux]

This logic is usable by querying for getFulfillmentMethodsWithQuotes. This is what I was referring to. Even though it also returns rates with it, it will subsequently return any enabled shipping provider for a given shop. Wouldn't this do what you're looking for?

[sheunglaili]

And I think you are looking at the wrong place (?) I am currently using 3.4.0 . The problem occur in the included plugin shipping-rates. I think it's move into a independent npm packages

[sheunglaili]

This logic is usable by querying for getFulfillmentMethodsWithQuotes. This is what I was referring to. Even though it also returns rates with it, it will subsequently return any enabled shipping provider for a given shop. Wouldn't this do what you're looking for?

I was working on the reaction-admin shipping setting part. which currently depends on meteor.
The api should return a page of fulfillment method instead of whole shipping according to docs. Also , In the graphql schema , It state that it's should return a page of fulfillment method. But meanwhile the resolver is returning a list of shipping

[loan-laux]

@sheunglaili I'm on 3.6.0 with the latest trunk. src/plugins/shipping-rates/getFulfillmentMethodsWithQuotes.js is still indeed part of reaction and not yet moved into its own NPM package.

Okay, now I understand better what you're trying to do. So this issue is a feature request to add shopId and enabled filters to the flatRateFulfillmentMethods query, right?

[sheunglaili]

I think you are getting it wrong 🤒 . The issue occured in reaction/src/plugins/shipping-rates/queries/flatRateFulfillmentMethods.js which should be in a independent npm package now.

[sheunglaili]

I think you are getting it wrong 🤒 . The issue occured in reaction/src/plugins/shipping-rates/queries/flatRateFulfillmentMethods.js which should be in a independent npm package now.

This queries should return methods. but Instead it return the whole shipping which won't fit into the graphql schema

[sheunglaili]

type FlatRateFulfillmentMethod
implements Node {
_id: ID!
cost: Float
fulfillmentTypes: [FulfillmentType]!
group: String!
handling: Float!
isEnabled: Boolean!
label: String!
name: String!
rate: Float!
shop: Shop!
}

The schema state that it should return this instead of the whole shipping entity.

[loan-laux]

@sheunglaili Just to clarify any confusion: flatRateFulfillmentMethods returns a FlatRateFulfillmentMethodConnection, which itself contains an array of FlatRateFulfillmentMethod in the nodes field.

Now, I've just tried to run the query and I got a "Cannot return null for non-nullable field FlatRateFulfillmentMethod.label." error. Is this what you're referring to?

[sheunglaili]

@sheunglaili Just to clarify any confusion: flatRateFulfillmentMethods returns a FlatRateFulfillmentMethodConnection, which itself contains an array of FlatRateFulfillmentMethod in the nodes field.

Now, I've just tried to run the query and I got a "Cannot return null for non-nullable field FlatRateFulfillmentMethod.label." error. Is this what you're referring to?

Yeah. That's what I'm talking about

[loan-laux]

Great! Next time, it would be better if you could use this repo's issue template when reporting a bug, and simply refer to the error message you're getting. It always helps us contributors to narrow down what's going on.

I'll see what I can do for this. Thanks for the report!

[sheunglaili]

Great! Next time, it would be better if you could use this repo's issue template when reporting a bug, and simply refer to the error message you're getting. It always helps us contributors to narrow down what's going on.

I'll see what I can do for this. Thanks for the report!

Sure great. It's my first time reporting issue. Will do next time ! Thank you for your time! .You can reference the aggregate above. I used it as a solution! thanks

[loan-laux]

@sheunglaili If you feel like it, feel free to PR your solution directly. I probably will take a bit of time to solve this since I have quite a lot of work on my plate.

[sheunglaili]

PR

sure. let me try

[sheunglaili]

I have created a pr reactioncommerce/api-plugin-shipments-flat-rate#2. please have a look when you have time !

[loan-laux]

@sheunglaili I can't review PRs as I'm not part of the core team, but you should ask @aldeed for example. Also, thanks for taking the time to do that!