From 4e36a5809ea75a0a68aa0f1eb2c93177c2082c00 Mon Sep 17 00:00:00 2001 From: Hongbo Wu Date: Fri, 20 Oct 2023 11:05:58 +0800 Subject: [PATCH] fix failure of re-subscribing rss feed --- packages/api/src/generated/graphql.ts | 3 +- packages/api/src/generated/schema.graphql | 3 +- .../api/src/resolvers/subscriptions/index.ts | 116 ++++++++++-------- packages/api/src/schema.ts | 3 +- packages/api/src/services/subscriptions.ts | 11 +- .../api/test/resolvers/subscriptions.test.ts | 43 +++++-- 6 files changed, 100 insertions(+), 79 deletions(-) diff --git a/packages/api/src/generated/graphql.ts b/packages/api/src/generated/graphql.ts index 6d04536da..3bbc9a88d 100644 --- a/packages/api/src/generated/graphql.ts +++ b/packages/api/src/generated/graphql.ts @@ -2616,9 +2616,8 @@ export enum SubscribeErrorCode { } export type SubscribeInput = { - name?: InputMaybe; subscriptionType?: InputMaybe; - url?: InputMaybe; + url: Scalars['String']; }; export type SubscribeResult = SubscribeError | SubscribeSuccess; diff --git a/packages/api/src/generated/schema.graphql b/packages/api/src/generated/schema.graphql index f50844aee..e5192b308 100644 --- a/packages/api/src/generated/schema.graphql +++ b/packages/api/src/generated/schema.graphql @@ -2058,9 +2058,8 @@ enum SubscribeErrorCode { } input SubscribeInput { - name: String subscriptionType: SubscriptionType - url: String + url: String! } union SubscribeResult = SubscribeError | SubscribeSuccess diff --git a/packages/api/src/resolvers/subscriptions/index.ts b/packages/api/src/resolvers/subscriptions/index.ts index dd0b8a7c7..8d5b74a83 100644 --- a/packages/api/src/resolvers/subscriptions/index.ts +++ b/packages/api/src/resolvers/subscriptions/index.ts @@ -175,25 +175,8 @@ export const subscribeResolver = authorized< SubscribeSuccessPartial, SubscribeError, MutationSubscribeArgs ->(async (_, { input }, { authTrx, uid, log }) => { - log.info('subscribeResolver') - +>(async (_, { input }, { uid, log }) => { try { - // find existing subscription - const subscription = await authTrx((t) => - t.getRepository(Subscription).findOneBy({ - url: input.url || undefined, - name: input.name || undefined, - user: { id: uid }, - type: input.subscriptionType || SubscriptionType.Rss, // default to rss - }) - ) - if (subscription) { - return { - errorCodes: [SubscribeErrorCode.AlreadySubscribed], - } - } - analytics.track({ userId: uid, event: 'subscribed', @@ -203,58 +186,83 @@ export const subscribeResolver = authorized< }, }) - // create new rss subscription - if (input.url) { - const MAX_RSS_SUBSCRIPTIONS = 150 - // validate rss feed - const feed = await parser.parseURL(input.url) - - // limit number of rss subscriptions to 50 - const newSubscriptions = (await authTrx((t) => - t.query( - `insert into omnivore.subscriptions (name, url, description, type, user_id, icon) - select $1, $2, $3, $4, $5, $6 from omnivore.subscriptions - where user_id = $5 and type = 'RSS' and status = 'ACTIVE' - having count(*) < $7 - returning *;`, - [ - feed.title, - input.url, - feed.description || null, - SubscriptionType.Rss, - uid, - feed.image?.url || null, - MAX_RSS_SUBSCRIPTIONS, - ] - ) - )) as Subscription[] - - if (newSubscriptions.length === 0) { + // find existing subscription + const existingSubscription = await getRepository(Subscription).findOneBy({ + url: input.url, + user: { id: uid }, + type: SubscriptionType.Rss, + }) + if (existingSubscription) { + if (existingSubscription.status === SubscriptionStatus.Active) { return { - errorCodes: [SubscribeErrorCode.ExceededMaxSubscriptions], + errorCodes: [SubscribeErrorCode.AlreadySubscribed], } } - const newSubscription = newSubscriptions[0] + // re-subscribe + const updatedSubscription = await getRepository(Subscription).save({ + ...existingSubscription, + status: SubscriptionStatus.Active, + }) - // create a cloud task to fetch rss feed item for the new subscription + // create a cloud task to fetch rss feed item for resub subscription await enqueueRssFeedFetch({ userIds: [uid], url: input.url, - subscriptionIds: [newSubscription.id], + subscriptionIds: [updatedSubscription.id], scheduledDates: [new Date()], // fetch immediately - fetchedDates: [null], - checksums: [null], + fetchedDates: [updatedSubscription.lastFetchedAt || null], + checksums: [updatedSubscription.lastFetchedChecksum || null], }) return { - subscriptions: [newSubscription], + subscriptions: [updatedSubscription], } } - log.info('missing url or name') + // create new rss subscription + const MAX_RSS_SUBSCRIPTIONS = 150 + // validate rss feed + const feed = await parser.parseURL(input.url) + + // limit number of rss subscriptions to 150 + const results = (await getRepository(Subscription).query( + `insert into omnivore.subscriptions (name, url, description, type, user_id, icon) + select $1, $2, $3, $4, $5, $6 from omnivore.subscriptions + where user_id = $5 and type = 'RSS' and status = 'ACTIVE' + having count(*) < $7 + returning *;`, + [ + feed.title, + input.url, + feed.description || null, + SubscriptionType.Rss, + uid, + feed.image?.url || null, + MAX_RSS_SUBSCRIPTIONS, + ] + )) as Subscription[] + + if (results.length === 0) { + return { + errorCodes: [SubscribeErrorCode.ExceededMaxSubscriptions], + } + } + + const newSubscription = results[0] + + // create a cloud task to fetch rss feed item for the new subscription + await enqueueRssFeedFetch({ + userIds: [uid], + url: input.url, + subscriptionIds: [newSubscription.id], + scheduledDates: [new Date()], // fetch immediately + fetchedDates: [null], + checksums: [null], + }) + return { - errorCodes: [SubscribeErrorCode.BadRequest], + subscriptions: [newSubscription], } } catch (error) { log.error('failed to subscribe', error) diff --git a/packages/api/src/schema.ts b/packages/api/src/schema.ts index 4d125b10a..de3f5209c 100755 --- a/packages/api/src/schema.ts +++ b/packages/api/src/schema.ts @@ -2538,8 +2538,7 @@ const schema = gql` } input SubscribeInput { - url: String - name: String + url: String! subscriptionType: SubscriptionType } diff --git a/packages/api/src/services/subscriptions.ts b/packages/api/src/services/subscriptions.ts index 9e6e78b37..fcf25af01 100644 --- a/packages/api/src/services/subscriptions.ts +++ b/packages/api/src/services/subscriptions.ts @@ -205,15 +205,8 @@ export const createSubscription = async ( }) } -export const deleteSubscription = async ( - id: string, - userId: string -): Promise => { - return authTrx( - (tx) => tx.getRepository(Subscription).delete(id), - undefined, - userId - ) +export const deleteSubscription = async (id: string): Promise => { + return getRepository(Subscription).delete(id) } export const createRssSubscriptions = async ( diff --git a/packages/api/test/resolvers/subscriptions.test.ts b/packages/api/test/resolvers/subscriptions.test.ts index 085845eb4..02f7cd1b8 100644 --- a/packages/api/test/resolvers/subscriptions.test.ts +++ b/packages/api/test/resolvers/subscriptions.test.ts @@ -397,21 +397,47 @@ describe('Subscriptions API', () => { }) after(async () => { - await deleteSubscription(existingSubscription.id, user.id) + await deleteSubscription(existingSubscription.id) }) it('returns an error', async () => { - const res = await graphqlRequest( - query, - authToken, - { input: { url, subscriptionType } }, - ).expect(200) + const res = await graphqlRequest(query, authToken, { + input: { url, subscriptionType }, + }).expect(200) expect(res.body.data.subscribe.errorCodes).to.eql([ 'ALREADY_SUBSCRIBED', ]) }) }) + context('when the user unsubscribed the feed', () => { + let existingSubscription: Subscription + + before(async () => { + existingSubscription = await createSubscription( + user.id, + 'RSS Feed', + undefined, + SubscriptionStatus.Unsubscribed, + url, + subscriptionType, + url + ) + }) + + after(async () => { + await deleteSubscription(existingSubscription.id) + }) + + it('re-subscribes the user', async () => { + const res = await graphqlRequest(query, authToken, { + input: { url, subscriptionType }, + }).expect(200) + expect(res.body.data.subscribe.subscriptions).to.have.lengthOf(1) + expect(res.body.data.subscribe.subscriptions[0].id).to.be.a('string') + }) + }) + it('creates a rss subscription', async () => { const res = await graphqlRequest( query, @@ -422,10 +448,7 @@ describe('Subscriptions API', () => { expect(res.body.data.subscribe.subscriptions[0].id).to.be.a('string') // clean up - await deleteSubscription( - res.body.data.subscribe.subscriptions[0].id, - user.id - ) + await deleteSubscription(res.body.data.subscribe.subscriptions[0].id) }) }) })