Mirage cannot include non-camel cased model attributes when using json-api-serializer about miragejs HOT 19 CLOSED

Here's a simplified example:

The relevant part is the typeKeyForModel, keyForAttribute and keyForRelationship hooks on the JSONAPISerializer:

application: JSONAPISerializer.extend({
  typeKeyForModel(model) {
    return snakeCase(model.modelName);
  },
  keyForAttribute(key) {
    return snakeCase(key);
  },
  keyForRelationship(key) {
    return snakeCase(key);
  }
})

Does that response look like what you're after?

from miragejs.

@btrude Yes – as I wrote in my SO answer, Mirage needs to make assumptions about attribute and relationship name formatting within the confines of Mirage's data layer. The format of your API shouldn't affect how Mirage's data layer works – that's what the Serializer layer is for.

If you're working with Mirage in your tests, for example, you shouldn't do

server.create('user', {
  first_name: 'Barry'
});

just because your API is snake-cased. There are too many formatting decisions around APIs to have them leak into Mirage's model & data layer. For example, APIs sometimes include a root, and sometimes don't. Therefore, Mirage's architecture is designed to keep all of these concerns within the serializer layer, and to keep Mirage's data layer using the camelCase convention everywhere.

So, that line in the code does transform the keys into camelCase, because Mirage is expecting to find them that way.

If your API sends snake-cased attributes for certain models, I would have those models share a customized Serializer that uses the keyFor* hooks to snake-case those attributes. If you need an example of this I am happy to show you! But you definitely shouldn't override _addPrimaryModelToRequestedIncludesGraph as it's not public API and will likely break your app during a future upgrade.

from miragejs.

If your API sends snake-cased attributes for certain models, I would have those models share a customized Serializer that uses the keyFor* hooks to snake-case those attributes. If you need an example of this I am happy to show you! But you definitely shouldn't override _addPrimaryModelToRequestedIncludesGraph as it's not public API and will likely break your app during a future upgrade.

I would definitely appreciate an example because the line I am calling out explicitly overrides any transformations that would have been done in the keyFor* functions making it impossible (as far as my limited understanding tells me) to override this behavior. Our serializer prior to running into this issue already made use of those functions to reconcile the shape of our data with mirage's expectations - it is only when we need to include relationships that are not camel cased on the model that we run into this issue (which admittedly is not often).

from miragejs.

Sure I can help with an example. Could you post a complete sample JSON response for exactly what you're trying to mock?

from miragejs.

Thank you @samselikoff I really appreciate the help!

In this case we are making a request to:
/api/orders?include=for_reservation,for_reservation.spot,user,order_lines

and expecting to get back something looking like this (attributes removed for readability):

{ 
   "data":[ 
      { 
         "type":"orders",
         "id":"1",
         "attributes":{},
         "relationships":{ 
            "billing_address":{ 
               "data":null
            },
            "broker":{ 
               "data":{ 
                  "type":"users",
                  "id":"12998"
               }
            },
            "cart":{ 
               "data":{ 
                  "type":"carts",
                  "id":"1448700"
               }
            },
            "for_reservation":{ 
               "data":{ 
                  "type":"reservations",
                  "id":"7755412"
               }
            },
            "order_lines":{ 
               "data":[ 
                  { 
                     "type":"order_lines",
                     "id":"1421453"
                  }
               ]
            },
            "originating_partner":{ 
               "data":{ 
                  "type":"partners",
                  "id":"800"
               }
            },
            "user":{ 
               "data":{ 
                  "type":"users",
                  "id":"560972"
               }
            }
         }
      }
   ],
   "included":[ 
      { 
         "type":"spots",
         "id":"26279",
         "attributes":{},
         "relationships":{ 
            "layout":{ 
               "data":{ 
                  "type":"layouts",
                  "id":"3309"
               }
            },
            "classroom":{ 
               "data":{ 
                  "type":"classrooms",
                  "id":"3346"
               }
            },
            "location":{ 
               "data":{ 
                  "type":"locations",
                  "id":"9594"
               }
            },
            "region":{ 
               "data":{ 
                  "type":"regions",
                  "id":"9642"
               }
            },
            "site":{ 
               "data":{ 
                  "type":"sites",
                  "id":"12009"
               }
            },
            "spot_type":{ 
               "data":{ 
                  "type":"spot_types",
                  "id":"3"
               }
            }
         }
      },
      { 
         "type":"reservations",
         "id":"7755412",
         "attributes":{},
         "relationships":{ 
            "booked_on_behalf_of_user":{ 
               "data":{ 
                  "type":"users",
                  "id":"560972"
               }
            },
            "broker":{ 
               "data":{ 
                  "type":"users",
                  "id":"12998"
               }
            },
            "class_session":{ 
               "data":{ 
                  "type":"class_sessions",
                  "id":"389631"
               }
            },
            "credit_transactions":{ 
               "data":[ 
                  { 
                     "type":"credit_transactions",
                     "id":"3195348"
                  }
               ]
            },
            "membership_transactions":{ 
               "data":[]
            },
            "spot":{ 
               "data":{ 
                  "type":"spots",
                  "id":"26279"
               }
            },
            "user":{ 
               "data":{ 
                  "type":"users",
                  "id":"560972"
               }
            }
         }
      },
      { 
         "type":"order_lines",
         "id":"1421453",
         "attributes":{},
         "relationships":{ 
            "order":{ 
               "data":{ 
                  "type":"orders",
                  "id":"1"
               }
            },
            "product":{ 
               "data":{ 
                  "type":"child_products",
                  "id":"7108"
               }
            }
         }
      },
      { 
         "type":"users",
         "id":"560972",
         "attributes":{},
         "relationships":{ 
            "last_region":{ 
               "data":null
            },
            "home_location":{ 
               "data":{ 
                  "type":"locations",
                  "id":"9627"
               }
            }
         }
      }
   ]
}

