feat(#4848) - Return only isVisible Tags, unless admin

imports/plugins/core/catalog/server/no-meteor/queries/tag.js

@@ -5,18 +5,33 @@
  * @summary query the Tags collection and return a tag by tag ID or slug
  * @param {Object} context - an object containing the per-request state
  * @param {String} slugOrId - ID or slug of tag to query
+ * @param {Boolean} [params.shouldIncludeInvisible] - Whether or not to include `isVisible=true` tags. Default is `false`
  * @return {Object} - A Tag document if one was found
  */
-export default async function tag(context, slugOrId) {
-  const { collections } = context;
-
+export default async function tag(context, slugOrId, { shouldIncludeInvisible = false } = {}) {
+  const { collections, shopId: contextShopId, userHasPermission } = context;
   const { Tags } = collections;
-  const query = {
-    $or: [
-      { _id: slugOrId },
-      { slug: slugOrId }
+  let query = {
+    $and: [
+      { isVisible: true },
+      { $or: [{ _id: slugOrId }, { slug: slugOrId }] }
     ]
   };
 
+  if (userHasPermission(["owner", "admin"], contextShopId)) {
+    if (shouldIncludeInvisible === true) {
+      query = {
+        $and: [
+          { isVisible: { $in: [false, true] } },
+          { $or: [{ _id: slugOrId }, { slug: slugOrId }] }
+        ]
+      };
+    } else {
+      query;
+    }
+  } else {
+    query;
+  }
+
   return Tags.findOne(query);
 }

imports/plugins/core/catalog/server/no-meteor/queries/tags.js

@@ -7,17 +7,33 @@
  * @param {String} shopId - ID of shop to query
  * @param {Object} [params] - Additional options for the query
  * @param {Boolean} [params.isTopLevel] - If set, look for `isTopLevel` matching this value
- * @param {Boolean} [params.shouldIncludeDeleted] - Whether or not to include `isDeleted=true` tags. Default is `false`
+ * @param {Boolean} [params.shouldIncludeDeleted] - Admin only. Whether or not to include `isDeleted=true` tags. Default is `false`
+ * @param {Boolean} [params.shouldIncludeInvisible] - Admin only. Whether or not to include `isVisible=false` tags.  Default is `false`.
  * @return {Promise<MongoCursor>} - A MongoDB cursor for the proper query
  */
-export default async function tags(context, shopId, { shouldIncludeDeleted = false, isTopLevel } = {}) {
+export default async function tags(context, shopId, { shouldIncludeDeleted = false, isTopLevel, shouldIncludeInvisible = false } = {}) {
   const { collections } = context;
 
   const { Tags } = collections;
   const query = { shopId };
 
   if (isTopLevel === false || isTopLevel === true) query.isTopLevel = isTopLevel;
-  if (shouldIncludeDeleted !== true) query.isDeleted = { $ne: true };
+
+  if (context.userHasPermission(["owner", "admin"], shopId)) {
+    if (shouldIncludeDeleted === true) {
+      query.isDeleted = { $in: [false, true] };
+    } else {
+      query.isDeleted = false;
+    }
+    if (shouldIncludeInvisible === true) {
+      query.isVisible = { $in: [false, true] };
+    } else {
+      query.isVisible = true;
+    }
+  } else {
+    query.isDeleted = false;
+    query.isVisible = true;
+  }
 
   return Tags.find(query);
 }

imports/plugins/core/catalog/server/no-meteor/queries/tags.test.js

@@ -7,44 +7,95 @@ beforeEach(() => {
   jest.resetAllMocks();
 });
 
-test("default", async () => {
+test("default - isVisible only, excludes isDeleted", async () => {
   mockContext.collections.Tags.find.mockReturnValueOnce("CURSOR");
+  mockContext.userHasPermission.mockReturnValueOnce(true);
   const result = await tags(mockContext, mockShopId);
-  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isDeleted: { $ne: true } });
+  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isDeleted: false, isVisible: true });
   expect(result).toBe("CURSOR");
 });
 
-test("include deleted", async () => {
+test("explicit - isVisible only, excludes not isVisible", async () => {
   mockContext.collections.Tags.find.mockReturnValueOnce("CURSOR");
-  const result = await tags(mockContext, mockShopId, { shouldIncludeDeleted: true });
-  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId });
+  mockContext.userHasPermission.mockReturnValueOnce(true);
+  const result = await tags(mockContext, mockShopId, { shouldIncludeInvisible: false });
+  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isDeleted: false, isVisible: true });
+  expect(result).toBe("CURSOR");
+});
+
+test("explicit - isVisible only, excludes isDeleted", async () => {
+  mockContext.collections.Tags.find.mockReturnValueOnce("CURSOR");
+  mockContext.userHasPermission.mockReturnValueOnce(true);
+  const result = await tags(mockContext, mockShopId, { shouldIncludeDeleted: false });
+  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isDeleted: false, isVisible: true });
+  expect(result).toBe("CURSOR");
+});
+
+test("explicit - isVisible only, excludes isDeleted and not isVisible", async () => {
+  mockContext.collections.Tags.find.mockReturnValueOnce("CURSOR");
+  mockContext.userHasPermission.mockReturnValueOnce(true);
+  const result = await tags(mockContext, mockShopId, { shouldIncludeInvisible: false, shouldIncludeDeleted: false });
+  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isDeleted: false, isVisible: true });
   expect(result).toBe("CURSOR");
 });
 
 test("top-level only", async () => {
   mockContext.collections.Tags.find.mockReturnValueOnce("CURSOR");
+  mockContext.userHasPermission.mockReturnValueOnce(true);
   const result = await tags(mockContext, mockShopId, { isTopLevel: true });
-  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isDeleted: { $ne: true }, isTopLevel: true });
+  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isDeleted: false, isTopLevel: true, isVisible: true });
   expect(result).toBe("CURSOR");
 });
 
 test("non-top-level only", async () => {
   mockContext.collections.Tags.find.mockReturnValueOnce("CURSOR");
+  mockContext.userHasPermission.mockReturnValueOnce(true);
   const result = await tags(mockContext, mockShopId, { isTopLevel: false });
-  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isDeleted: { $ne: true }, isTopLevel: false });
+  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isDeleted: false, isTopLevel: false, isVisible: true });
+  expect(result).toBe("CURSOR");
+});
+
+test("include isDeleted", async () => {
+  mockContext.collections.Tags.find.mockReturnValueOnce("CURSOR");
+  mockContext.userHasPermission.mockReturnValueOnce(true);
+  const result = await tags(mockContext, mockShopId, { shouldIncludeDeleted: true });
+  expect(mockContext.userHasPermission).toHaveBeenCalledWith(["owner", "admin"], mockShopId);
+  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isVisible: true, isDeleted: { $in: [false, true] } });
   expect(result).toBe("CURSOR");
 });
 
