Related papers notification #886

maxkfranz · 2020-10-28T16:25:13Z

Details:

This is currently a prototype.
A new email template type is defined as relatedPaperNotification.
The emails are triggered whenever the related papers are updated for a document.
A new metadata field is available on a document. doc.relatedPapersNotified() can be used to set or get whether the notifications have been sent. This prevents duplicates.
Novel interactions are an optional enhancement in the emails: An author can receive an email whether or not the originating factoid has a novel interaction. However, if there is a novel interaction, this should be mentioned in the email.
Currently, only notifications are sent to the terminal. The emails are disabled until the template is set up.
Action items are marked by // TODO RPM

Action items:

Add a template to Mailjet (@jvwong), based on our email document on Google Docs.
Reuse our Indra querying code to determine whether there is a novel interaction -- i.e. an interaction that has zero results in Indra (@metincansiper).
Verify that our emails send correctly on unstable. We'll temporary set unstable to send these notifications to Max and Jeff (@maxkfranz @jvwong).
Finalise the email template text (@maxkfranz).
Research the feasibility of the bonus 'factoidable' nudge: Can we determine using AI whether a paper is a good candidate for Biofactoid (@cannin @JohnGiorgi @jvwong)?

Ref. : Novel interaction notification #858

@jvwong

Details: - This is currently a prototype. - A new email template type is defined as `relatedPaperNotification`. - The emails are triggered whenever the related papers are updated for a document. - A new metadata field is available on a document. `doc.relatedPapersNotified()` can be used to set or get whether the notifications have been sent. This prevents duplicates. - Novel interactions are an optional enhancement in the emails: An author can receive an email whether or not the originating factoid has a novel interaction. However, if there is a novel interaction, this should be mentioned in the email. - Currently, only notifications are sent to the terminal. The emails are disabled until the template is set up. - Action items are marked by `// TODO RPM` Action items: - [ ] Add a template to Mailjet (@jvwong), based on our email document on Google Docs. - [ ] Reuse our Indra querying code to determine whether there is a novel interaction -- i.e. an interaction that has zero results in Indra (@metincansiper). - [ ] Verify that our emails send correctly on unstable. We'll temporary set unstable to send these notifications to Max and Jeff (@maxkfranz @jvwong). - [ ] Finalise the email template text (@maxkfranz). - [ ] Research the feasibility of the bonus 'factoidable' nudge: Can we determine using AI whether a paper is a good candidate for Biofactoid? Ref. : Novel interaction notification #858

maxkfranz · 2020-10-28T18:59:55Z

Note: The linting errors will show you the main places that still need to be updated.

jvwong · 2020-11-03T21:23:24Z

I had one refinement for the pool of papers used to select authors to contact:

factoid/src/server/routes/api/document/index.js

Line 1268 in 715ede0

const papers = await getSendablePapers(doc.relatedPapers());

Document-level (doc.relatedPapers()) derive from NCBI's EUTILS ELINK service, using the Biofactoid document's article PMID as the query to retrieve what PubMed defines as a linked article. Now recall Biofactoid does some filtering and sorting (semantic-search) before setting the result in the relatedPapers.

A subset of the raw set of ELINK articles retrieved consist of:

articles citing the one in Biofactoid (incoming)
articles referenced by the one in Biofactoid (outgoing)

So the idea is to select from these incoming and outgoing articles for authors to contact. There's a few reasons to select these:

It's self-evident why a researcher would be interested: Someone cited your work, here, take a look in Biofactoid
The article in Biofactoid was based on research/interactions in the outgoing subset, and similarly, the incoming subset would often be building upon the Biofactoid article
The ELINK service wrapper already exists, you can filter by how recently it was published, and just needs a little update
An article being related to the current Factoid might not correlate well with its ability to be deposited etc

maxkfranz · 2020-11-03T22:14:05Z

Great points, Jeff.

Re.:

An article being related to the current Factoid might not correlate well with its ability to be deposited etc

This applies to the bonus nudge, but we can still notify authors of papers that aren't good candidates to be deposited. Emails for those papers would exclude the nudge, but those emails would still help to increase mindshare for Biofactoid.

maxkfranz · 2020-11-05T19:26:54Z

Template info:

        "TemplateID": 1871553,
        "TemplateLanguage": true,
        "Subject": "Test of related papers notification",
        "Variables": {}