from miragejs.

Ok. So you want to camelize both the type name and the relationship names, correct?

(Sorry about the delayed response! Busy holidays.)

from miragejs.

Ok. So you want to camelize both the type name and the relationship names, correct?

(Sorry about the delayed response! Busy holidays.)

@samselikoff No worries, thanks for getting back to me!

The issue is specifically with type names in the array of included relationships but we do also need to camelize type/relationship names across the board.

from miragejs.

Here's a simplified example:

The relevant part is the typeKeyForModel, keyForAttribute and keyForRelationship hooks on the JSONAPISerializer:

application: JSONAPISerializer.extend({
  typeKeyForModel(model) {
    return snakeCase(model.modelName);
  },
  keyForAttribute(key) {
    return snakeCase(key);
  },
  keyForRelationship(key) {
    return snakeCase(key);
  }
})

Does that response look like what you're after?

So if I'm understanding correctly we should be setting up our serializer as such and then instantiating factories with camel cased attributes instead of what we were trying to accomplish previously which is instantiating factories with snake case attributes and then using the serializer to camelize them, correct?

from miragejs.

You got it 👍 Camel case everywhere in Mirage JS code, since it's JS and that's just normal JS convention. Then serializers to turn Mirage's data into the format your app expects based on your production API.

from miragejs.

You got it 👍 Camel case everywhere in Mirage JS code, since it's JS and that's just normal JS convention. Then serializers to turn Mirage's data into the format your app expects based on your production API.

Cool, thanks for clarifying. In trying to implement this in our codebase I'm now running into the issue of our models not having valid associations because we're now passing all of our factory attributes in camelized. Specifically I'm getting the following error:

Mirage: You're trying to create a reservation model and you passed in a model:class-session(95710) under the classSession key, but you haven't defined that key as an association on your model

Is there another serializer hook that I'm missing to handle this case?

from miragejs.

That error means you’re creating data somewhere with the ORM, and passing in ‘class-session’ as a model name where mirage expects ‘classSession’.

Typically in mirage you create data either with factories in the ‘seeds()‘ method, in tests, or in POST or PUT route handlers. So it could be you or it could be one of Mirage’s shorthands.

Depending on which, you might need to further customize the serializer. But I don’t think you will. It’s probably from a factory somewhere.

from miragejs.

Depending on which, you might need to further customize the serializer. But I don’t think you will. It’s probably from a factory somewhere.

In this case the error is being thrown from within a test like this:

const serverClassSession = server.create('class-session');
server.create('reservation', {
    classSession: serverClassSession,
});

from miragejs.

Hm ok I don't think dasherize vs. camelCase is the problem here... can you show me your reservation model definition?

from miragejs.

Hm ok I don't think dasherize vs. camelCase is the problem here... can you show me your reservation model definition?

It occurs to me that I should have mentioned at some point that we are using Ember data and that mirage infers all of our models from our ember data models. I would guess that the suggestion here will be to create mirage-specific models that override the snake casing on our ember data models.

from miragejs.

Hm, yes that might be it. Though I would suggest to refactor your ED models, if it's not too late. Your API format should never leak into your ORM like that, ideally.

from miragejs.

Hm, yes that might be it.

Testing it now and creating a model with camelized attributes for reservation resolves the errors in this case.

Though I would suggest to refactor your ED models, if it's not too late. Your API format should never leak into your ORM like that, ideally.

Completely agree, we're working with older code and an extremely naive implementation of mirage so hopefully we can get to that soon. Thanks again for all your help, I think this should solve everything.

from miragejs.

Awesome. Good luck + let me know if you need more help!

from miragejs.

i want to remove attributes from response but need data whatever inside it. any idea how to do that?

from miragejs.

Which attributes do you want to remove? and Im not sure I understand what you mean by need data inside of it. If you need the data, why are you removing them?

The response is crafted by the mirage serializer. For what ever you need most likely you will need to create a serializer and customize that

from miragejs.