-test("top-level only, including deleted", async () => {
+test("top-level only, include isDeleted", async () => {
   mockContext.collections.Tags.find.mockReturnValueOnce("CURSOR");
+  mockContext.userHasPermission.mockReturnValueOnce(true);
   const result = await tags(mockContext, mockShopId, { isTopLevel: true, shouldIncludeDeleted: true });
-  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isTopLevel: true });
+  expect(mockContext.userHasPermission).toHaveBeenCalledWith(["owner", "admin"], mockShopId);
+  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isTopLevel: true, isVisible: true, isDeleted: { $in: [false, true] } });
   expect(result).toBe("CURSOR");
 });
 
-test("non-top-level only, including deleted", async () => {
+test("non-top-level only, include isDeleted", async () => {
   mockContext.collections.Tags.find.mockReturnValueOnce("CURSOR");
+  mockContext.userHasPermission.mockReturnValueOnce(true);
   const result = await tags(mockContext, mockShopId, { isTopLevel: false, shouldIncludeDeleted: true });
-  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isTopLevel: false });
+  expect(mockContext.userHasPermission).toHaveBeenCalledWith(["owner", "admin"], mockShopId);
+  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isTopLevel: false, isVisible: true, isDeleted: { $in: [false, true] } });
+  expect(result).toBe("CURSOR");
+});
+
+test("include not visible - by an admin", async () => {
+  mockContext.collections.Tags.find.mockReturnValueOnce("CURSOR");
+  mockContext.userHasPermission.mockReturnValueOnce(true);
+  const result = await tags(mockContext, mockContext.shopId, { shouldIncludeInvisible: true });
+  expect(mockContext.userHasPermission).toHaveBeenCalledWith(["owner", "admin"], mockContext.shopId);
+  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockContext.shopId, isDeleted: false, isVisible: { $in: [false, true] } });
+  expect(result).toBe("CURSOR");
+});
+
+test("include not visible and only topLevel - by an admin", async () => {
+  mockContext.collections.Tags.find.mockReturnValueOnce("CURSOR");
+  mockContext.userHasPermission.mockReturnValueOnce(true);
+  const result = await tags(mockContext, mockShopId, { shouldIncludeInvisible: true, isTopLevel: true });
+  expect(mockContext.userHasPermission).toHaveBeenCalledWith(["owner", "admin"], mockShopId);
+  expect(mockContext.collections.Tags.find).toHaveBeenCalledWith({ shopId: mockShopId, isDeleted: false, isTopLevel: true, isVisible: { $in: [false, true] } });
   expect(result).toBe("CURSOR");
 });

imports/plugins/core/core/server/fixtures/products.js