- Accept an optional filter for linknames

…he 'getRelPprsForDoc'

jvwong · 2020-11-06T20:34:27Z

So I'm setting a new document attribute referencedPapers and its being done alongside the relatedPapers so that it hooks into all of the lifecycle updates.

Notes
- It's entirely possible that the the references are not made available to PubMed for whatever reason.
- Right now the emailRelatedPaperAuthors gets triggered on doc creation, which means the document wont have any entries much less submitted. So I guess it should be triggered post submit?

maxkfranz · 2020-11-09T16:36:31Z

Right now the emailRelatedPaperAuthors gets triggered on doc creation, which means the document wont have any entries much less submitted. So I guess it should be triggered post submit?

This should only get triggered at the same time as related papers are saved (or refreshed). That should be done on submission, not on creation. Was there a reason for changing the related papers behaviour to search on creation?

Alternatively, emailRelatedPaperAuthors() could bail out if the doc state isn't submitted.

maxkfranz · 2020-11-09T16:40:25Z

Re.

Reuse our Indra querying code to determine whether there is a novel interaction -- i.e. an interaction that has zero results in Indra (@metincansiper).

It's not absolutely necessary to have this feature for an initial version of the notifications, but it would be great in future iterations to vary the email template based on this factor. @metincansiper, would it be possible to use a metadata field (e.g. intn.isNovel()) in the interactions and just mark them as novel or not during the existing related papers queries?

src/server/routes/api/document/index.js

jvwong · 2020-11-09T17:00:14Z

Right now the emailRelatedPaperAuthors gets triggered on doc creation, which means the document wont have any entries much less submitted. So I guess it should be triggered post submit?

This should only get triggered at the same time as related papers are saved (or refreshed). That should be done on submission, not on creation. Was there a reason for changing the related papers behaviour to search on creation?

I retrieve document-level related papers on creations, because they are drawn from PubMed, and so that there was something to show in the explorer-view immediately post-submit (in fact, on create).

…g switched to 'public', following submission - EMAIL_RELPPRS_CONTACT config that flags whether the relatedPapersEmails should be sent to contacts (default true)

jvwong · 2020-11-09T19:13:52Z

OK two updates:

Put the code for sending these emails in the set of post-public actions to take (tweet, updated related papers, send follow up emails).
Another config EMAIL_RELPPRS_CONTACT that by default, sends to the contact address unless explicitly instructed not to, in which case sends to EMAIL_ADMIN_ADDR. So this won't require any changes to Docker-compose/prod at least.

maxkfranz · 2020-11-09T19:38:35Z

Put the code for sending these emails in the set of post-public actions to take (tweet, updated related papers, send follow up emails).

OK, I had it together with getting the related papers so that the admin UI could just refresh things with the existing refresh button. In case of an Indra failure, for example, the related papers may not exist and the emails would never be sent. The refresh button would allow for both of those to recover, and the related papers refreshing should already be integrated into the admin panel.

The new approach is fine, but the admin UI would now need to explicitly send these emails on refresh (if the sent flag is false).

metincansiper · 2020-11-09T19:47:24Z

It's not absolutely necessary to have this feature for an initial version of the notifications, but it would be great in future iterations to vary the email template based on this factor. @metincansiper, would it be possible to use a metadata field (e.g. intn.isNovel()) in the interactions and just mark them as novel or not during the existing related papers queries?

