-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(publish-quota): per user publishArticle quota #3862
base: develop
Are you sure you want to change the base?
Conversation
@@ -50,6 +57,21 @@ const resolver: GQLMutationResolvers['publishArticle'] = async ( | |||
throw new UserInputError('content is required') | |||
} | |||
|
|||
const fieldName = 'publishArticle' | |||
const pass = await checkOperationLimit({ | |||
user: viewer.id || viewer.ip, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
viewer.ip will never be used as only logged-in user can publish articles by now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And as opercation limit is checked here, rateLimitDirective (@ratelimit) on publishArticle
in schema need to be removed
@@ -16,3 +18,6 @@ export const AUTHOR_TYPE = { | |||
default: 'default', | |||
trendy: 'trendy', | |||
} as const | |||
|
|||
export const PUBLISH_ARTICLE_RATE_LIMIT = isProd ? 1 : 1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this rate confirmed with the CC team?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was the default before:
"hotfix(publish-rate): reduce rate of publishArticle"
This reverts commit 2717c88.
}) | ||
|
||
if (!pass) { | ||
throw new ActionLimitExceededError( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add unit test for this mutation changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added in src/types/__test__/1/article.test.ts
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3862 +/- ##
===========================================
+ Coverage 58.57% 58.59% +0.01%
===========================================
Files 887 890 +3
Lines 17184 17216 +32
Branches 3089 3095 +6
===========================================
+ Hits 10065 10087 +22
- Misses 7068 7078 +10
Partials 51 51 ☔ View full report in Codecov by Sentry. |
fd27c9b
to
c444559
Compare
how about Revert "hotfix(publish-rate): reduce rate of publishArticle" This reverts commit 2717c88. Let the graphql directive still handles a |
This reverts commit 2717c88.
resolves #3842