Page MenuHomePhabricator

[keyserver] Update sendPushNotif to detect mention for a ENS name
ClosedPublic

Authored by rohan on Dec 19 2023, 9:18 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 23, 2:01 AM
Unknown Object (File)
Nov 8 2024, 10:02 PM
Unknown Object (File)
Nov 8 2024, 7:17 PM
Unknown Object (File)
Nov 8 2024, 7:17 PM
Unknown Object (File)
Nov 8 2024, 7:17 PM
Unknown Object (File)
Nov 8 2024, 1:13 PM
Unknown Object (File)
Oct 11 2024, 8:54 AM
Unknown Object (File)
Oct 6 2024, 9:28 AM
Subscribers

Details

Summary

We currently bypass notif settings for when a user is @ mentioned, so they will get the notification even if they have backgrounded the chat. Now that we allow users to be @ mentioned by their ENS name, we need to update the conditions for userWasMentioned to detect if a user is mentioned.

This should be a pretty simple change, but I don't have the most context on notifs in general. Here, the idea is we:

  1. Get the resolved ENS name for the raw username provided (if exists)
  2. Instead of just checking if the raw username was mentioned, we should also check if the ENS name was mentioned
  3. If either of the two conditions are true, we consider the user to be mentioned (as opposed to just the raw username being mentioned)

Addressess https://linear.app/comm/issue/ENG-5074/update-sendpushnotif-to-detect-mention-for-a-ens-name

Depends on D10389

Test Plan

Please see the video below where I backgrounded a chat, then observed that the user got notifs for mentions including both their wallet address and ENS name

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rohan requested review of this revision.Dec 19 2023, 9:36 AM
keyserver/src/push/send.js
242 ↗(On Diff #34859)

Why is this variable declared in this scope? Is it ever used outside of the conditional below? (Sorrry if I'm missing it)

keyserver/src/push/send.js
242 ↗(On Diff #34859)

Oh yeah I meant to leave a comment saying I'll update this diff, but began to prioritize the urgent issue I was working on first. I'll fix this now, it should just be defined in the if block below

Define userInfosWithENSNames in the correct scope

This revision is now accepted and ready to land.Dec 20 2023, 6:25 AM