-
Notifications
You must be signed in to change notification settings - Fork 19
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
Handle group messages #238
Conversation
- Add createGroupReplyMessages function and extracts common reply from createReplyMessages which is for single user - Add TimeoutError which will be throw in groupHandler if event replyToken expired
…`uuid` Should also create a user scope custom dimension from web : Setting -> Account -> Property -> Custom Dimension
Avoid processing expired event if there may be other active events
90ddb0a
to
5622b1a
Compare
…number of line group members
5622b1a
to
8db218f
Compare
045ded5
to
5bedfb0
Compare
…om in groupMessage
and cover test of `edgesSortedWithSimilarity.sort` and `invalidArticleReply`
5bedfb0
to
a04a06c
Compare
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.
I'm still reviewing, just send out 2 comments here
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.
Thanks for the implementation!
I have added some comments and some hints on GA. Happy Lunar New Year!
gh.addJob(param, { jobId: 'testExpiredId1' }); | ||
gh.addJob(param, { jobId: 'testExpiredId2' }); | ||
gh.addJob(param, { jobId: 'testExpiredId3' }); | ||
gh.addJob(param, { jobId: 'testExpiredId4' }); |
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.
I am confused by the 4 jobs and the changing processGroupEvent
mock. May I know what would happen to the 4 jobs here?
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.
有點難描述,我用中文好了 哈哈
這裡要測試 expireQueue 執行時,如果有新的 event 進來,expireQueue 要被暫停,並且執行完 jobQueue 後再繼續,利用監聽 expiredJobQueue 的 active event 加上 concurrency
設成 1
,我們可以確保在 expiredJobQueue 執行期間,插入一個沒過期的 job。
-
因為這裡把
GroupHandler
的concurrency
設成1
,所以四個 expiredJob 會在 jobQueue 的 process functionprocessJob
一個一個(不保證順序)處理並加進 expiredJobQueue,這裡並不會執行到processJob
裡面的processGroupEvent
。 -
當所有(四個) jobQueue 的 job 被執行完之後(
onDrained
),expiredJobQueue 會開始(resume)執行。 -
當第一個 expired job 開始執行時,會同時送出一個 expiredJobQueue 的 job active event。
-
在收到 expired job active event 的時候加入一個沒過期的 job (到 jobQueue),並且為了讓它成功,把
processGroupEvent
改成 resolve,在 jobQueue on('completed' 改回來。
其實這邊我期望是可以 mockImplementationOnce(expired x 4 + success x 1),但是processGroupEvent
被執行的順序可能是- testExpiredId1 -> (expiredJobQueue active event) -> successJobId -> testExpiredId2 -> ...
- testExpiredId1 -> (expiredJobQueue active event) testExpiredId2 (幾乎同時)-> successJobId -> ...
因為我們不能保證 active event 收到的時機,不知道 success mock 要放在第幾個順位才能正確讓 successJobId 是 resolve 的,所以就在收到第一個 expiredJobQueue (job) 的 active event 時把它改成 resolve,在它完成時再改回 reject,這樣就會造成一個問題是:把
processGroupEvent
改成 resolve 時剛好 testExpiredId2 開始了 expiredJobQueue 還沒 pause,所以 testExpiredId2 的結果就是 complete 的(理論上 expiredJobQueue 的 job 要在processGroupEvent
裡面全被 reject)
這裡只 expect jobQueue 的 success/fail count,因為 expiredJobQueue 的結果可能每次都不一樣
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.
感謝感謝
我在想如果「這裡要測試 expireQueue 執行時,如果有新的 event 進來,expireQueue 要被暫停,並且執行完 jobQueue 後再繼續」
若把下面這兩塊拆成兩個分開的 test case,會不會比較簡單
expireQueue
執行時,如果有新的 event 進來,expireQueue
要被暫停 --> 先往expireQueue
塞東西,再gh.addJob
,然後 expectexpireQueue
的 status- 執行完
jobQueue
後再繼續 --> 先準備一個暫停的 expireQueue,然後gh.addJob
,然後在該 job 完成之後 expect expireQueue 的 status 是否不是 pause
Co-authored-by: Johnson Liang <johnsonliang7@gmail.com>
…rge object (req) to redis
…pper case to indicate that it is a constant
…umber of normal article replies
Pull Request Test Coverage Report for Build 1270
💛 - Coveralls |
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.
Seems that most issues are resolved. Things left are
- documentation regarding the custom dimensions; and if we should use custom metric instead of custom dimension for user count in chat groups
- excessive comment
- simplify test case
I think we can discuss on 1 because setting custom metric / dimension cannot be undone in the future. Documentations, comments & tests can wait.
We will merge first and just setup 1st custom dimension. Leave 2nd dimension / metric for the future. |
Fixes #13
Description
Not ready
Change validCategories from categoryName to categoryIdShould filter out the category which negative vote > positive voteShould disable job queue rate limit and addconcurrency
to env variableFlow
webhook/index (get group message event) -> GroupHandler(jobQueue) -> processGroupEvent -> groupMessage
Response rule
Hi cofacts
orHi confacts
.validCategories
and be Identical enough.validCategories
:疾病、醫藥🆕
,COVID-19 疫情🆕
,科技、資安、隱私
,保健秘訣、食品安全
,免費訊息詐騙
positiveFeedbackCount
> candidate'snegativeFeedbackCount.
positiveFeedbackCount
> non-rumors'positiveFeedbackCount
.Job queue
jobQueue
will process events whichreplytoken
are not expired.jobQueue
is going to process is expired, it will be skipped and added toexpiredJobQueue
.expiredJobQueue
start only when there's no jobs injobQueue
(drained).expiredJobQueue
pause when there's jobs adding tojobQueue
.GA
Add two user scope custom dimension
Message Source
:user
|group
|room
will set every time sends the event.NumberOfGroupMembers
: It will only set when bot join group.Article event in group
Category / Action / Label
UserInput
/Intro
/UserInput
/ArticleSearch
/ArticleFound
Article
/Search
/<article id>
for each article foundTo calculate total group the bot joined, we set ga event value 1 as join, -1 as leave
Category / Action / Value
Snapshot shows where to create custom dimension in ga.