#2645: Only update the threadLastSeen if its later than the current one

[tomwhale]

Status

• WIP
• Ready for review
• Needs testing

Deploy after merge (delete what needn't be deployed)

• athena

[mxstbr]

Thanks so much, this is great! Just one tiny nitpick to make sure we don't encounter annoying bugs, then we'll be good 💯

[mxstbr]

Let's make sure to wrap these in new Date calls since they might be unix timestamps or date objects and we might be comparing proverbial apples to oranges:

if (new Date(record.lastSeen).getTime() > new Date(date).getTime())

[mxstbr]

This should be lastSeen: Date | number because I'm not certain we always store Dates in the db. (and not unix timestamps) Mind trying that and seeing if it breaks things? If it does, don't worry about it.

[mxstbr]

Thank you! Now flow fails due to a typing error, I think you'll just have to change athena/models/usersThreads.js:21:4 to return Promise<?DBUserThread>?

[mxstbr]

Hmm that Flow error is very weird. I'll try figuring it out locally, one sec.

[mxstbr]

I tried the same thing, but that throws another error. I think Flow wants us to explicitly check whether lastSeen is a Date before passing it to new Date?

[mxstbr]

Could that be it?

[tomwhale]

Sorry, only just seen your replies! I'll give that a go later.

[mxstbr]

No worries—if you can't fix that flow error just mark it as // $FlowIssue, I tried for a bit but have no clue how to fix it either 😕

[brianlovin]

The flow issue here is pretty weird, I dug into it last night. Basically if we change the type def to the callsite of the getUsersThread function, it works.

Before:

const record = await getUsersThread(userId, threadId);

After:

const record: ?DBUsersThreads = await getUsersThread(userId, threadId);

Then there will be a flow issue on line 39 where it cant coerce record.lastSeen to a string, so just add a .toString() there :)

[brianlovin]

Ref: facebook/flow#5294

[brianlovin]

Thanks for the fixes! We're working on the failing e2e tests in #2674