From b7c958c39b06e70717847cd9dc647690f5f5af8c Mon Sep 17 00:00:00 2001
From: Matchu <matchu1993@gmail.com>
Date: Tue, 1 Sep 2020 17:00:27 -0700
Subject: [PATCH] don't restrict zones if the item is not compatible

Previously, if you switched species/color such that one of your items was no longer compatible, we _would_ still apply its zone restrictions to the visible layer set.

In this change, we fix that server-side, since I think it makes the most sense for an empty appearance to be truly empty!
---
 src/server/index.js                 | 12 ++++-
 src/server/loaders.js               | 51 +++++++++++----------
 src/server/query-tests/Item.test.js | 69 +++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+), 25 deletions(-)

diff --git a/src/server/index.js b/src/server/index.js
index acfb4a3..e859408 100644
--- a/src/server/index.js
+++ b/src/server/index.js
@@ -360,7 +360,17 @@ const resolvers = {
 
       return allSwfAssets.filter((sa) => sa.url.endsWith(".swf"));
     },
-    restrictedZones: async ({ item: { id: itemId } }, _, { itemLoader }) => {
+    restrictedZones: async (
+      { item: { id: itemId }, bodyId },
+      _,
+      { itemSwfAssetLoader, itemLoader }
+    ) => {
+      // Check whether this appearance is empty. If so, restrict no zones.
+      const allSwfAssets = await itemSwfAssetLoader.load({ itemId, bodyId });
+      if (allSwfAssets.length === 0) {
+        return [];
+      }
+
       const item = await itemLoader.load(itemId);
       return getRestrictedZoneIds(item.zonesRestrict).map((id) => ({ id }));
     },
diff --git a/src/server/loaders.js b/src/server/loaders.js
index 977a8ce..681b631 100644
--- a/src/server/loaders.js
+++ b/src/server/loaders.js
@@ -276,38 +276,41 @@ const buildSwfAssetLoader = (db) =>
   });
 
 const buildItemSwfAssetLoader = (db, loaders) =>
-  new DataLoader(async (itemAndBodyPairs) => {
-    const conditions = [];
-    const values = [];
-    for (const { itemId, bodyId } of itemAndBodyPairs) {
-      conditions.push(
-        "(rel.parent_id = ? AND (sa.body_id = ? OR sa.body_id = 0))"
-      );
-      values.push(itemId, bodyId);
-    }
+  new DataLoader(
+    async (itemAndBodyPairs) => {
+      const conditions = [];
+      const values = [];
+      for (const { itemId, bodyId } of itemAndBodyPairs) {
+        conditions.push(
+          "(rel.parent_id = ? AND (sa.body_id = ? OR sa.body_id = 0))"
+        );
+        values.push(itemId, bodyId);
+      }
 
-    const [rows, _] = await db.execute(
-      `SELECT sa.*, rel.parent_id FROM swf_assets sa
+      const [rows, _] = await db.execute(
+        `SELECT sa.*, rel.parent_id FROM swf_assets sa
        INNER JOIN parents_swf_assets rel ON
          rel.parent_type = "Item" AND
          rel.swf_asset_id = sa.id
        WHERE ${conditions.join(" OR ")}`,
-      values
-    );
+        values
+      );
 
-    const entities = rows.map(normalizeRow);
+      const entities = rows.map(normalizeRow);
 
-    for (const swfAsset of entities) {
-      loaders.swfAssetLoader.prime(swfAsset.id, swfAsset);
-    }
+      for (const swfAsset of entities) {
+        loaders.swfAssetLoader.prime(swfAsset.id, swfAsset);
+      }
 
-    return itemAndBodyPairs.map(({ itemId, bodyId }) =>
-      entities.filter(
-        (e) =>
-          e.parentId === itemId && (e.bodyId === bodyId || e.bodyId === "0")
-      )
-    );
-  });
+      return itemAndBodyPairs.map(({ itemId, bodyId }) =>
+        entities.filter(
+          (e) =>
+            e.parentId === itemId && (e.bodyId === bodyId || e.bodyId === "0")
+        )
+      );
+    },
+    { cacheKeyFn: ({ itemId, bodyId }) => `${itemId},${bodyId}` }
+  );
 
 const buildPetSwfAssetLoader = (db, loaders) =>
   new DataLoader(async (petStateIds) => {
diff --git a/src/server/query-tests/Item.test.js b/src/server/query-tests/Item.test.js
index f91c10a..87fd14f 100644
--- a/src/server/query-tests/Item.test.js
+++ b/src/server/query-tests/Item.test.js
@@ -152,6 +152,75 @@ describe("Item", () => {
     `);
   });
 
+  it("returns empty appearance for incompatible items", async () => {
+    const res = await query({
+      query: gql`
+        query {
+          items(ids: ["38912"]) {
+            id
+            name
+
+            appearanceOn(speciesId: "1", colorId: "8") {
+              layers {
+                id
+              }
+
+              # Pay particular attention to this: normally this item restricts
+              # zones, but not when the appearance is empty!
+              restrictedZones {
+                id
+              }
+            }
+          }
+        }
+      `,
+    });
+
+    expect(res).toHaveNoErrors();
+    expect(res.data).toMatchInlineSnapshot(`
+      Object {
+        "items": Array [
+          Object {
+            "appearanceOn": Object {
+              "layers": Array [],
+              "restrictedZones": Array [],
+            },
+            "id": "38912",
+            "name": "Zafara Agent Robe",
+          },
+        ],
+      }
+    `);
+    expect(getDbCalls()).toMatchInlineSnapshot(`
+      Array [
+        Array [
+          "SELECT * FROM item_translations WHERE item_id IN (?) AND locale = \\"en\\"",
+          Array [
+            "38912",
+          ],
+        ],
+        Array [
+          "SELECT * FROM pet_types WHERE (species_id = ? AND color_id = ?)",
+          Array [
+            "1",
+            "8",
+          ],
+        ],
+        Array [
+          "SELECT sa.*, rel.parent_id FROM swf_assets sa
+             INNER JOIN parents_swf_assets rel ON
+               rel.parent_type = \\"Item\\" AND
+               rel.swf_asset_id = sa.id
+             WHERE (rel.parent_id = ? AND (sa.body_id = ? OR sa.body_id = 0))",
+          Array [
+            "38912",
+            "93",
+          ],
+        ],
+      ]
+    `);
+  });
+
   it("skips appearance data for audio assets", async () => {
     const res = await query({
       query: gql`