@@ -156,6 +156,7 @@ export default function () {
    * @property {String} name - `"Tag"`
    * @property {String} slug - `"tag"`
    * @property {Number} position - `_.random(0, 100000)`
+   * @property {Boolean} isVisible - `true`
    * @property {Boolean} isTopLevel - `true`
    * @property {String} shopId - `getShop()._id`
    * @property {Date} createdAt - `faker.date.past()`
@@ -166,6 +167,7 @@ export default function () {
     slug: "tag",
     position: _.random(0, 100000),
     //  relatedTagIds: [],
+    isVisible: true,
     isTopLevel: true,
     shopId: getShop()._id,
     createdAt: faker.date.past(),

imports/plugins/core/core/server/no-meteor/resolvers/Query/tag.js

@@ -1,16 +1,27 @@
 import { decodeTagOpaqueId } from "@reactioncommerce/reaction-graphql-xforms/tag";
 
+/**
+ * Arguments passed by the client for a tags query
+ * @typedef {ConnectionArgs} TagConnectionArgs - An object of all arguments that were sent by the client
+ * @memberof Tag/GraphQL
+ * @property {ConnectionArgs} args - An object of all arguments that were sent by the client. {@link ConnectionArgs|See default connection arguments}
+ * @property {String} args.slugOrId - ID or slug of tag to query
+ * @property {Boolean} args.shouldIncludeInvisible - Whether or not to include `isVisible=true` tags. Default is `false`
+ */
+
 /**
  * @name "Query.tag"
  * @method
  * @memberof Tag/GraphQL
  * @summary Returns a tag for a shop, based on tag slug or ID
  * @param {Object} _ - unused
- * @param {Object} args - arguments sent by the client {@link ConnectionArgs|See default connection arguments}
+ * @param {Object} connectionArgs - arguments sent by the client {@link ConnectionArgs|See default connection arguments}
  * @param {Object} context - an object containing the per-request state
  * @return {Promise<Object[]>} Promise that resolves with array of Tag objects
  */
-export default async function tag(_, { slugOrId }, context) {
+export default async function tag(_, connectionArgs, context) {
+  const { slugOrId } = connectionArgs;
+
   let dbTagId;
 
   try {
@@ -19,5 +30,5 @@ export default async function tag(_, { slugOrId }, context) {
     dbTagId = slugOrId;
   }
 
-  return context.queries.tag(context, dbTagId);
+  return context.queries.tag(context, dbTagId, connectionArgs);
 }

imports/plugins/core/core/server/no-meteor/resolvers/Query/tags.js

@@ -7,6 +7,7 @@ import { decodeShopOpaqueId } from "@reactioncommerce/reaction-graphql-xforms/sh
  * @memberof Tag/GraphQL
  * @property {ConnectionArgs} args - An object of all arguments that were sent by the client. {@link ConnectionArgs|See default connection arguments}
  * @property {Boolean} args.shouldIncludeDeleted - If set to true, include deleted. Default false.
+ * @property {Boolean} ags.shouldIncludeInvisible - If set to true, include invisible. Default false.
  * @property {Boolean} args.isTopLevel - If set to a boolean, filter by this.
  * @property {String} args.shopId - The ID of shop to filter tags by
  * @property {Number} args.sortBy - Sort results by a TagSortByField enum value of `_id`, `name`, `position`, `createdAt`, `updatedAt`

imports/plugins/core/core/server/no-meteor/schemas/tag.graphql

@@ -15,6 +15,9 @@ type Tag implements Node & Deletable {
   "If `true`, this tag should be shown at the top level of the tag hierarchy"
   isTopLevel: Boolean!
 
+  "If `true`, this tag's Tag Listing Page should be visible to the public"
+  isVisible: Boolean!
+
   "Arbitrary additional metadata about this tag"
   metafields: [Metafield]
 
@@ -39,11 +42,12 @@ type Tag implements Node & Deletable {
   "The date and time at which this tag was last updated"
   updatedAt: DateTime!
 
-  "A string containing the hero image url for a tag listing page"
+  "A string containing the hero image url for a Tag Listing Page"
   heroMediaUrl: String
 
-  "A string of the title to be displayed on a tag listing page"
+  "A string of the title to be displayed on a Tag Listing Page"
   displayTitle: String
+
 }
 
 "The fields by which you are allowed to sort any query that returns a `TagConnection`"
@@ -70,8 +74,14 @@ type TagConnection implements NodeConnection {
 }
 
 extend type Query {
-  "Returns a tag from a provided tag ID or slug"
-  tag(slugOrId: String): Tag
+  "Returns a tag from a provided tag ID or slug. Tags with isVisible set to false are excluded by default."
+  tag(
+    "Slug or ID of Tag"
+    slugOrId: String,
+
+    "Set to true if you want to include tags that have isVisible set to false"
+    shouldIncludeInvisible: Boolean = false
+  ): Tag
 
   "Returns a paged list of tags for a shop. You must include a shopId when querying."
   tags(
@@ -84,6 +94,9 @@ extend type Query {
     "Set to true if you want soft deleted tags to be included in the response"
     shouldIncludeDeleted: Boolean = false,
 
+    "Set to true if you want to include tags that have isVisible set to false"
+    shouldIncludeInvisible: Boolean = false,
+
     after: ConnectionCursor,
     before: ConnectionCursor,
     first: ConnectionLimitInt,