Skip to content
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

Adds a Node interface to Sale #795

Merged
merged 3 commits into from
Oct 28, 2017
Merged

Conversation

ashfurrow
Copy link
Contributor

Continues work from #794. Paired with @orta. This is required to get Relay working with Emission for sales.

@ashfurrow
Copy link
Contributor Author

I will address the test failures on Monday.

@ashfurrow
Copy link
Contributor Author

This will probably be useful for getting the tests to pass: https://medium.com/@boriscoder/the-hidden-power-of-jest-matchers-f3d86d8101b0

@@ -12,6 +13,8 @@ import { connectionFromArraySlice, connectionDefinitions } from "graphql-relay"
import { amount } from "schema/fields/money"
import { exclude } from "lib/helpers"
import { map } from "lodash"
Copy link
Contributor

Choose a reason for hiding this comment

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

You can add has here and avoid the import below.

@@ -47,6 +47,7 @@ const SupportedTypes = {
"./partner",
"./partner_show",
"./show",
"./sale",
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -59,6 +62,8 @@ const saleArtworkConnection = connectionDefinitions({

const SaleType = new GraphQLObjectType({
name: "Sale",
interfaces: [NodeInterface],
isTypeOf: obj => _.has(obj, "is_auction") && _.has(obj, "currency"),
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 seems reasonable!

@mzikherman mzikherman merged commit 80829d0 into artsy:master Oct 28, 2017
@mzikherman
Copy link
Contributor

Nice!

I've experienced that before- adding the NodeInterface and isTypeOf sometimes requires updating mocked sale fixtures with those newly required fields.

@ashfurrow ashfurrow deleted the auctions-relay branch October 28, 2017 20:55
@ashfurrow
Copy link
Contributor Author

Thanks! I've written up some notes and will add what I learned to the wiki for next time.

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.

3 participants