cofacts

Month: 2020-08

2020-08-01

2020-08-02

mrorz 13:42:26
coscup 聽到把 GA 的 image beacon 塞進 flex message,可以追蹤 push message 實際被點開閱讀的數字
https://hackmd.io/@taichunmin/slide-coscup-2020#/43

好像可以放在我們提醒使用者有新文章的 push message 來追蹤有多少人會因為收到推播而點開 chatroom,這樣一來下面這三個個動作才都有數字:
發推播 --> 開 chatroom --> 載入 LIFF

HackMD

在 LINE Chatbot 中串接 Google Analytics 的經驗分享 - HackMD

在 LINE Chatbot 中串接 Google Analytics 的經驗分享

github 14:41:09

#189 Date util

small refactor, moving date range helper to util to be reused in analytics dataloader

github 14:44:27

#190 Analytics data loader

depending on <https://github.com/cofacts/rumors-api/pull/188|#188> and <https://github.com/cofacts/rumors-api/pull/189|#189>

github 14:45:44

Comment on #189 Date util

<https://coveralls.io/builds/32483673|Coverage Status> Coverage decreased (-0.02%) to 87.574% when pulling *<https://github.com/cofacts/rumors-api/commit/839bcd08f441b5d18d1b6e0015c6545c6a5e3b0f|839bcd0> on dateUtil* into *<https://github.com/cofacts/rumors-api/commit/7dc45064a84a9ae4e129d45dc77d1ff44270ae23|7dc4506> on fetchGA*.

github 14:47:05

Comment on #190 Analytics data loader

<https://coveralls.io/builds/32483677|Coverage Status> Coverage increased (+0.2%) to 87.78% when pulling *<https://github.com/cofacts/rumors-api/commit/095d1eaa1fad7697f9425e609889d5bd740c84fa|095d1ea> on analyticsDataLoader* into *<https://github.com/cofacts/rumors-api/commit/839bcd08f441b5d18d1b6e0015c6545c6a5e3b0f|839bcd0> on dateUtil*.

github 15:10:02

Comment on #191 Article stats

<https://coveralls.io/builds/32483773|Coverage Status> Coverage increased (+0.02%) to 87.798% when pulling *<https://github.com/cofacts/rumors-api/commit/969bc5b810aadad8c8722af076eee796e8093513|969bc5b> on articleStats* into *<https://github.com/cofacts/rumors-api/commit/095d1eaa1fad7697f9425e609889d5bd740c84fa|095d1ea> on analyticsDataLoader*.

github 19:58:22

Review on #188 script to fetch google analytics stats

Thanks for the commit! I have finished commenting on the script implementation. Going to inspect the test cases tonight.

github 19:58:22

Comment on #188 script to fetch google analytics stats

Since this try-catch statement covers the entire function, it is equivalent to doing try-catch in the call site of `upsertDocStats`. However, the latter allows the user of `upsertDocStats` to determine if they want to catch it. I would suggest removing try-catch here.

github 19:58:22

Comment on #188 script to fetch google analytics stats

I find it difficult to differentiate the use case between L291-294 and L307-315. A comment elaborating this case (edge-case handling across batches) would be helpful.

github 19:58:22

Comment on #188 script to fetch google analytics stats

dotenv variables should be string already, thus `''` can be omitted. For example, `process.env.PORT` is `5000` even without `''`.

github 19:58:22

Comment on #188 script to fetch google analytics stats

Seems that errors are all unexpected items that should be reported to Rollbar. It would be nice if we send them using `Rollbar.error(err)`.

github 19:58:22

Comment on #188 script to fetch google analytics stats

Some explanation on how to execute the script here (or in `README.md` instead) would be great. Newly introduced env vars in `.env.sample` needs some documentation in here (or in `README.md` instead).

github 19:58:22

Comment on #188 script to fetch google analytics stats

I suppose `row.metrics[0].values` are numbers in strings? If so, `parseInt(visits, 10)` or even `+visits` can convert strings to number. Use of eval is <https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval#Never_use_eval!|highly discouraged>.

github 19:58:22

Comment on #188 script to fetch google analytics stats

Since it's <https://developers.google.com/analytics/devguides/reporting/core/v4/resource-based-quota|analytics 360 only function>, we will probably never uncomment it. Let's remove this line~

