From 6116746e808b22e582b7e4ba510d7494a5baf408 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 9 Jun 2023 12:48:46 +0800 Subject: [PATCH 1/6] feat: make newsletters and favorites internal labels --- packages/api/src/elastic/labels.ts | 2 + packages/api/src/entity/label.ts | 3 ++ packages/api/src/generated/graphql.ts | 3 ++ packages/api/src/generated/schema.graphql | 2 + packages/api/src/resolvers/labels/index.ts | 45 ++++++++-------- packages/api/src/schema.ts | 2 + packages/api/src/services/groups.ts | 23 +++++---- packages/api/src/services/labels.ts | 51 ++++++++++--------- .../0113.do.add_internal_field_in_labels.sql | 11 ++++ ...0113.undo.add_internal_field_in_labels.sql | 9 ++++ 10 files changed, 95 insertions(+), 56 deletions(-) create mode 100755 packages/db/migrations/0113.do.add_internal_field_in_labels.sql create mode 100755 packages/db/migrations/0113.undo.add_internal_field_in_labels.sql diff --git a/packages/api/src/elastic/labels.ts b/packages/api/src/elastic/labels.ts index 5fcb0ba49..a0bcf1b30 100644 --- a/packages/api/src/elastic/labels.ts +++ b/packages/api/src/elastic/labels.ts @@ -254,6 +254,8 @@ export const updateLabel = async ( }, refresh: ctx.refresh, conflicts: 'proceed', // ignore conflicts + requests_per_second: 500, // throttle the requests + slices: 'auto', // parallelize the requests }) body.updated > 0 && diff --git a/packages/api/src/entity/label.ts b/packages/api/src/entity/label.ts index 0088e3319..dd0c65079 100644 --- a/packages/api/src/entity/label.ts +++ b/packages/api/src/entity/label.ts @@ -31,4 +31,7 @@ export class Label { @Column('integer', { default: 0 }) position!: number + + @Column('boolean', { default: false }) + internal!: boolean } diff --git a/packages/api/src/generated/graphql.ts b/packages/api/src/generated/graphql.ts index 343d03c58..41fbc517d 100644 --- a/packages/api/src/generated/graphql.ts +++ b/packages/api/src/generated/graphql.ts @@ -586,6 +586,7 @@ export type DeleteLabelError = { export enum DeleteLabelErrorCode { BadRequest = 'BAD_REQUEST', + Forbidden = 'FORBIDDEN', NotFound = 'NOT_FOUND', Unauthorized = 'UNAUTHORIZED' } @@ -1021,6 +1022,7 @@ export type Label = { createdAt?: Maybe; description?: Maybe; id: Scalars['ID']; + internal: Scalars['Boolean']; name: Scalars['String']; position?: Maybe; }; @@ -4874,6 +4876,7 @@ export type LabelResolvers, ParentType, ContextType>; description?: Resolver, ParentType, ContextType>; id?: Resolver; + internal?: Resolver; name?: Resolver; position?: Resolver, ParentType, ContextType>; __isTypeOf?: IsTypeOfResolverFn; diff --git a/packages/api/src/generated/schema.graphql b/packages/api/src/generated/schema.graphql index f7ff74334..d31ea1083 100644 --- a/packages/api/src/generated/schema.graphql +++ b/packages/api/src/generated/schema.graphql @@ -518,6 +518,7 @@ type DeleteLabelError { enum DeleteLabelErrorCode { BAD_REQUEST + FORBIDDEN NOT_FOUND UNAUTHORIZED } @@ -908,6 +909,7 @@ type Label { createdAt: Date description: String id: ID! + internal: Boolean! name: String! position: Int } diff --git a/packages/api/src/resolvers/labels/index.ts b/packages/api/src/resolvers/labels/index.ts index b39f358c8..514cb6dce 100644 --- a/packages/api/src/resolvers/labels/index.ts +++ b/packages/api/src/resolvers/labels/index.ts @@ -1,4 +1,4 @@ -import { Between, ILike } from 'typeorm' +import { Between } from 'typeorm' import { createPubSubClient } from '../../datalayer/pubsub' import { getHighlightById } from '../../elastic/highlights' import { @@ -39,9 +39,13 @@ import { UpdateLabelSuccess, } from '../../generated/graphql' import { AppDataSource } from '../../server' -import { getLabelsByIds } from '../../services/labels' +import { + createLabel, + getLabelByName, + getLabelsByIds, +} from '../../services/labels' import { analytics } from '../../utils/analytics' -import { authorized, generateRandomColor } from '../../utils/helpers' +import { authorized } from '../../utils/helpers' export const labelsResolver = authorized( async (_obj, _params, { claims: { uid }, log }) => { @@ -90,8 +94,6 @@ export const createLabelResolver = authorized< >(async (_, { input }, { claims: { uid }, log }) => { log.info('createLabelResolver') - const { name, color, description } = input - try { const user = await getRepository(User).findOneBy({ id: uid }) if (!user) { @@ -101,30 +103,20 @@ export const createLabelResolver = authorized< } // Check if label already exists ignoring case of name - const existingLabel = await getRepository(Label).findOneBy({ - user: { id: user.id }, - name: ILike(name), - }) + const existingLabel = await getLabelByName(uid, input.name) if (existingLabel) { return { errorCodes: [CreateLabelErrorCode.LabelAlreadyExists], } } - const label = await getRepository(Label).save({ - user, - name, - color: color || generateRandomColor(), - description: description || '', - }) + const label = await createLabel(uid, input) analytics.track({ userId: uid, event: 'label_created', properties: { - name, - color, - description, + ...input, env: env.server.apiEnv, }, }) @@ -156,7 +148,7 @@ export const deleteLabelResolver = authorized< } const label = await getRepository(Label).findOne({ - where: { id: labelId }, + where: { id: labelId, user: { id: uid } }, relations: ['user'], }) if (!label) { @@ -165,9 +157,11 @@ export const deleteLabelResolver = authorized< } } - if (label.user.id !== uid) { + // internal labels cannot be deleted + if (label.internal) { + log.info('internal labels cannot be deleted') return { - errorCodes: [DeleteLabelErrorCode.Unauthorized], + errorCodes: [DeleteLabelErrorCode.Forbidden], } } @@ -310,6 +304,15 @@ export const updateLabelResolver = authorized< } } + // internal labels cannot be updated + if (label.internal && label.name.toLowerCase() !== name.toLowerCase()) { + log.info('internal labels cannot be updated') + + return { + errorCodes: [UpdateLabelErrorCode.Forbidden], + } + } + log.info('Updating a label', { labels: { source: 'resolver', diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index f81e8aa27..d42941a0a 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -1418,6 +1418,7 @@ const schema = gql` description: String createdAt: Date position: Int + internal: Boolean! } type LabelsSuccess { @@ -1467,6 +1468,7 @@ const schema = gql` UNAUTHORIZED BAD_REQUEST NOT_FOUND + FORBIDDEN } type DeleteLabelError { diff --git a/packages/api/src/services/groups.ts b/packages/api/src/services/groups.ts index dd0fb8002..3f2979182 100644 --- a/packages/api/src/services/groups.ts +++ b/packages/api/src/services/groups.ts @@ -1,16 +1,16 @@ -import { User } from '../entity/user' -import { Group } from '../entity/groups/group' -import { Invite } from '../entity/groups/invite' -import { GroupMembership } from '../entity/groups/group_membership' import { nanoid } from 'nanoid' -import { AppDataSource } from '../server' -import { RecommendationGroup, User as GraphqlUser } from '../generated/graphql' +import { Group } from '../entity/groups/group' +import { GroupMembership } from '../entity/groups/group_membership' +import { Invite } from '../entity/groups/invite' +import { RuleActionType } from '../entity/rule' +import { User } from '../entity/user' import { getRepository } from '../entity/utils' import { homePageURL } from '../env' +import { RecommendationGroup, User as GraphqlUser } from '../generated/graphql' +import { AppDataSource } from '../server' import { userDataToUser } from '../utils/helpers' -import { createLabel } from './labels' +import { createLabel, getLabelByName } from './labels' import { createRule } from './rules' -import { RuleActionType } from '../entity/rule' export const createGroup = async (input: { admin: User @@ -228,8 +228,11 @@ export const createLabelAndRuleForGroup = async ( userId: string, groupName: string ) => { - // create a new label for the group - const label = await createLabel(userId, { name: groupName }) + let label = await getLabelByName(userId, groupName) + if (!label) { + // create a new label for the group + label = await createLabel(userId, { name: groupName }) + } // create a rule to add the label to all pages in the group const addLabelPromise = createRule(userId, { diff --git a/packages/api/src/services/labels.ts b/packages/api/src/services/labels.ts index b2251b57a..ebb02c92c 100644 --- a/packages/api/src/services/labels.ts +++ b/packages/api/src/services/labels.ts @@ -9,6 +9,12 @@ import { getRepository } from '../entity/utils' import { CreateLabelInput } from '../generated/graphql' import { generateRandomColor } from '../utils/helpers' +const INTERNAL_LABELS_IN_LOWERCASE = ['newsletters', 'favorites'] + +const isLabelInternal = (name: string): boolean => { + return INTERNAL_LABELS_IN_LOWERCASE.includes(name.toLowerCase()) +} + const batchGetLabelsFromLinkIds = async ( linkIds: readonly string[] ): Promise => { @@ -40,19 +46,12 @@ export const addLabelToPage = async ( return false } - let labelEntity = await getRepository(Label) - .createQueryBuilder() - .where({ user: { id: user.id } }) - .andWhere('LOWER(name) = LOWER(:name)', { name: label.name }) - .getOne() + let labelEntity = await getLabelByName(user.id, label.name) if (!labelEntity) { console.log('creating new label', label.name) - labelEntity = await getRepository(Label).save({ - ...label, - user, - }) + labelEntity = await createLabel(user.id, label) } console.log('adding label to page', label.name, pageId) @@ -80,30 +79,31 @@ export const getLabelsByIds = async ( }) } +export const getLabelByName = async ( + userId: string, + name: string +): Promise