Comment on #257 Make most redis keys volatile
*Pull Request Test Coverage Report for <https://coveralls.io/builds/40227976|Build 1360>* • *1* of *5* *(20.0%)* changed or added relevant lines in *2* files are covered. • No unchanged relevant lines lost coverage. • Overall coverage decreased (*-0.3%*) to *86.691%* * * * * * * *:yellow_heart: - <https://coveralls.io|Coveralls>*
Comment on #253 Dockerize LINE bot
Just curious. Why do you pack graphql schema in the bundle pipeline, but not in the development? In my opinion, the schema is part of API structure code, which should be part of the codebase.
Comment on #253 Dockerize LINE bot
> Just curious. Why do you pack graphql schema in the bundle pipeline, but not in the development? In my opinion, the schema is part of API structure code, which should be part of the codebase. Good question. I was also thinking the same thing. Including the remote GraphQL schema in the codebase helps developer understand the *expected* API. It also helps keep track of the API change in LINE bot code base, after API server is changed. I should include the remote GraphQL schema directly in the codebase in future PRs.
Comment on #253 Dockerize LINE bot
Default branch changed back to `dev`.
Previously, docker images built by Github actions are always in English. `console.log` in `babel.config.js` always detects `en_US` locale: <https://user-images.githubusercontent.com/108608/120918743-8ae66780-c6e8-11eb-80a3-b51a29b8cdf6.png|圖片> The root cause is that `actions/checkout@v2` action will remove `.env` file. This PR moves `.env` file download to after `actions/checkout@v2` and the build process can correctly pick up the env file: <https://user-images.githubusercontent.com/108608/120918792-cbde7c00-c6e8-11eb-992b-375cec4fe3ba.png|圖片>
#259 Fix server startup warning messages
When starting up the server under Node.js 16, it will emit the following warnings: > PM2 error: TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received type number (18) This is fixed by <https://github.com/Unitech/pm2/issues/4685|Unitech/pm2#4685>. > (node:29) Warning: Accessing non-existent property 'findOne' of module exports inside circular dependency This is fixed by <https://developer.mongodb.com/community/forums/t/warning-accessing-non-existent-property-count-of-module-exports-inside-circular-dependency/3183|https://developer.mongodb.com/community/forums/t/warning-accessing-non-existent-property-count-of-module-exports-inside-circular-dependency/3183> Also fixes npm audit warnings.
Comment on #259 Fix server startup warning messages
Deployed to staging, working fine. Will merge as there is no feature added.
g0v.hackmd.io
#260 "Missing process handler for job type __default__" when JOBQUEUE_CONCURRENCY is set
Currently when `JOBQUEUE_CONCURRENCY` env is explicitly set, group chat handler will not work. Every time a message is send to group, it emits errors as below: <https://user-images.githubusercontent.com/108608/120934669-d6713380-c731-11eb-9763-2249a453fe7c.png|image> The root cause is that when `JOBQUEUE_CONCURRENCY` env var is given, it will be a string , which will be interpreted as a <https://github.com/OptimalBits/bull/blob/develop/REFERENCE.md#queueprocess|processer name> in <https://github.com/cofacts/rumors-line-bot/blob/dev/src/webhook/handlers/groupHandler.js#L22|https://github.com/cofacts/rumors-line-bot/blob/dev/src/webhook/handlers/groupHandler.js#L22> . When the processor is registered as a named processor, calling `.add()` without "name" will <https://github.com/OptimalBits/bull/issues/1529#issuecomment-548402563|cause the "missing handler for *default*" error>. Current workaround: do not set `JOBQUEUE_CONCURRENCY` in env var.
#261 Change main branch to master
Currently, both staging LINE bot and production LINE bot are deployed using docker. • Staging: build & push to `cofacts/rumors-line-bot:dev-tw` after `master` branch merge • Production: build & push to `cofacts/rumors-line-bot:latest-tw` after `release/XXX` tag is pushed Therefore, we no longer need `dev` and `master` branch. We can just use `master` branch only. This PR: • replaces `dev` branch existence in code base • connects CI test to Coveralls
#261 Change main branch to master
Currently, both staging LINE bot and production LINE bot are deployed using docker. • Staging: build & push to `cofacts/rumors-line-bot:dev-tw` after `master` branch merge • Production: build & push to `cofacts/rumors-line-bot:latest-tw` after `release/XXX` tag is pushed Therefore, we no longer need `dev` and `master` branch. We can just use `master` branch only. This PR: • replaces `dev` branch existence in code base • connects CI test to Coveralls
#262 Setup Storybook for LIFF components
As a prep work for <https://www.figma.com/file/DvmAQjMJCncuPORWKnljM1/Cofacts-website-MrOrz?node-id=3005%3A0|Article LIFF>, we are setting up storybook to demonstrate components. Built storybook: <https://cofacts.github.io/rumors-line-bot/|https://cofacts.github.io/rumors-line-bot/> <https://user-images.githubusercontent.com/108608/121784414-76bdd100-cbe6-11eb-85b4-fc453c684471.png|圖片> <https://user-images.githubusercontent.com/108608/121784477-b4baf500-cbe6-11eb-9d4c-ff71bbb70dd9.png|圖片>
#263 Include Cofacts API schema in repo
As discussed in <https://github.com/cofacts/rumors-line-bot/pull/253#issuecomment-854124426|previous code review comment>, we should consider Cofacts API graphql schema as part of the LINE bot code base.
Comment on #257 Make most redis keys volatile
As we move to larger redis instance, we no longer need this fix. 1. In the future we plan to pass context along with actions (only store full text in redis), which should further reduce access of redis keys. 2. As `lastScannedAt` should never expire, we should put it in mongodb instead of redis. Thus the solution proposed in this pull request is suboptimal, should be rejected.
Comment on #263 Include Cofacts API schema in repo
*Pull Request Test Coverage Report for <https://coveralls.io/builds/40539150|Build 933010003>* • *0* of *0* changed or added relevant lines in *0* files are covered. • No unchanged relevant lines lost coverage. • Overall coverage remained the same at *87.013%* * * * * * * *:yellow_heart: - <https://coveralls.io|Coveralls>*
Comment on #262 Setup Storybook for LIFF components
*Pull Request Test Coverage Report for <https://coveralls.io/builds/40539169|Build 933018419>* • *0* of *0* changed or added relevant lines in *0* files are covered. • No unchanged relevant lines lost coverage. • Overall coverage remained the same at *87.013%* * * * * * * *:yellow_heart: - <https://coveralls.io|Coveralls>*
• new svelte-loader only works with Webpack 5: svelte-loader#178 • upgrades all loaders
Comment on #264 Upgrade to webpack 5
*Pull Request Test Coverage Report for <https://coveralls.io/builds/40539906|Build 933244265>* • *0* of *0* changed or added relevant lines in *0* files are covered. • No unchanged relevant lines lost coverage. • Overall coverage remained the same at *87.013%* * * * * * * *:yellow_heart: - <https://coveralls.io|Coveralls>*
Comment on #264 Upgrade to webpack 5
Upgrading storybook's webpack to v5 is also having the following issues: • Storybook 6.2's webpack v5 is still rc version, not stable yet • After making storybook support webpack v5, stories will dissapeear, which requires <https://github.com/storybookjs/addon-svelte-csf/issues/23|more configuration> to fix I decided that we should wait until storybook officially support webpack v5 before we jump into webpack v5. We are upgrading svelte-loader to `master` branch's version to mitigate the <https://github.com/sveltejs/svelte-loader/issues/178|original issue>. Closing this PR.
#265 Exclude replies with negative feedback when notifying users
Discussion: <https://g0v.hackmd.io/giYWfM8yRkKghtPOG1EBig#%E4%BA%82%E5%9B%9E%E8%88%87%E9%80%9A%E7%9F%A5|https://g0v.hackmd.io/giYWfM8yRkKghtPOG1EBig#%E4%BA%82%E5%9B%9E%E8%88%87%E9%80%9A%E7%9F%A5> We should not notify LINE user of replies with negative feedbacks. `getArticlesInBatch()` in `src/scripts/lib.js` should only yield articles when there are new replies having positive or zero feedbacks (positive - negative score >= 0).
#266 Cannot read property split of undefined/null expression
View details in Rollbar: <https://rollbar.com/mrorz/rumors-line-bot/items/431/|https://rollbar.com/mrorz/rumors-line-bot/items/431/> ``` TypeError: Cannot read property 'split' of undefined File "/srv/www/build/webhook/handlers/utils.js", line 500, in createHighlightContents for (let highlightPair of text.split('</HIGHLIGHT>')) { File "/srv/www/build/webhook/handlers/initState.js", line 166, in <unknown> contents: (0, _utils.createHighlightContents)(highlight, text), File "<anonymous>", line unknown, in Array.map File "/srv/www/build/webhook/handlers/initState.js", line 141, in initState const articleOptions = edgesSortedWithSimilarity.map(({ File "<anonymous>", line unknown, in runMicrotasks File "node:internal/process/task_queues", line 96, in processTicksAndRejections File "/srv/www/build/webhook/handleInput.js", line 117, in async handleInput params = await (0, _initState.default)(params); File "/srv/www/build/webhook/index.js", line 241, in async processText result = await (0, _handleInput.default)(context, { File "/srv/www/build/webhook/index.js", line 160, in async singleUserHandler result = await processText(context, type, input, otherFields, userId, req); ``` <https://user-images.githubusercontent.com/108608/122190796-ba555b00-cec4-11eb-9825-55f3131b53aa.png|image> Seems that `text` can be `undefined` and break code below. *Text* > 陳培哲說這1年多來參加了約50多次疫情會議,如果篩檢出新冠肺炎的高齡患者、糖尿病、腎臟病等慢性病者,立刻打新冠單株抗體,可以減少5到6成死亡率。「這我很早就講過了,他們充耳不聞,那些生命是可以挽救的,你就知道我的挫折有多深。這代表這個指揮中心的科學判斷與能力不足。」 > > 陳培哲指出,另一個疫情中心不夠科學專業的地方是「核酸檢測方式」。美國食藥局(FDA)有一套檢測系統,但台灣的病管局(CDC)卻自己弄另外一套,沒自動化、量能少又不凖,專家建議應採FDA的系充,但指揮中心根本不聽。「這次中國廣州疫情爆發,1000多萬人1個禮拜做完採檢,用我們手動方式要做到哪一年?」 > <https://www.storm.mg/new7/article/3740414?fbclid=IwAR3KtouSJVdWjFM0VVA3dj7J-0kWBxxObEw0jNjpy87LpQil0fmMYrm0OzE|https://www.storm.mg/new7/article/3740414?fbclid=IwAR3KtouSJVdWjFM0VVA3dj7J-0kWBxxObEw0jNjpy87LpQil0fmMYrm0OzE> *Output* <https://user-images.githubusercontent.com/108608/122191184-14562080-cec5-11eb-84a2-b897d91f51fa.png|image>
Comment on #266 Cannot read property split of undefined/null expression
Same as <https://github.com/cofacts/rumors-line-bot/issues/220|#220> ?
Comment on #266 Cannot read property split of undefined/null expression
You are right. let's just use <https://github.com/cofacts/rumors-line-bot/issues/220|#220>.
<https://github.com/cofacts/rumors-line-bot/pull/267|#267 [WIP] Common components in LIFF>
• Remove global material design styles, which is not currently in use • Add Cofacts palette as CSS custom properties • Implement `Button` and `TextArea` • Add SVG Icons
:white_check_mark: No checks have passed
<https://github.com/cofacts/rumors-line-bot/pull/268|#268 [WIP] Remove svelte-material-ui>
• Reduces bundle size from XXX to OOO • Apply common style to match <https://www.figma.com/file/DvmAQjMJCncuPORWKnljM1/Cofacts-website-MrOrz?node-id=3005%3A0|new article page>
:white_check_mark: No checks have passed
*Pull Request Test Coverage Report for <https://coveralls.io/builds/40725306|Build 954714778>* • *0* of *0* changed or added relevant lines in *0* files are covered. • No unchanged relevant lines lost coverage. • Overall coverage remained the same at *87.013%* * * * * * * *:yellow_heart: - <https://coveralls.io|Coveralls>*
*Pull Request Test Coverage Report for <https://coveralls.io/builds/40725634|Build 954836834>* • *0* of *0* changed or added relevant lines in *0* files are covered. • No unchanged relevant lines lost coverage. • Overall coverage remained the same at *87.013%* * * * * * * *:yellow_heart: - <https://coveralls.io|Coveralls>*