github 19:58:22

Comment on #188 script to fetch google analytics stats

Seems that errors are all unexpected items that should be reported to Rollbar. It would be nice if we send them using `Rollbar.error(err)`.

github 20:02:42

Comment on #188 script to fetch google analytics stats

I am wondering if <https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/Map.html#putAll(java.util.Map)|`putAll()`> is useful here. I also like the current implementation, which explicitly lists all changed fields, this makes modifications crystal clear :gem: .

2020-08-03

github 01:07:55

Review on #189 Date util

Thanks! I've added a minor suggestion regarding the use of `isValid`.

github 01:07:55

Comment on #189 Date util

Personally I prefer throwing errors with message text for these utility functions. Pros: • removes the need to do `else` block and simplifies code structure a lot. • have the same expressiveness of error message + `isValid` • when it's invalid, only `error` field is relevant. `isValid` is derived from `error`, and `startDate` / `endDate` are simply not applicable. • in this case, an exception with message can fully express the same amount of information. Cons: • `validateOOO` functions do imply returning error instead of throwing them. • may need to rename to something like `assertOOO`

github 01:25:58

Comment on #188 script to fetch google analytics stats

Another idea that come across my mind is, processing items across batches should be pretty similar to merging different `pagePathLevel2` -- they all memorize the last row and perform update when necessary. This means that in theory we can just implement merging of different `pagePathLevel2`. When batch changes, we should put `prevLastRow` into the "last-row" memory and perform identical logic that handles `pagePathLevel2`. Another way of thinking is that we blur the boundary of "batch" and only work on one row at a time, providing last "row" (no matter it's from same batch or previous batch) during the process.

stbb1025 12:10:15
https://www.figma.com/file/zpD45j8nqDB2XfA6m2QskO/Cofacts-website?node-id=912%3A384
@lucien @mrorz @bil
116155217_350793832748131_7278387044960444471_n.jpg
116207176_1231006320590790_4735981624252461450_n.jpg
116212778_216506436350194_8151330362891010884_n.jpg
116235900_378582476456256_530460603154245394_n.jpg
116800306_291163598653757_6610934379152638743_n.jpg
117068519_327122458671111_816188199161369806_n.jpg
github 14:40:57

Comment on #304 Fix 262 rss function

I was thinking if we can directly convert `listQueryVars` to JSON and put them in feed URL. That should simplify `api/articles/[feed]` (because it don't need `getQueryVars`, just `JSON.parse`) and allows greater flexibility for the feed. What do you think <https://github.com/nonumpa|@nonumpa> ?

github 14:40:57

Comment on #304 Fix 262 rss function

It's brilliant that we can remove default option from `getQueryVars()`. It makes the code easier to read :+1: I am thinking if we can further simplify this to: ``` const listQueryVars = getQueryVars({ // Defaults that can be overridden by query filters: defaultFilters.join(','), orderBy: defaultOrder, // URL query ...query, // Others to join query timeRangeKey, userId: user?.id, }); ```

github 14:40:57

Review on #304 Fix 262 rss function

Thanks for the fix! I have attached 2 comments at first glance. Will look at other parts tonight.

2020-08-04

github 03:01:42

Comment on #190 Analytics data loader

Elasticsearch <https://www.elastic.co/guide/en/elasticsearch/reference/6.8/query-dsl-range-query.html#ranges-on-dates|range query> actually supports <https://www.elastic.co/guide/en/elasticsearch/reference/6.8/common-options.html#date-math|date math> for date fields. Currently several inputs like `ListArticleFilter.createdAt`, `ListArticleFilter.repliedAt` leverages `TimeRangeInput` to achieve date range inputs that supports date math, which is essentially exposing the whole range query filter and letting Elasticsearch check the format directly. I am thinking if we can do the same for `analyticsLoader`. I am OK with open ends (For example, with `gte` given but no `lte`) query since Elasticsearch always bound its result to a certain <https://www.elastic.co/guide/en/elasticsearch/reference/6.8/search-request-from-size.html|size>.

github 03:01:42

Review on #190 Analytics data loader

Thanks for implementing the analytics data loader! I have raised some questions on the `analyticsLoaderFactory` and hope we can discuss before merge.

github 03:01:42

Comment on #190 Analytics data loader

Since the `ID` used for this data loader is not primitive, we may need to define `cacheKeyFn` so that dataloader can tell if any "id"s given are identical. This is useful for caching and batching. Example: <https://github.com/cofacts/rumors-api/blob/master/src/graphql/dataLoaders/docLoaderFactory.js#L21|https://github.com/cofacts/rumors-api/blob/master/src/graphql/dataLoaders/docLoaderFactory.js#L21> Documentation: <https://github.com/graphql/dataloader#new-dataloaderbatchloadfn--options|https://github.com/graphql/dataloader#new-dataloaderbatchloadfn--options>

github 03:01:43

Comment on #190 Analytics data loader

Elasticsearch body search by <https://www.elastic.co/guide/en/elasticsearch/reference/6.8/search-request-from-size.html|returns 10 results by default>. This may cause this dataloader never return more than 10 results. We may want to relax that using <https://www.elastic.co/guide/en/elasticsearch/reference/6.8/search-request-from-size.html|`size` param>. Personally I think we can just set a reasonable constant here (Maybe 30 / 60 / 90?). I am ok with crazy values as well -- `articleReplyFeedbackLoader` uses 10,000 lol

github 03:05:35

Review on #189 Date util

LGTM. Thanks for the change!

github 11:43:36

#305 RSS UI

figma <https://www.figma.com/file/YikWPPauMnukDH0U2Nq4YS/Components?node-id=1%3A310|https://www.figma.com/file/YikWPPauMnukDH0U2Nq4YS/Components?node-id=1%3A310> <https://user-images.githubusercontent.com/6376572/89249900-ffbec000-d645-11ea-8880-78fbb9516f8e.gif|desktop-zh> <https://user-images.githubusercontent.com/6376572/89250032-5b894900-d646-11ea-8923-a9c532abda4b.gif|mobile-en> *Subscribe through IFTTT* <https://user-images.githubusercontent.com/6376572/89250041-62b05700-d646-11ea-8b9e-f4023ffc8d20.png|image> Not Ready: 1. Icon (currently download and search from google image) 2. Copy rss feed button is better to popup a tip 'copied to clipboard'.

github 14:37:27

Comment on #188 script to fetch google analytics stats

It would be neat if we have comment noting that this block may (implicitly) modify entries that is pushed into `bulkUpdates` in previous loop.

github 14:37:28

Review on #188 script to fetch google analytics stats

Thanks for the fix. Please allow me to spend more time inspecting test cases. I need more time to verify if `processReport`is calling `upsertDocStats` with correct `bulk` for the given mocked `rows`.

2020-08-05

github 01:38:48

Review on #188 script to fetch google analytics stats

Finished looking at the test cases of `processReport` and `upsertDocStats`. They look great :)

github 01:38:48

Comment on #188 script to fetch google analytics stats

If we are going to repeat the `source` of the script many many times in a batch, I think we may consider using stored scripts. <https://www.elastic.co/guide/en/elasticsearch/reference/6.8/modules-scripting-using.html#modules-scripting-stored-scripts|https://www.elastic.co/guide/en/elasticsearch/reference/6.8/modules-scripting-using.html#modules-scripting-stored-scripts> We can update the stored script everytime the cron job starts to make sure the script stored in database is the latest one.

mrorz 10:54:53
今天又要開會囉
有很多要討論的東西,包含 LINE bot richmenu、RSS 的教學、onboarding UX 等等
https://g0v.hackmd.io/@mrorz/cofacts-meeting-notes
github 13:53:25

#192 Hide obsolete categories

From <https://g0v.hackmd.io/r3NK7xssSwCNPFxthlw7tA#Category-%E6%A8%99%E8%A8%98-UI-%E6%AA%A2%E8%A8%8E|20200729> We should hide obsolete categories from users, but leave their articleCategories in tact (or back it up somewhere)

github 13:55:58

#306 Improve category dialog

From <https://g0v.hackmd.io/r3NK7xssSwCNPFxthlw7tA#Category-%E6%A8%99%E8%A8%98-UI-%E6%AA%A2%E8%A8%8E|20200729>: The add-category dialog is now too long and requires a lot of scrolling. Solution TBA.

github 13:58:53

#307 List 25 items per page in list page UIs

From <https://g0v.hackmd.io/r3NK7xssSwCNPFxthlw7tA#%E3%80%8C%E7%AD%89%E4%BD%A0%E4%BE%86%E7%AD%94%E3%80%8D%E7%9A%84%E5%95%8F%E9%A1%8C|20200729> Currently article list page only show 10 item at a time. We should increase the number to 25 to reduce the need to click "Load more".

github 14:04:22

#308 Show URL title in article list

From <https://g0v.hackmd.io/r3NK7xssSwCNPFxthlw7tA#%E3%80%8C%E7%AD%89%E4%BD%A0%E4%BE%86%E7%AD%94%E3%80%8D%E7%9A%84%E5%95%8F%E9%A1%8C|20200729> We can display URL's title text in article list. Figma: • Desktop <https://www.figma.com/file/DvmAQjMJCncuPORWKnljM1/Cofacts-website-MrOrz-s-copy?node-id=2675%3A221|https://www.figma.com/file/DvmAQjMJCncuPORWKnljM1/Cofacts-website-MrOrz-s-copy?node-id=2675%3A221> • Mobile <https://www.figma.com/file/DvmAQjMJCncuPORWKnljM1/Cofacts-website-MrOrz-s-copy?node-id=2673%3A220|https://www.figma.com/file/DvmAQjMJCncuPORWKnljM1/Cofacts-website-MrOrz-s-copy?node-id=2673%3A220>

github 14:12:53

#193 Reload preview for logged in users

From <https://g0v.hackmd.io/r3NK7xssSwCNPFxthlw7tA#%E5%A4%A7%E6%9D%BE%E6%AA%A2%E8%A8%8E|20200729> Sometimes our URL scrapper will be temporarily blocked, leaving useless URL previews like <https://cofacts.org/article/39n5zi0t4vlci|this>. During <https://g0v.hackmd.io/r3NK7xssSwCNPFxthlw7tA#Youtube-%E9%98%BB%E6%93%8B-metadata-scraping|20200729 discussion> we can provide an API for logged in editors to reload broken review. Spec TBA

github 14:33:43

#309 Show "OK" button in mobile article filter dialog

From <https://g0v.hackmd.io/r3NK7xssSwCNPFxthlw7tA#%E6%9C%AA%E7%AB%9F%E9%A0%85%E7%9B%AE1|20200729>: &gt; 手機上 filter 按 X 會以為是 cancel,實驗之後才發現是按下去就會 apply Figma: "Apply filter" <https://www.figma.com/file/zpD45j8nqDB2XfA6m2QskO/Cofacts-website?node-id=2322%3A0|https://www.figma.com/file/zpD45j8nqDB2XfA6m2QskO/Cofacts-website?node-id=2322%3A0> We can retain the behavior that pressing filter buttons starts filter process immediately (because it's the current behavior and is easier to implement), but add an "OK" button at the bottom of filter dialog so that user can click to close filter dialog.

github 14:39:42

#310 Reply request submission button jumps around when clicking

*As-is* <https://user-images.githubusercontent.com/108608/89379879-2f92c400-d729-11ea-90f9-bcc7bcea0bbc.gif|reason-submission> 1. After inputting reason, the reason submission button jumps up and cancels the first click 2. During submission, there is no visual feedback indicating that the reply request is being sent *To-be* 1. Don't change reason input height when reason input is blurred 2. Add a loading state (or simply disable the submission button) to reason submission button when it's being submitted to server.

mrorz 15:43:03
上面洗版的 github issue 是來自上週的討論項目,列表在這裡:https://g0v.hackmd.io/rEtx-47bRHeW2aM05DINDQ#%E4%B8%8A%E6%AC%A1%E6%9C%AA%E7%AB%9F%E4%BA%8B%E9%A0%85%E9%96%8B%E7%A5%A8

今晚開會也會帶過ㄛ

g0v.hackmd.io

20200805 會議記錄 - HackMD

github 18:25:00

Comment on #304 Fix 262 rss function

I also wanted to remove the dependency of `ArticlePageLayout.getQueryVars` from `api/articles/[feed]`, but it seems that it's not easy to directly convert `listQueryVars` to querystring. <https://user-images.githubusercontent.com/6376572/89401059-5102a880-d747-11ea-8185-e96c7c51b9c3.png|螢幕快照 2020-08-05 下午6 01 23> I'm afraid that I should write a `listQueryVarsToQuerystring` function and another `getQueryVars` for `api/articles/[feed]`.

github 18:28:55

Comment on #304 Fix 262 rss function

I was thinking a direct `JSON.stringify` should do the job :P Is its result too long for an URL?

github 18:49:03

Comment on #304 Fix 262 rss function

Sorry, I mean querystring, not JSON. Maybe I can encode filter and orderBy like <https://stackoverflow.com/questions/41641426/passing-objects-as-url-parameters-c-sharp|this>.

mrorz 20:38:27
原 article list URL: 162 byte (但 `REPLY_BY_ME` 還另外需要 user id)
```filters=REPLIED_MANY_TIMES%2CNO_USEFUL_REPLY_YET%2CREPLIED_BY_ME&amp;types=OPINIONATED&amp;categoryIds=lT3h7XEBrIRcahlYugqq%2Ckj287XEBrIRcahlYvQoS%2CmD2n7nEBrIRcahlYLAr7```
query json:358 byte
```{"filter":{"replyRequestCount":{"GTE":1},"categoryIds":["lT3h7XEBrIRcahlYugqq","kj287XEBrIRcahlYvQoS","mD2n7nEBrIRcahlYLAr7","mj2n7nEBrIRcahlYdArf"],"replyCount":{"GTE":3},"hasArticleReplyWithMorePositiveFeedback":false,"articleRepliesFrom":{"userId":"AVqVwjqQyrDaTqlmmp_a","exists":true},"replyTypes":["OPINIONATED"]},"orderBy":[{"lastRequestedAt":"DESC"}]}```
json + urlencode: 520 byte
```%7B%22filter%22%3A%7B%22replyRequestCount%22%3A%7B%22GTE%22%3A1%7D%2C%22categoryIds%22%3A%5B%22lT3h7XEBrIRcahlYugqq%22%2C%22kj287XEBrIRcahlYvQoS%22%2C%22mD2n7nEBrIRcahlYLAr7%22%2C%22mj2n7nEBrIRcahlYdArf%22%5D%2C%22replyCount%22%3A%7B%22GTE%22%3A3%7D%2C%22hasArticleReplyWithMorePositiveFeedback%22%3Afalse%2C%22articleRepliesFrom%22%3A%7B%22userId%22%3A%22AVqVwjqQyrDaTqlmmp_a%22%2C%22exists%22%3Atrue%7D%2C%22replyTypes%22%3A%5B%22OPINIONATED%22%5D%7D%2C%22orderBy%22%3A%5B%7B%22lastRequestedAt%22%3A%22DESC%22%7D%5D%7D```
json-url (lzma + base64): 332 bytes
```XQAAAAIsAQAAAAAAAABBKYjGk5HEXfadWXKw7jepkVvpHoaw6thdjhrr-b1XehFIp1i0lfNrdYZgFkEbF3OF5gn8efegcgyfPNxJUo1ugsbXwWOwRZ5d67whl8PiANl3KmDjNbsg1xWrJY5S3_cdF2twLn2_u5RKgHrQE-DPp_568wT_wym2FAQlLV6pr-bhqJEwOsubFr4Ih4vp37oJEVyT1kjMcRSLBlEycFbnSkTK9sVPQyWfwMnc5634qeydZhieKsknWJI7U7oWtEfsvqnk5boOkkzM0EVC_PneGyjKvck3KpXqE25WwyyIF411o32v_v_mSwQA```
ljharb/qs: 535 byte
```filter%5BreplyRequestCount%5D%5BGTE%5D=1&amp;filter%5BcategoryIds%5D%5B0%5D=lT3h7XEBrIRcahlYugqq&amp;filter%5BcategoryIds%5D%5B1%5D=kj287XEBrIRcahlYvQoS&amp;filter%5BcategoryIds%5D%5B2%5D=mD2n7nEBrIRcahlYLAr7&amp;filter%5BcategoryIds%5D%5B3%5D=mj2n7nEBrIRcahlYdArf&amp;filter%5BreplyCount%5D%5BGTE%5D=3&amp;filter%5BhasArticleReplyWithMorePositiveFeedback%5D=false&amp;filter%5BarticleRepliesFrom%5D%5BuserId%5D=AVqVwjqQyrDaTqlmmp_a&amp;filter%5BarticleRepliesFrom%5D%5Bexists%5D=true&amp;filter%5BreplyTypes%5D%5B0%5D=OPINIONATED&amp;orderBy%5B0%5D%5BlastRequestedAt%5D=DESC```
Lien Chen 23:10:56
@screen.leon has joined the channel

2020-08-06

github 00:18:13

Comment on #187 Load more reply request at once

Merged because opened for 10+ days without review.

github 03:05:32

Comment on #305 RSS UI

Is this file used anywhere?

github 03:05:32

Comment on #305 RSS UI

Maybe <https://material-ui.com/api/list-item-text/#props|`secondaryTypographyProps`> could help? e.g. ``` &lt;ListItemText primary={t`Email`} secondary={t`Via Feedrabbit`} secondaryTypographyProps={{variant: 'subtitle2', color: 'textSecondary'}} /&gt; ``` Actually I think it's OK if we use Material UI's default style for `&lt;ListItemText&gt;`. The figma link provided by <https://github.com/LucienLee|@LucienLee> is at wireframe granularity, the font size &amp; color is not fine tuned yet. The actual design system used in mockup is defined here: <https://www.figma.com/file/3tGoYDSrqU8Rqmstyn9NMn/Cofacts-UI-Design-Kit?node-id=0%3A5005|https://www.figma.com/file/3tGoYDSrqU8Rqmstyn9NMn/Cofacts-UI-Design-Kit?node-id=0%3A5005> We can go for Material UI's default first, and see if we need any adjustment in the future.

github 03:05:32

Comment on #305 RSS UI

he figma link provided by <https://github.com/LucienLee|@LucienLee> is at wireframe granularity, the font size &amp; color is not fine tuned yet. The actual design system used in mockup is defined here: <https://www.figma.com/file/3tGoYDSrqU8Rqmstyn9NMn/Cofacts-UI-Design-Kit?node-id=0%3A5005|https://www.figma.com/file/3tGoYDSrqU8Rqmstyn9NMn/Cofacts-UI-Design-Kit?node-id=0%3A5005> I suggest we use `&lt;Typography variant="caption"&gt;` for all `&lt;CustomTypography variant="subtitle2"&gt;`, and use `&lt;Typography variant="body2"&gt;` for all `&lt;CustomTypography variant="subtitle1"&gt;`.

github 03:05:33

Review on #305 RSS UI

The UI is beautiful &lt;3 thanks a lot! I have some suggestions regarding using the use of `CustomTypography` and code dependencies.

github 03:05:33

Comment on #305 RSS UI

We may need to add `copy-to-clipboard` to `package.json`, because it's currently a dev dependency. It won't be installed in production environment. I think `copy-to-clipboard`'s API is much simpler than the ClipboardJS we are currently using. Please consider removing `"clipboard": "^2.0.4"` and replace its usage (only in `CopyButton`) with copy-to-clipboard.

joshuatw 09:11:15
@joshuatw has joined the channel

2020-08-08

ichieh 13:39:06
今天在 SITCON 遇到一群小夥伴,他們用 Cofacts 的資料庫做了 chrome 的外掛插件,讚讚!
https://github.com/DSC-TW/SC-cofacts-extension

DSC-TW/SC-cofacts-extension

5438arthur 13:39:50
@5438arthur has joined the channel
alanhc 13:42:20
@alanhc.tseng1999 has joined the channel
wanlin 16:03:45
@leo12345639 has joined the channel

2020-08-09

github 00:22:00

Review on #304 Fix 262 rss function

LGTM! It's beautiful. Thanks a million!

github 15:25:28

Review on #188 script to fetch google analytics stats

LGTM now! <https://github.com/ztsai|@ztsai> would you like to add <https://github.com/cofacts/rumors-api/pull/188#discussion_r465199034|stored script mechanism> in this PR or do you want to do it in another PR as an enhancement item?

github 19:24:46

Review on #305 RSS UI

Most of the PR looks good! Some final comments on `&lt;IFTTTItem&gt;` here.

github 19:24:47

Comment on #305 RSS UI

It's not recommended to use `&lt;table&gt;` for something that is not a data table. I suggest using flexbox for this particular UI, which should yield much simpler DOM and CSS style.

github 19:24:47

Comment on #305 RSS UI

If we use <https://material-ui.com/components/dialogs/|`Dialog`> instead of its underlying component <https://material-ui.com/components/modal/|`Modal`>, we would not need to define paper style and positioning here. Dialogs are not necessary used with `DialogTitle`, `DialogContent` etc, it can be used alone, as shown by `ListPageControls`' `Filters`.

lucien 19:48:13
@mrorz @bil @ggm
關於/教學的初步 spec ,等等我會跟 @stbb1025 討論,不過這一兩頁屬於文案很多的頁面,需要大家一起填肉,另外想問問條款需要放在 about 嗎?有什麼項目大家還想放的
https://g0v.hackmd.io/sB_zayWjTo-W0R7xe_U34w?both

g0v.hackmd.io

關於 / 教學 - HackMD

github 20:31:34

#311 ArticleReplyFeedbackControl revamp

This PR is a revamp to this component, including its popover: <https://user-images.githubusercontent.com/108608/89732120-487bdc00-da7f-11ea-9af3-06cdb1f5c13e.png|image> • Rename `ReplyFeedback` to `ArticleReplyFeedbackControl`.

github 23:57:13

#312 Upgrade to Apollo Client 3

Apollo Client 3 stable is <https://www.apollographql.com/blog/announcing-the-release-of-apollo-client-3-0/|released>. This PR <https://www.apollographql.com/docs/react/migrating/apollo-client-3-migration/|migrates Apollo Client to 3> and <https://github.com/adamsoffer/next-apollo/blob/master/CHANGELOG.md|next-apollo to latest>. `dataIdFromObject` we are currently using is deprecated in Apollo Client 3; however, in this PR I leave this intact because there are already too much code change.

2020-08-10

github 00:02:35

Comment on #312 Upgrade to Apollo Client 3

Leaving out `{ssr: true}` in `withApollo` here will cause next.js's `useRouter` returning `{}` for `router.query` on server-side rendering &amp; first render on client, because next.js will think the page <https://nextjs.org/docs/api-reference/next/router#router-object|does not meet data fetching requirements>. It's valid for search page because this page is not meant to be accessed directly nor by search engines.

github 15:37:50

Comment on #188 script to fetch google analytics stats

Thanks! I'll add that in a separate PR as discussed offline

stbb1025 18:32:18
https://drive.google.com/drive/folders/15z_3lhJgvxuQU8jw82hYLxSmbqfAskDd?fbclid=IwAR0WUKD_YhREOQI03WXnl1mARIoyTXJxQAGfrXHGqzJaLfR-qL6ZbrefJi0
成就的圖出爐囉,先是最基本版的,之後會隨著n值提高更改顏色跟加上光芒
bil 20:28:05
豪好看~~
github 21:49:35

#194 Load script

follow up PR for <https://github.com/cofacts/rumors-api/pull/188#discussion_r465199034|stored script mechanism> as discussed in <https://github.com/cofacts/rumors-api/pull/188|#188>

github 21:53:19

Comment on #194 Load script

<https://coveralls.io/builds/32649138|Coverage Status> Coverage increased (+0.5%) to 88.065% when pulling *<https://github.com/cofacts/rumors-api/commit/d2c3cd52b7a7cf5759f6ddcfbed3505328396c9c|d2c3cd5> on loadScript* into *<https://github.com/cofacts/rumors-api/commit/2ee6cc9cc8c654b25e2eaae2aa7b7558a7f1d9ee|2ee6cc9> on master*.

github 23:42:25

Comment on #126 Update all dependencies

*:warning: Artifact update problem* Renovate failed to update an artifact related to this branch. You probably do not want to merge this PR as-is. :recycle: Renovate will retry this branch, including artifacts, only when one of the following happens: • any of the package files in this branch needs updating, or • the branch becomes conflicted, or • you check the rebase/retry checkbox if found above, or • you rename this PR's title to start with "rebase!" to trigger it manually The artifact failure details are included below: *File name: package-lock.json*

2020-08-11

github 01:40:24

Review on #190 Analytics data loader

LGTM! Thanks for the code change :rocket:

github 01:41:40

Review on #191 Article stats

LGTM! Lets <https://github.githubassets.com/images/icons/emoji/shipit.png|:shipit:>

github 01:46:18

Review on #194 Load script

LGTM! :+1: :+1: :+1:

github 13:26:47

#214 Don't ask for article source in LIFF anymore

Spec TBD • 提案&過去討論整理:<https://g0v.hackmd.io/r3NK7xssSwCNPFxthlw7tA?view#chatbot-%E4%B8%8D%E5%95%8F%E4%BE%86%E6%BA%90|https://g0v.hackmd.io/r3NK7xssSwCNPFxthlw7tA?view#chatbot-%E4%B8%8D%E5%95%8F%E4%BE%86%E6%BA%90> • 方案選擇、文案討論:<https://g0v.hackmd.io/@mrorz/cofacts-meeting-notes/%2FrEtx-47bRHeW2aM05DINDQ|https://g0v.hackmd.io/@mrorz/cofacts-meeting-notes/%2FrEtx-47bRHeW2aM05DINDQ>

mrorz 13:45:35
明天開會預計的議程:
https://g0v.hackmd.io/@mrorz/cofacts-meeting-notes/%2FeitM7s0bSSeS3kg-MAcpDw
有些是上週沒討論到的東西~
ba 20:09:07
@angel112811a has left the channel

2020-08-12

github 21:28:30

Comment on #305 RSS UI

It seems that doesn't need to set success at next tick

github 22:17:36

#215 Fix issue, node process won't exit automatically when running notify

Disconnect redis and mongodb at the end of `scanRepliesAndNotify`.

github 22:21:33

Comment on #215 Fix issue, node process won't exit automatically when running notify

*Pull Request Test Coverage Report for <https://coveralls.io/builds/32706266|Build 1098>* • *2* of *2* *(100.0%)* changed or added relevant lines in *1* file are covered. • No unchanged relevant lines lost coverage. • Overall coverage increased (+*0.03%*) to *84.292%* * * * * * * *:yellow_heart: - <https://coveralls.io|Coveralls>*

2020-08-13

github 10:50:02

Review on #215 Fix issue, node process won't exit automatically when running notify

LGTM! Will test it out when I have access to latop.

github 14:15:16

Review on #215 Fix issue, node process won't exit automatically when running notify

It is really working :+1: I suggest adding `.catch((e) =&gt; {console.error(e); process.exit(1)})` to the end of `scanRepliesAndNotify()` so that errors can be seen when there are exceptions.

github 23:33:21

#45 [WIP] Add migration script for analytics index

1. Add migration script for `analytics` index so that we can add the new index mapping to existing database 2. Make `db/loadSchema.js` exit with non-zero return code so that Travis can capture the error when schema is invalid Need <https://github.com/ztsai|@ztsai> 's help looking at errors inserting index to database :pray:

2020-08-14

github 13:25:41

#195 Update rumors-db with analytics index mappings

• Update `test/rumors-db` submodule revision • Update unit test snapshot accordingly • Mostly about index name change (alias --&gt; real index name) • Seems that elasticsearch is returning different order for indexes that has mappings. Not sure why, but I'll just accept that :P

github 13:29:18

Comment on #195 Update rumors-db with analytics index mappings

<https://coveralls.io/builds/32752094|Coverage Status> Coverage remained the same at 88.265% when pulling *<https://github.com/cofacts/rumors-api/commit/3bb554cb0d76052fc62e40b1b2ed054aec82ecdc|3bb554c> on rumors-db-update* into *<https://github.com/cofacts/rumors-api/commit/a0a43a18523b0a595b15b48bef9a03c4102b8d73|a0a43a1> on master*.

mrorz 13:29:28
以上兩個 PR 是我試著 deploy analytics到 staging 所開的
• rumors-db 的那個補上 migration script 讓我比較好做 operation (順便修掉 typo、讓 CI 更能偵錯)
• rumors-api 那個則是讓 unit test 能使用最新的 rumors-db schema (須等 rumors-db 的 PR merge 之後再更新一次版本)
麻煩 @zoetwca 幫忙 review 了 m(_ _)m
github 18:05:12

Review on #195 Update rumors-db with analytics index mappings

For the unit tests, now that I think about it, the returned order is more or less random since all _score values are 1. It would probably be better to provide sorting options in the unit tests since we are doing snapshot matches and the indeterminacy of returned order makes them unreliable. But it's outside of the scope of this PR, I can do that in a separate PR.

github 22:56:01