From f3ecbf5dc6501b628a616e4ba62ea0a87af5213e Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 20 Dec 2019 18:06:30 +0000 Subject: [PATCH] [CORL-824] Feature Flags (#2756) * feat: implemented feature flag * fix: support deprecated enums --- .../server/graph/tenant/mutators/Settings.ts | 17 +++- .../server/graph/tenant/resolvers/Mutation.ts | 8 ++ .../server/graph/tenant/resolvers/Settings.ts | 17 +++- .../server/graph/tenant/schema/schema.graphql | 84 ++++++++++++++++++- src/core/server/models/tenant/helpers.ts | 23 ++++- src/core/server/models/tenant/tenant.ts | 63 +++++++++++++- .../comments/pipeline/phases/toxic.ts | 17 +++- src/core/server/services/tenant/index.ts | 57 +++++++++++++ 8 files changed, 275 insertions(+), 11 deletions(-) diff --git a/src/core/server/graph/tenant/mutators/Settings.ts b/src/core/server/graph/tenant/mutators/Settings.ts index 9bbd57cfd..589ae19bb 100644 --- a/src/core/server/graph/tenant/mutators/Settings.ts +++ b/src/core/server/graph/tenant/mutators/Settings.ts @@ -1,7 +1,16 @@ import TenantContext from "coral-server/graph/tenant/context"; -import { GQLUpdateSettingsInput } from "coral-server/graph/tenant/schema/__generated__/types"; import { Tenant } from "coral-server/models/tenant"; -import { regenerateSSOKey, update } from "coral-server/services/tenant"; +import { + disableFeatureFlag, + enableFeatureFlag, + regenerateSSOKey, + update, +} from "coral-server/services/tenant"; + +import { + GQLFEATURE_FLAG, + GQLUpdateSettingsInput, +} from "coral-server/graph/tenant/schema/__generated__/types"; export const Settings = ({ mongo, @@ -15,4 +24,8 @@ export const Settings = ({ update(mongo, redis, tenantCache, config, tenant, input.settings), regenerateSSOKey: (): Promise => regenerateSSOKey(mongo, redis, tenantCache, tenant, now), + enableFeatureFlag: (flag: GQLFEATURE_FLAG) => + enableFeatureFlag(mongo, redis, tenantCache, tenant, flag), + disableFeatureFlag: (flag: GQLFEATURE_FLAG) => + disableFeatureFlag(mongo, redis, tenantCache, tenant, flag), }); diff --git a/src/core/server/graph/tenant/resolvers/Mutation.ts b/src/core/server/graph/tenant/resolvers/Mutation.ts index 37c71f461..59d2e7b11 100644 --- a/src/core/server/graph/tenant/resolvers/Mutation.ts +++ b/src/core/server/graph/tenant/resolvers/Mutation.ts @@ -229,4 +229,12 @@ export const Mutation: Required> = { user: await ctx.mutators.Users.deleteModeratorNote(input), clientMutationId: input.clientMutationId, }), + enableFeatureFlag: async (source, { input }, ctx) => ({ + flags: await ctx.mutators.Settings.enableFeatureFlag(input.flag), + clientMutationId: input.clientMutationId, + }), + disableFeatureFlag: async (source, { input }, ctx) => ({ + flags: await ctx.mutators.Settings.disableFeatureFlag(input.flag), + clientMutationId: input.clientMutationId, + }), }; diff --git a/src/core/server/graph/tenant/resolvers/Settings.ts b/src/core/server/graph/tenant/resolvers/Settings.ts index 61536fd41..f1cf38473 100644 --- a/src/core/server/graph/tenant/resolvers/Settings.ts +++ b/src/core/server/graph/tenant/resolvers/Settings.ts @@ -1,6 +1,21 @@ -import { GQLSettingsTypeResolver } from "coral-server/graph/tenant/schema/__generated__/types"; import { Tenant } from "coral-server/models/tenant"; +import { + GQLFEATURE_FLAG, + GQLSettingsTypeResolver, +} from "coral-server/graph/tenant/schema/__generated__/types"; + +const filterValidFeatureFlags = () => { + // Compute the valid flags based on this enum. + const flags = Object.values(GQLFEATURE_FLAG); + + // Return a type guard for the feature flag. + return (flag: string | GQLFEATURE_FLAG): flag is GQLFEATURE_FLAG => + flags.includes(flag); +}; + export const Settings: GQLSettingsTypeResolver = { slack: ({ slack = {} }) => slack, + featureFlags: ({ featureFlags = [] }) => + featureFlags.filter(filterValidFeatureFlags()), }; diff --git a/src/core/server/graph/tenant/schema/schema.graphql b/src/core/server/graph/tenant/schema/schema.graphql index a3e7d7061..ccefe4ada 100644 --- a/src/core/server/graph/tenant/schema/schema.graphql +++ b/src/core/server/graph/tenant/schema/schema.graphql @@ -333,6 +333,14 @@ type ActionPresence { ## Settings ################################################################################ +enum FEATURE_FLAG { + """ + DISABLE_WARN_USER_OF_TOXIC_COMMENT when enabled will turn off warnings for + toxic comments. + """ + DISABLE_WARN_USER_OF_TOXIC_COMMENT +} + # The moderation mode of the site. enum MODERATION_MODE { """ @@ -967,7 +975,6 @@ type SlackChannelTriggers { featuredComments is whether this channel will receive featured comments """ featuredComments: Boolean! - } type SlackChannel { @@ -1323,6 +1330,11 @@ type Settings { """ stories: StoryConfiguration! @auth(roles: [ADMIN]) + """ + featureFlags provides the enabled feature flags. + """ + featureFlags: [FEATURE_FLAG!]! @auth(roles: [ADMIN]) + """ createdAt is the time that the Settings was created at. """ @@ -3480,7 +3492,6 @@ input SlackTriggersConfigurationInput { featuredComments is whether this channel will receive featured comments """ featuredComments: Boolean - } input SlackChannelConfigurationInput { @@ -5247,6 +5258,62 @@ type RequestUserCommentsDownloadPayload { archiveURL: String! } +######################### +# enableFeatureFlag +######################### + +input EnableFeatureFlagInput { + """ + clientMutationId is required for Relay support. + """ + clientMutationId: String! + + """ + flag is the feature flag to create. + """ + flag: FEATURE_FLAG! +} + +type EnableFeatureFlagPayload { + """ + clientMutationId is required for Relay support. + """ + clientMutationId: String! + + """ + flags is the current set of flags enabled. + """ + flags: [FEATURE_FLAG!]! +} + +######################### +# disableFeatureFlag +######################### + +input DisableFeatureFlagInput { + """ + clientMutationId is required for Relay support. + """ + clientMutationId: String! + + """ + flag is the feature flag to delete. + """ + flag: FEATURE_FLAG! +} + +type DisableFeatureFlagPayload { + """ + clientMutationId is required for Relay support. + """ + clientMutationId: String! + + """ + flags is the current set of flags enabled. + """ + flags: [FEATURE_FLAG!]! +} + ################## ## Mutation ################## @@ -5605,6 +5672,19 @@ type Mutation { deleteModeratorNote( input: DeleteModeratorNoteInput! ): DeleteModeratorNotePayload! @auth(roles: [ADMIN, MODERATOR]) + + """ + enableFeatureFlag will enable a given FEATURE_FLAG. + """ + enableFeatureFlag(input: EnableFeatureFlagInput!): EnableFeatureFlagPayload! + @auth(roles: [ADMIN]) + + """ + disableFeatureFlag will disable a given FEATURE_FLAG + """ + disableFeatureFlag( + input: DisableFeatureFlagInput! + ): DisableFeatureFlagPayload! @auth(roles: [ADMIN]) } ################## diff --git a/src/core/server/models/tenant/helpers.ts b/src/core/server/models/tenant/helpers.ts index c449c9e49..a9e575bbe 100644 --- a/src/core/server/models/tenant/helpers.ts +++ b/src/core/server/models/tenant/helpers.ts @@ -1,13 +1,16 @@ import crypto from "crypto"; import { FluentBundle } from "fluent/compat"; +import { translate } from "coral-server/services/i18n"; + import { + GQLFEATURE_FLAG, GQLReactionConfiguration, GQLStaffConfiguration, } from "coral-server/graph/tenant/schema/__generated__/types"; -import { translate } from "coral-server/services/i18n"; import { SSOKey } from "../settings"; +import { Tenant } from "./tenant"; export const getDefaultReactionConfiguration = ( bundle: FluentBundle @@ -46,3 +49,21 @@ export function generateSSOKey(createdAt: Date): SSOKey { return { kid, secret, createdAt }; } + +/** + * hasFeatureFlag will check to see if the Tenant has a particular feature flag + * enabled. + * + * @param tenant the Tenant to test for a feature flag + * @param flag the FEATURE_FLAG to check for + */ +export function hasFeatureFlag( + tenant: Pick, + flag: GQLFEATURE_FLAG +) { + if (tenant.featureFlags && tenant.featureFlags.includes(flag)) { + return true; + } + + return false; +} diff --git a/src/core/server/models/tenant/tenant.ts b/src/core/server/models/tenant/tenant.ts index 289535c50..ef1e25e3a 100644 --- a/src/core/server/models/tenant/tenant.ts +++ b/src/core/server/models/tenant/tenant.ts @@ -6,14 +6,16 @@ import { DEFAULT_SESSION_LENGTH } from "coral-common/constants"; import { LanguageCode } from "coral-common/helpers/i18n/locales"; import { DeepPartial, Omit, Sub } from "coral-common/types"; import { dotize } from "coral-common/utils/dotize"; -import { - GQLMODERATION_MODE, - GQLSettings, -} from "coral-server/graph/tenant/schema/__generated__/types"; import { Settings } from "coral-server/models/settings"; import { I18n } from "coral-server/services/i18n"; import { tenants as collection } from "coral-server/services/mongodb/collections"; +import { + GQLFEATURE_FLAG, + GQLMODERATION_MODE, + GQLSettings, +} from "coral-server/graph/tenant/schema/__generated__/types"; + import { generateSSOKey, getDefaultReactionConfiguration, @@ -40,6 +42,11 @@ export interface TenantSettings * locale is the specified locale for this Tenant. */ locale: LanguageCode; + + /** + * featureFlags is the set of flags enabled on this Tenant. + */ + featureFlags?: GQLFEATURE_FLAG[]; } /** @@ -337,3 +344,51 @@ export async function rotateTenantSSOKey( return result.value || null; } + +export async function enableTenantFeatureFlag( + mongo: Db, + id: string, + flag: GQLFEATURE_FLAG +) { + // Update the Tenant. + const result = await collection(mongo).findOneAndUpdate( + { id }, + { + // Add the flag to the set of enabled flags. + $addToSet: { + featureFlags: flag, + }, + }, + { + // False to return the updated document instead of the original + // document. + returnOriginal: false, + } + ); + + return result.value || null; +} + +export async function disableTenantFeatureFlag( + mongo: Db, + id: string, + flag: GQLFEATURE_FLAG +) { + // Update the Tenant. + const result = await collection(mongo).findOneAndUpdate( + { id }, + { + // Pull the flag from the set of enabled flags. + $pull: { + featureFlags: flag, + }, + }, + { + // False to return the updated document instead of the original + // document. + returnOriginal: false, + } + ); + + return result.value || null; +} diff --git a/src/core/server/services/comments/pipeline/phases/toxic.ts b/src/core/server/services/comments/pipeline/phases/toxic.ts index bf49e5158..4293afcd8 100644 --- a/src/core/server/services/comments/pipeline/phases/toxic.ts +++ b/src/core/server/services/comments/pipeline/phases/toxic.ts @@ -14,6 +14,7 @@ import { Omit } from "coral-common/types"; import { ToxicCommentError } from "coral-server/errors"; import logger from "coral-server/logger"; import { ACTION_TYPE } from "coral-server/models/action/comment"; +import { hasFeatureFlag } from "coral-server/models/tenant"; import { IntermediateModerationPhase, IntermediatePhaseResult, @@ -23,6 +24,7 @@ import { import { GQLCOMMENT_FLAG_REASON, GQLCOMMENT_STATUS, + GQLFEATURE_FLAG, GQLPerspectiveExternalIntegration, } from "coral-server/graph/tenant/schema/__generated__/types"; @@ -118,7 +120,20 @@ export const toxic: IntermediateModerationPhase = async ({ // Throw an error if we're nudging instead of recording. if (nudge) { - throw new ToxicCommentError(model, score, threshold); + // Only if the feature flag for disabling this behavior is not enabled. + if ( + !hasFeatureFlag( + tenant, + GQLFEATURE_FLAG.DISABLE_WARN_USER_OF_TOXIC_COMMENT + ) + ) { + throw new ToxicCommentError(model, score, threshold); + } else { + log.trace( + { flag: GQLFEATURE_FLAG.DISABLE_WARN_USER_OF_TOXIC_COMMENT }, + "not nudging because of feature experiment" + ); + } } return { diff --git a/src/core/server/services/tenant/index.ts b/src/core/server/services/tenant/index.ts index afd03533d..5fef05ae5 100644 --- a/src/core/server/services/tenant/index.ts +++ b/src/core/server/services/tenant/index.ts @@ -12,6 +12,8 @@ import { createTenant, CreateTenantInput, createTenantSSOKey, + disableTenantFeatureFlag, + enableTenantFeatureFlag, rotateTenantSSOKey, Tenant, updateTenant, @@ -19,6 +21,7 @@ import { import { I18n } from "coral-server/services/i18n"; import { + GQLFEATURE_FLAG, GQLSettingsInput, GQLSettingsWordListInput, } from "coral-server/graph/tenant/schema/__generated__/types"; @@ -196,3 +199,57 @@ export async function discoverOIDCConfiguration(issuerString: string) { // Discover the configuration. return discover(issuer); } + +export async function enableFeatureFlag( + mongo: Db, + redis: Redis, + cache: TenantCache, + tenant: Tenant, + flag: GQLFEATURE_FLAG +) { + // If the Tenant already has this flag, don't bother adding it again. + if (tenant.featureFlags && tenant.featureFlags.includes(flag)) { + return tenant.featureFlags; + } + + // Enable the feature flag. + const updated = await enableTenantFeatureFlag(mongo, tenant.id, flag); + if (!updated || !updated.featureFlags) { + // As we just added the feature flag, we would expect that the Tenant would + // always have the feature flags set to some array. + throw new Error("tenant not found"); + } + + // Update the tenant cache. + await cache.update(redis, updated); + + // Return the updated feature flags. + return updated.featureFlags; +} + +export async function disableFeatureFlag( + mongo: Db, + redis: Redis, + cache: TenantCache, + tenant: Tenant, + flag: GQLFEATURE_FLAG +) { + // If the feature flag doesn't exist on the Tenant (or the Tenant has no + // feature flags), don't bother trying to remove it again. + if (!tenant.featureFlags || !tenant.featureFlags.includes(flag)) { + return tenant.featureFlags || []; + } + + // Remove the feature flag. + const updated = await disableTenantFeatureFlag(mongo, tenant.id, flag); + if (!updated) { + throw new Error("tenant not found"); + } + + // Update the tenant cache. + await cache.update(redis, updated); + + // Return the updated feature flags (or [] if there was no feature flags to + // begin with). + return updated.featureFlags || []; +}