[Artist] Only include displayable shows.

[alloy]

Depends on https://github.com/artsy/gravity/pull/10566
Fixes #473 & artsy/eigen#1873

[mzikherman]

I have a feeling this is now going to break/undo the work to make reference shows visible on the Artist page.

For instance: https://github.com/artsy/force/blob/master/apps/artist/queries/cv.coffee

We do want to be able to return those reference shows for the CV listing.

It seems like with the filter you added in https://github.com/artsy/gravity/pull/10566, displayable: false will return all shows.

So is the correct thing to make Metaphysics allow that to be specified by a client (but can default to true), and then we have to update Force/Microgravity to pass in false where it wants to be able to fetch/display non-displayable shows?

Basically as-is, I think this will no longer allow those reference shows to show up on the front-end.

[alloy]

I see what you mean, good catch 👍

[alloy]

It seems like with the filter you added in https://github.com/artsy/gravity/pull/10566, displayable: false will return all shows.

No it has to be nil, so undefined, for all shows to show up.

[mzikherman]

No it has to be nil, so undefined, for all shows to show up.

Ah right, nil in the ES query, which you get by leaving the query param out entirely. ?displayable=true is only displayable shows, and ?displayable=false is only non displayable shows. Got it.

[alloy]

Ok, so this is embarrassing, I don’t actually believe that displayable is the real solution here. In retrospect the issue is that there’s no partner information for these shows. I’ve updated the PR to only guard against that (and have removed the use of the displayable param).

I don’t actually think that guarding here is the real proper fix, because a lot of code in PartnerShow will assume a partner and it seems kind of obvious that there should be such data. Could it be that this is a case that you mentioned where the partner might have been removed from Galaxy?

[mzikherman]

Oy, now I'm confused.

You can have shows that have a galaxy_partner_id, and won't have any partner_id or partner in the JSON.

The new Show schema allows for that, but there's lots of copy-pasta from PartnerShow, so I think we should update that to be more defensive in all the places a partner is assumed, and generally switch to using that. For instance

metaphysics/schema/show.js

Lines 210 to 240 in 6bbb98f

	artworks: {
	type: new GraphQLList(Artwork.type),
	args: {
	size: {
	type: GraphQLInt,
	description: 'Number of artworks to return',
	defaultValue: 25,
	},
	published: {
	type: GraphQLBoolean,
	defaultValue: true,
	},
	page: {
	type: GraphQLInt,
	defaultValue: 1,
	},
	all: {
	type: GraphQLBoolean,
	default: false,
	},
	for_sale: {
	type: GraphQLBoolean,
	default: false,
	},
	exclude: {
	type: new GraphQLList(GraphQLString),
	description: 'List of artwork IDs to exclude from the response (irrespective of size)',
	},
	},
	resolve: (show, options) => {
	const path = `partner/${show.partner.id}/show/${show.id}/artworks`;

doesn't make sense (currently) for one of these shows.

So the issue is that shows w/o partners (and only a galaxy_partner_id set) are coming back from the API? And causing crashes in querying other fields that do partner.id? In that case, since those partner-less shows are something we support and are valid upstream in Gravity/API-land, I think the right fix (which I should have done/realized but totally didn't!), is to make things more defensive to allow for that, as you've done here for cover_image.

There are probably a few other places in the PartnerShow and Show schemas where a similar defense should technically exist, where a similar crash may lurk.

I think the issue we were looking at with an older show with unpublished works coming back was something different however, or was that essentially this crash, where a show w/o a partner was being returned. AFAIK if a partner is deleted their shows will not be deleted and might still return. But with the advent of reference shows and fuller CV's, there'll be a lot more 'partner-less' shows.