From 0eebfe923aa786e4063ff58a1dd2dec498d9ff59 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Thu, 1 Dec 2022 10:26:32 +0800 Subject: [PATCH] Add position in filter table and move filter api --- packages/api/src/entity/filter.ts | 3 + packages/api/src/generated/graphql.ts | 58 +++++++ packages/api/src/generated/schema.graphql | 23 +++ packages/api/src/resolvers/filters/index.ts | 144 +++++++++++++++++- .../api/src/resolvers/function_resolvers.ts | 3 + packages/api/src/schema.ts | 23 +++ packages/db/migrations/0100.do.filters.sql | 30 +++- packages/db/migrations/0100.undo.filters.sql | 2 + 8 files changed, 282 insertions(+), 4 deletions(-) diff --git a/packages/api/src/entity/filter.ts b/packages/api/src/entity/filter.ts index 4452d172e..54583167a 100644 --- a/packages/api/src/entity/filter.ts +++ b/packages/api/src/entity/filter.ts @@ -29,6 +29,9 @@ export class Filter { @Column('varchar', { length: 255 }) filter!: string + @Column('integer', { default: 0 }) + position!: number + @CreateDateColumn({ default: () => 'CURRENT_TIMESTAMP' }) createdAt!: Date diff --git a/packages/api/src/generated/graphql.ts b/packages/api/src/generated/graphql.ts index c21ab2ca9..8498e29bd 100644 --- a/packages/api/src/generated/graphql.ts +++ b/packages/api/src/generated/graphql.ts @@ -704,6 +704,7 @@ export type Filter = { filter: Scalars['String']; id: Scalars['ID']; name: Scalars['String']; + position: Scalars['Int']; updatedAt: Scalars['Date']; }; @@ -1014,6 +1015,29 @@ export type MergeHighlightSuccess = { overlapHighlightIdList: Array; }; +export type MoveFilterError = { + __typename?: 'MoveFilterError'; + errorCodes: Array; +}; + +export enum MoveFilterErrorCode { + BadRequest = 'BAD_REQUEST', + NotFound = 'NOT_FOUND', + Unauthorized = 'UNAUTHORIZED' +} + +export type MoveFilterInput = { + afterFilterId?: InputMaybe; + filterId: Scalars['ID']; +}; + +export type MoveFilterResult = MoveFilterError | MoveFilterSuccess; + +export type MoveFilterSuccess = { + __typename?: 'MoveFilterSuccess'; + filter: Filter; +}; + export type MoveLabelError = { __typename?: 'MoveLabelError'; errorCodes: Array; @@ -1064,6 +1088,7 @@ export type Mutation = { googleSignup: GoogleSignupResult; logOut: LogOutResult; mergeHighlight: MergeHighlightResult; + moveFilter: MoveFilterResult; moveLabel: MoveLabelResult; optInFeature: OptInFeatureResult; reportItem: ReportItemResult; @@ -1215,6 +1240,11 @@ export type MutationMergeHighlightArgs = { }; +export type MutationMoveFilterArgs = { + input: MoveFilterInput; +}; + + export type MutationMoveLabelArgs = { input: MoveLabelInput; }; @@ -3023,6 +3053,11 @@ export type ResolversTypes = { MergeHighlightInput: MergeHighlightInput; MergeHighlightResult: ResolversTypes['MergeHighlightError'] | ResolversTypes['MergeHighlightSuccess']; MergeHighlightSuccess: ResolverTypeWrapper; + MoveFilterError: ResolverTypeWrapper; + MoveFilterErrorCode: MoveFilterErrorCode; + MoveFilterInput: MoveFilterInput; + MoveFilterResult: ResolversTypes['MoveFilterError'] | ResolversTypes['MoveFilterSuccess']; + MoveFilterSuccess: ResolverTypeWrapper; MoveLabelError: ResolverTypeWrapper; MoveLabelErrorCode: MoveLabelErrorCode; MoveLabelInput: MoveLabelInput; @@ -3410,6 +3445,10 @@ export type ResolversParentTypes = { MergeHighlightInput: MergeHighlightInput; MergeHighlightResult: ResolversParentTypes['MergeHighlightError'] | ResolversParentTypes['MergeHighlightSuccess']; MergeHighlightSuccess: MergeHighlightSuccess; + MoveFilterError: MoveFilterError; + MoveFilterInput: MoveFilterInput; + MoveFilterResult: ResolversParentTypes['MoveFilterError'] | ResolversParentTypes['MoveFilterSuccess']; + MoveFilterSuccess: MoveFilterSuccess; MoveLabelError: MoveLabelError; MoveLabelInput: MoveLabelInput; MoveLabelResult: ResolversParentTypes['MoveLabelError'] | ResolversParentTypes['MoveLabelSuccess']; @@ -4105,6 +4144,7 @@ export type FilterResolvers; id?: Resolver; name?: Resolver; + position?: Resolver; updatedAt?: Resolver; __isTypeOf?: IsTypeOfResolverFn; }; @@ -4344,6 +4384,20 @@ export type MergeHighlightSuccessResolvers; }; +export type MoveFilterErrorResolvers = { + errorCodes?: Resolver, ParentType, ContextType>; + __isTypeOf?: IsTypeOfResolverFn; +}; + +export type MoveFilterResultResolvers = { + __resolveType: TypeResolveFn<'MoveFilterError' | 'MoveFilterSuccess', ParentType, ContextType>; +}; + +export type MoveFilterSuccessResolvers = { + filter?: Resolver; + __isTypeOf?: IsTypeOfResolverFn; +}; + export type MoveLabelErrorResolvers = { errorCodes?: Resolver, ParentType, ContextType>; __isTypeOf?: IsTypeOfResolverFn; @@ -4384,6 +4438,7 @@ export type MutationResolvers>; logOut?: Resolver; mergeHighlight?: Resolver>; + moveFilter?: Resolver>; moveLabel?: Resolver>; optInFeature?: Resolver>; reportItem?: Resolver>; @@ -5388,6 +5443,9 @@ export type Resolvers = { MergeHighlightError?: MergeHighlightErrorResolvers; MergeHighlightResult?: MergeHighlightResultResolvers; MergeHighlightSuccess?: MergeHighlightSuccessResolvers; + MoveFilterError?: MoveFilterErrorResolvers; + MoveFilterResult?: MoveFilterResultResolvers; + MoveFilterSuccess?: MoveFilterSuccessResolvers; MoveLabelError?: MoveLabelErrorResolvers; MoveLabelResult?: MoveLabelResultResolvers; MoveLabelSuccess?: MoveLabelSuccessResolvers; diff --git a/packages/api/src/generated/schema.graphql b/packages/api/src/generated/schema.graphql index 0b36bf4ad..7f9087672 100644 --- a/packages/api/src/generated/schema.graphql +++ b/packages/api/src/generated/schema.graphql @@ -620,6 +620,7 @@ type Filter { filter: String! id: ID! name: String! + position: Int! updatedAt: Date! } @@ -901,6 +902,27 @@ type MergeHighlightSuccess { overlapHighlightIdList: [String!]! } +type MoveFilterError { + errorCodes: [MoveFilterErrorCode!]! +} + +enum MoveFilterErrorCode { + BAD_REQUEST + NOT_FOUND + UNAUTHORIZED +} + +input MoveFilterInput { + afterFilterId: ID + filterId: ID! +} + +union MoveFilterResult = MoveFilterError | MoveFilterSuccess + +type MoveFilterSuccess { + filter: Filter! +} + type MoveLabelError { errorCodes: [MoveLabelErrorCode!]! } @@ -948,6 +970,7 @@ type Mutation { googleSignup(input: GoogleSignupInput!): GoogleSignupResult! logOut: LogOutResult! mergeHighlight(input: MergeHighlightInput!): MergeHighlightResult! + moveFilter(input: MoveFilterInput!): MoveFilterResult! moveLabel(input: MoveLabelInput!): MoveLabelResult! optInFeature(input: OptInFeatureInput!): OptInFeatureResult! reportItem(input: ReportItemInput!): ReportItemResult! diff --git a/packages/api/src/resolvers/filters/index.ts b/packages/api/src/resolvers/filters/index.ts index 87cc44590..19619b5f1 100644 --- a/packages/api/src/resolvers/filters/index.ts +++ b/packages/api/src/resolvers/filters/index.ts @@ -6,15 +6,23 @@ import { FiltersError, FiltersErrorCode, FiltersSuccess, + MoveFilterError, + MoveFilterErrorCode, + MoveFilterSuccess, MutationDeleteFilterArgs, + MutationMoveFilterArgs, MutationSaveFilterArgs, SaveFilterError, SaveFilterErrorCode, SaveFilterSuccess, } from '../../generated/graphql' import { Filter } from '../../entity/filter' -import { getRepository } from '../../entity/utils' +import { getRepository, setClaims } from '../../entity/utils' import { User } from '../../entity/user' +import { AppDataSource } from '../../server' +import { Between } from 'typeorm' +import { analytics } from '../../utils/analytics' +import { env } from '../../env' export const saveFilterResolver = authorized< SaveFilterSuccess, @@ -134,8 +142,9 @@ export const filtersResolver = authorized( } } - const filters = await getRepository(Filter).findBy({ - user: { id: claims.uid }, + const filters = await getRepository(Filter).find({ + where: { user: { id: claims.uid } }, + order: { position: 'ASC' }, }) return { @@ -157,3 +166,132 @@ export const filtersResolver = authorized( } } ) + +export const moveFilterResolver = authorized< + MoveFilterSuccess, + MoveFilterError, + MutationMoveFilterArgs +>(async (_, { input }, { claims: { uid }, log }) => { + log.info('Moving filters', { + input, + filters: { + source: 'resolver', + resolver: 'moveFilterResolver', + uid, + }, + }) + + const { filterId, afterFilterId } = input + + try { + const user = await getRepository(User).findOneBy({ id: uid }) + if (!user) { + return { + errorCodes: [MoveFilterErrorCode.Unauthorized], + } + } + + const filter = await getRepository(Filter).findOne({ + where: { id: filterId }, + relations: ['user'], + }) + if (!filter) { + return { + errorCodes: [MoveFilterErrorCode.NotFound], + } + } + if (filter.user.id !== uid) { + return { + errorCodes: [MoveFilterErrorCode.Unauthorized], + } + } + + if (filter.id === afterFilterId) { + // nothing to do + return { filter } + } + + const oldPosition = filter.position + // if afterFilterId is not provided, move to the top + let newPosition = 1 + if (afterFilterId) { + const afterFilter = await getRepository(Filter).findOne({ + where: { id: afterFilterId }, + relations: ['user'], + }) + if (!afterFilter) { + return { + errorCodes: [MoveFilterErrorCode.NotFound], + } + } + if (afterFilter.user.id !== uid) { + return { + errorCodes: [MoveFilterErrorCode.Unauthorized], + } + } + newPosition = afterFilter.position + } + const moveUp = newPosition < oldPosition + + // move filter to the new position + const updated = await AppDataSource.transaction(async (t) => { + await setClaims(t, uid) + + // update the position of the other filters + const updated = await t.getRepository(Filter).update( + { + user: { id: uid }, + position: Between( + Math.min(newPosition, oldPosition), + Math.max(newPosition, oldPosition) + ), + }, + { + position: () => `position + ${moveUp ? 1 : -1}`, + } + ) + if (!updated.affected) { + return null + } + + // update the position of the filter + return t.getRepository(Filter).save({ + ...filter, + position: newPosition, + }) + }) + + if (!updated) { + return { + errorCodes: [MoveFilterErrorCode.BadRequest], + } + } + + analytics.track({ + userId: uid, + event: 'filter_moved', + properties: { + filterId, + afterFilterId, + env: env.server.apiEnv, + }, + }) + + return { + filter: updated, + } + } catch (error) { + log.error('Error moving filters', { + error, + labels: { + source: 'resolver', + resolver: 'moveFilterResolver', + uid, + }, + }) + + return { + errorCodes: [MoveFilterErrorCode.BadRequest], + } + } +}) diff --git a/packages/api/src/resolvers/function_resolvers.ts b/packages/api/src/resolvers/function_resolvers.ts index a0143ea84..2648160a7 100644 --- a/packages/api/src/resolvers/function_resolvers.ts +++ b/packages/api/src/resolvers/function_resolvers.ts @@ -58,6 +58,7 @@ import { labelsResolver, logOutResolver, mergeHighlightResolver, + moveFilterResolver, moveLabelResolver, newsletterEmailsResolver, reminderResolver, @@ -184,6 +185,7 @@ export const functionResolvers = { deleteRule: deleteRuleResolver, saveFilter: saveFilterResolver, deleteFilter: deleteFilterResolver, + moveFilter: moveFilterResolver, }, Query: { me: getMeUserResolver, @@ -631,4 +633,5 @@ export const functionResolvers = { ...resultResolveTypeResolver('SaveFilter'), ...resultResolveTypeResolver('Filters'), ...resultResolveTypeResolver('DeleteFilter'), + ...resultResolveTypeResolver('MoveFilter'), } diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index 663268439..294949f51 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -2068,6 +2068,7 @@ const schema = gql` id: ID! name: String! filter: String! + position: Int! description: String createdAt: Date! updatedAt: Date! @@ -2114,6 +2115,27 @@ const schema = gql` NOT_FOUND } + input MoveFilterInput { + filterId: ID! + afterFilterId: ID # null to move to the top + } + + union MoveFilterResult = MoveFilterSuccess | MoveFilterError + + type MoveFilterSuccess { + filter: Filter! + } + + type MoveFilterError { + errorCodes: [MoveFilterErrorCode!]! + } + + enum MoveFilterErrorCode { + UNAUTHORIZED + BAD_REQUEST + NOT_FOUND + } + # Mutations type Mutation { googleLogin(input: GoogleLoginInput!): LoginResult! @@ -2190,6 +2212,7 @@ const schema = gql` deleteRule(id: ID!): DeleteRuleResult! saveFilter(input: SaveFilterInput!): SaveFilterResult! deleteFilter(id: ID!): DeleteFilterResult! + moveFilter(input: MoveFilterInput!): MoveFilterResult! } # FIXME: remove sort from feedArticles after all cached tabs are closed diff --git a/packages/db/migrations/0100.do.filters.sql b/packages/db/migrations/0100.do.filters.sql index 629dbf610..82516e745 100755 --- a/packages/db/migrations/0100.do.filters.sql +++ b/packages/db/migrations/0100.do.filters.sql @@ -10,14 +10,42 @@ CREATE TABLE omnivore.filters ( name character varying(255) NOT NULL, description character varying(255), filter character varying(255) NOT NULL, + position integer NOT NULL DEFAULT 0, created_at timestamptz NOT NULL DEFAULT current_timestamp, updated_at timestamptz NOT NULL DEFAULT current_timestamp, UNIQUE (user_id, name) ); -CREATE TRIGGER filters_modtime BEFORE UPDATE ON omnivore.filters +CREATE OR REPLACE FUNCTION update_filter_position() + RETURNS TRIGGER AS $$ +DECLARE + new_position INTEGER; +BEGIN + IF (TG_OP = 'DELETE') THEN + UPDATE omnivore.filters SET position = position - 1 WHERE user_id = OLD.user_id AND position > OLD.position; + RETURN OLD; + ELSIF (TG_OP = 'INSERT') THEN + SELECT COALESCE(MAX(position), 0) + 1 INTO new_position FROM omnivore.filters WHERE user_id = NEW.user_id AND name < NEW.name; + UPDATE omnivore.filters SET position = position + 1 WHERE user_id = NEW.user_id AND position >= new_position; + NEW.position = new_position; + RETURN NEW; + END IF; +END; +$$ LANGUAGE 'plpgsql'; + +CREATE TRIGGER update_filter_modtime BEFORE UPDATE ON omnivore.filters FOR EACH ROW EXECUTE PROCEDURE update_updated_at_column(); +CREATE TRIGGER increment_filter_position + BEFORE INSERT ON omnivore.filters + FOR EACH ROW +EXECUTE FUNCTION update_filter_position(); + +CREATE TRIGGER decrement_filter_position + AFTER DELETE ON omnivore.filters + FOR EACH ROW +EXECUTE FUNCTION update_filter_position(); + GRANT SELECT, INSERT, UPDATE, DELETE ON omnivore.filters TO omnivore_user; COMMIT; diff --git a/packages/db/migrations/0100.undo.filters.sql b/packages/db/migrations/0100.undo.filters.sql index 1837c1580..78b173e61 100755 --- a/packages/db/migrations/0100.undo.filters.sql +++ b/packages/db/migrations/0100.undo.filters.sql @@ -4,6 +4,8 @@ BEGIN; +DROP FUNCTION IF EXISTS update_filter_position; + DROP TABLE IF EXISTS omnivore.filters; COMMIT;