@maxkfranz I made the requested change in novel_intn branch (https://github.com/PathwayCommons/factoid/tree/novel_intn). Should I make a PR to your branch used in this PR? BTW I considered when an interaction has no result right after the query made for the interaction. Later, the interactions with not enough result are assigned with some random results from the document. I did not consider that step since I think that any interaction will not be novel in that sense as long as the document has enough results.

jvwong · 2020-11-09T19:51:27Z

Put the code for sending these emails in the set of post-public actions to take (tweet, updated related papers, send follow up emails).

OK, I had it together with getting the related papers so that the admin UI could just refresh things with the existing refresh button. In case of an Indra failure, for example, the related papers may not exist and the emails would never be sent. The refresh button would allow for both of those to recover, and the related papers refreshing should already be integrated into the admin panel.

The new approach is fine, but the admin UI would now need to explicitly send these emails on refresh (if the sent flag is false).

Oh I see now. If you want it tied to the existing admin refresh, i can do that by adding it below this:

factoid/src/server/routes/api/document/index.js

Line 1541 in 062ee8c

await updateRelatedPapers( doc );

maxkfranz · 2020-11-09T19:57:08Z

Sounds good

…

On Nov 9, 2020, at 14:51, Jeffrey ***@***.***> wrote: Put the code for sending these emails in the set of post-public actions to take (tweet, updated related papers, send follow up emails). OK, I had it together with getting the related papers so that the admin UI could just refresh things with the existing refresh button. In case of an Indra failure, for example, the related papers may not exist and the emails would never be sent. The refresh button would allow for both of those to recover, and the related papers refreshing should already be integrated into the admin panel. The new approach is fine, but the admin UI would now need to explicitly send these emails on refresh (if the sent flag is false). Oh I see now. If you want it tied to the existing admin refresh, i can do that by adding it below this: https://github.com/PathwayCommons/factoid/blob/062ee8cd52de38679d4db68c72614f30c761ee3e/src/server/routes/api/document/index.js#L1541 <https://github.com/PathwayCommons/factoid/blob/062ee8cd52de38679d4db68c72614f30c761ee3e/src/server/routes/api/document/index.js#L1541> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#886 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHRO47HHIZGKHG4KY5KD6DSPBB45ANCNFSM4TCRZ5DQ>.

maxkfranz · 2020-11-09T20:00:00Z

It's not absolutely necessary to have this feature for an initial version of the notifications, but it would be great in future iterations to vary the email template based on this factor. @metincansiper, would it be possible to use a metadata field (e.g. intn.isNovel()) in the interactions and just mark them as novel or not during the existing related papers queries?

@maxkfranz I made the requested change in novel_intn branch (https://github.com/PathwayCommons/factoid/tree/novel_intn). Should I make a PR to your branch used in this PR? BTW I considered when an interaction has no result right after the query made for the interaction. Later, the interactions with not enough result are assigned with some random results from the document. I did not consider that step since I think that any interaction will not be novel in that sense as long as the document has enough results.

@metincansiper, it's fine to just cherry-pick your commits into this branch.

maxkfranz · 2020-11-10T16:44:53Z

This branch is now ready to be merged for testing on the unstable instance.

maxkfranz · 2020-11-10T16:46:08Z

@jvwong, you can either merge this in now or wait for @metincansiper to add his commits re. Indra. If you want to merge sooner, @metincansiper could add his changes in a separate PR.

jvwong · 2020-11-10T17:05:17Z

@jvwong, you can either merge this in now or wait for @metincansiper to add his commits re. Indra. If you want to merge sooner, @metincansiper could add his changes in a separate PR.

@metincansiper can you add your commits to this branch?

metincansiper · 2020-11-10T18:25:28Z

@jvwong I added my commit into this branch

…ailRelatedPaperAuthors()` re. `doc.relatedPapersNotified()`

maxkfranz · 2020-11-10T18:35:15Z

OK, should be good to go

jvwong · 2020-11-10T19:09:15Z

This is throwing some error on build:


WARNING in ./node_modules/colors/lib/colors.js
127:29-43 Critical dependency: the request of a dependency is an expression
 @ ./node_modules/colors/lib/colors.js
 @ ./node_modules/colors/safe.js
 @ ./node_modules/winston/lib/winston/config.js
 @ ./node_modules/winston/lib/winston.js
 @ ./src/server/logger.js
 @ ./src/util/email.js
 @ ./src/util/index.js
 @ ./src/client/index.js

ERROR in ./node_modules/winston/lib/winston/common.js
Module not found: Error: Can't resolve 'fs' in '/Users/.../factoid/node_modules/winston/lib/winston'
 @ ./node_modules/winston/lib/winston/common.js 12:9-22
 @ ./node_modules/winston/lib/winston.js
 @ ./src/server/logger.js
 @ ./src/util/email.js
 @ ./src/util/index.js
 @ ./src/client/index.js

ERROR in ./node_modules/winston/lib/winston/transports/file.js
Module not found: Error: Can't resolve 'fs' in '/Users/.../factoid/node_modules/winston/lib/winston/transports'
 @ ./node_modules/winston/lib/winston/transports/file.js 10:9-22
 @ ./node_modules/winston/lib/winston/transports.js
 @ ./node_modules/winston/lib/winston.js
 @ ./src/server/logger.js
 @ ./src/util/email.js
 @ ./src/util/index.js
 @ ./src/client/index.js
^C

maxkfranz · 2020-11-10T20:04:49Z

For now, you could remove winston from email.js.

However, email.js shouldn't strictly be under /util in the first place, since it's server-side only. These util files aren't universal and should be moved, or we'll have this problem again later:

document.js - server only
email.js - server only
dom.js - client only

jvwong · 2020-11-10T20:05:47Z

I was noticing that right now. I'm going to move it now.

jvwong · 2020-11-10T20:07:55Z

Clarification: I'm moving email.js only.

maxkfranz requested review from jvwong and metincansiper October 28, 2020 16:25

maxkfranz assigned jvwong and metincansiper Oct 28, 2020

maxkfranz self-assigned this Oct 29, 2020

jvwong added 4 commits November 6, 2020 15:19

Attribute for stashing references for the article

1d1a5ed

- Expose helper function elink2UidList (renamed from data2UidList)

6dd39cc

- Accept an optional filter for linknames

Add a capability for setting the subset of referenced papers inside t…

ddfb05c

…he 'getRelPprsForDoc'

Use the new set of referenced papers.

dd4b45a

maxkfranz commented Nov 9, 2020

View reviewed changes

src/server/routes/api/document/index.js Outdated Show resolved Hide resolved

jvwong added 2 commits November 9, 2020 12:22

Add an admin email, use in the relatedPapers emailer.

fa2050a

- emailRelatedPaperAuthors bundled with code for document status bein…

062ee8c

…g switched to 'public', following submission - EMAIL_RELPPRS_CONTACT config that flags whether the relatedPapersEmails should be sent to contacts (default true)

metincansiper added 2 commits November 9, 2020 11:32

Mark novel interactios.

d67ad38

remove a console statement

fc92759

Tie the relatedPapers contact emailing to the admin refresh

827d70b

maxkfranz added 3 commits November 10, 2020 11:27

Use logger to remove linting warning in preparation for merging

b6623cf

Set doc.relatedPapersNotified() flag

fbbef39

Set template ID

bf0f241

metincansiper added 2 commits November 10, 2020 10:14

Set the default novel value of interactions as false.

41b4bfe

Merge branch 'novel_intn' into email-rel-pprs

88fab26

Add interactions filter based on isNovel() and add guard within `em…

5526dd7

…ailRelatedPaperAuthors()` re. `doc.relatedPapersNotified()`

maxkfranz merged commit bee0cd5 into unstable Nov 10, 2020

jvwong mentioned this pull request Nov 10, 2020

Moving email.js util to server-side. #898

Merged

jvwong deleted the email-rel-pprs branch November 16, 2020 17:18

Uh oh!

Related papers notification #886

Related papers notification #886

Uh oh!

Conversation

maxkfranz commented Oct 28, 2020

Uh oh!

maxkfranz commented Oct 28, 2020

Uh oh!

jvwong commented Nov 3, 2020

Uh oh!

maxkfranz commented Nov 3, 2020

Uh oh!

maxkfranz commented Nov 5, 2020

Uh oh!

jvwong commented Nov 6, 2020

Uh oh!

maxkfranz commented Nov 9, 2020

Uh oh!

maxkfranz commented Nov 9, 2020

Uh oh!

Uh oh!

jvwong commented Nov 9, 2020

Uh oh!

jvwong commented Nov 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maxkfranz commented Nov 9, 2020

Uh oh!

metincansiper commented Nov 9, 2020

Uh oh!

jvwong commented Nov 9, 2020

Uh oh!

maxkfranz commented Nov 9, 2020 via email

Uh oh!

maxkfranz commented Nov 9, 2020

Uh oh!

maxkfranz commented Nov 10, 2020

Uh oh!

maxkfranz commented Nov 10, 2020

Uh oh!

jvwong commented Nov 10, 2020

Uh oh!

metincansiper commented Nov 10, 2020

Uh oh!

maxkfranz commented Nov 10, 2020

Uh oh!

jvwong commented Nov 10, 2020

Uh oh!

maxkfranz commented Nov 10, 2020

Uh oh!

jvwong commented Nov 10, 2020

Uh oh!

jvwong commented Nov 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jvwong commented Nov 9, 2020 •

edited

Loading