fix: record my own Discord profile at READY so a sending message keeps my avatar

crates/chatd/src/adapters/discord/events.rs

@@ -169,7 +169,10 @@ impl Discord {
             "CHANNEL_UNREAD_UPDATE" | "PASSIVE_UPDATE_V2" => self.passive(from_str(data)?)?,
             "USER_UPDATE" => {
                 let user: User = from_str(data)?;
-                self.ctx.describe(user.name(), &user.id)?;
+                // Ours: a new name or picture.
+                if user.id == self.me() {
+                    self.myself(&user)?;
+                }
             }
             "USER_SETTINGS_PROTO_UPDATE" => self.settings_proto(from_str(data)?)?,
             // Discord wants a phone number, an email check or a captcha: nothing more is asked of

crates/chatd/src/adapters/discord/ready.rs

@@ -625,6 +625,15 @@ impl Discord {
         self.ctx.conversation(&row)
     }
 
+    /// Who we are, from READY or a change to it: the account's name, and our own row in People,
+    /// which is where the account's picture is read from. The row goes first, so the status
+    /// the name announces already has the picture.
+    pub(super) fn myself(&self, user: &User) -> rusqlite::Result<()> {
+        self.ctx.media(&[user.avatar()])?;
+        self.ctx.people(&[user.person(None)])?;
+        self.ctx.describe(user.name(), &user.id)
+    }
+
     /// The user's server folders as JSON settings or protobuf ones carry them. Only a folder with
     /// an id is one; a server alone in a nameless one is simply not in a folder.
     fn folders(ready: &Ready) -> Option<Vec<proto::GuildFolder>> {
@@ -704,7 +713,7 @@ impl Discord {
     /// Takes READY apart into the store. Only [`super::State`] outlives this call.
     pub(super) fn ready(&self, ready: Ready) -> Result<(), Failure> {
         let me = ready.user.id.clone();
-        self.ctx.describe(ready.user.name(), &me)?;
+        self.myself(&ready.user)?;
         self.capabilities(ready.user.premium_type);
         let order = Self::order(&ready);
         let folders = Self::folders(&ready);

crates/chatd/tests/discord.rs

@@ -88,6 +88,9 @@ enum Control {
 
 struct Shared {
     requests: Mutex<Vec<Seen>>,
+    /// Fetches of our own picture, kept out of `requests`: chatd gets the rail's picture on its
+    /// own clock, which no test can time.
+    own_pictures: AtomicUsize,
     /// Every frame the adapter sent on the gateway, in order.
     frames: Mutex<Vec<Value>>,
     control: Mutex<Sender<Control>>,
@@ -393,7 +396,11 @@ fn serve_http(shared: Arc<Shared>, stream: TcpStream) {
             bytes: body.len(),
             at: Instant::now(),
         };
-        shared.requests.lock().unwrap().push(seen.clone());
+        if seen.path.starts_with("/media/avatars/1000/") {
+            shared.own_pictures.fetch_add(1, Ordering::Relaxed);
+        } else {
+            shared.requests.lock().unwrap().push(seen.clone());
+        }
         let bare = seen.path.split('?').next().unwrap().to_owned();
         // What is not Discord's JSON: a file from the media proxy, the page that names the
         // build, and whatever a test wrote out by hand.
@@ -578,6 +585,7 @@ impl World {
         let (control, controlled) = channel();
         let shared = Arc::new(Shared {
             requests: Mutex::new(Vec::new()),
+            own_pictures: AtomicUsize::new(0),
             frames: Mutex::new(Vec::new()),
             control: Mutex::new(control),
             scripted: Mutex::new(HashMap::new()),
@@ -968,6 +976,35 @@ fn ready_fills_spaces_trees_people_and_unread_state() {
     assert_eq!(&sent, properties);
 }
 
+#[test]
+fn my_picture_is_the_accounts_from_ready_and_follows_a_change_to_it() {
+    let world = World::start();
+    let mine = || match world.ask(Request::Status) {
+        Response::Status { accounts, .. } => {
+            let ours = accounts.into_iter().find(|account| account.key == world.account()).unwrap();
+            (ours.name, ours.avatar.map(|avatar| avatar.id))
+        }
+        other => panic!("{other:?}"),
+    };
+    // READY names our picture, so the account has it before any message of ours is seen: a
+    // message being sent, which names none, shows it. It is registered, and fetched for the
+    // rail with nothing asked by the shell.
+    assert_eq!(mine(), ("Justin".to_owned(), Some("avatar:1000:me0hash".to_owned())));
+    wait("our picture to be fetched", || (world.discord.own_pictures.load(Ordering::Relaxed) > 0).then_some(()));
+    // Our own row is on none of the lists we own.
+    let owned = [world.people(None, None), world.people(Some(Relation::Friend), None), world.people(Some(Relation::Blocked), None)];
+    assert!(owned.iter().flatten().all(|person| person.key.id != "1000"), "we are listed among our own people");
+
+    // A new picture is the account's at once.
+    world.discord.push("USER_UPDATE", json!({"id": "1000", "username": "justin", "global_name": "Justin", "discriminator": "0", "avatar": "me1hash"}));
+    wait("the new picture", || (mine().1.as_deref() == Some("avatar:1000:me1hash")).then_some(()));
+    // Someone else's is not ours.
+    world.discord.push("USER_UPDATE", json!({"id": "2001", "username": "mara", "global_name": "Mara Okafor", "discriminator": "0", "avatar": "a1b2c3"}));
+    world.incoming("3002", None, "after the update", json!({}));
+    wait("the update to be handled", || world.messages("3002").pop().filter(|message| message.text == "after the update"));
+    assert_eq!(mine(), ("Justin".to_owned(), Some("avatar:1000:me1hash".to_owned())));
+}
+
 #[test]
 fn messages_map_to_the_one_model_and_follow_live_events() {
     let world = World::start();
@@ -2407,8 +2444,9 @@ fn a_ready_of_real_size_connects_quickly_within_the_memory_budget_and_is_not_kep
     assert!(released || kept_by_allocator, "settled at {settled} KB after a peak of {peak} KB");
 
     // The first thirty seconds: five unread conversations fetched ahead, at a person's pace,
-    // and nothing else asked of Discord at all.
+    // our own picture once, and nothing else asked of Discord at all.
     sleep(Duration::from_secs(30).saturating_sub(started.elapsed()));
+    assert_eq!(world.discord.own_pictures.load(Ordering::Relaxed), 1);
     let requests = world.discord.requests.lock().unwrap().clone();
     let paths: Vec<&str> = requests.iter().map(|seen| seen.path.as_str()).collect();
     assert_eq!(requests.len(), 5, "{paths:?}");

crates/tinyclient/src/gui/conversation/draw.rs

@@ -500,18 +500,18 @@ impl Pane {
     /// Someone in this conversation, from what its messages say of them, else from the people
     /// hub: their name and their picture.
     fn who(&self, person: &Key, cx: &mut Context<Self>) -> (String, Option<MediaFile>) {
-        let root = self.open.as_ref().and_then(|open| open.root.as_ref()).map(|(root, _)| root);
-        let theirs = || root.into_iter().chain(&self.window.messages).rev().filter(|message| message.sender.id == person.id);
-        // A tombstone keeps the name and loses the picture, so one that has it is looked for first.
-        if let Some(message) = theirs().find(|message| message.sender.avatar.is_some()).or_else(|| theirs().next()) {
+        if let Some(message) = said_by(self.root(), &self.window.messages, &person.id) {
             return (message.sender.name.clone(), message.sender.avatar.clone());
         }
         let known = self.shell.upgrade().and_then(|shell| shell.read(cx).people_state.hub().person(person));
         known.map(|person| (person.name, person.avatar)).unwrap_or_default()
     }
 
-    /// My own picture on this account. A message I am sending is not from the network yet,
-    /// so it names no picture itself.
+    fn root(&self) -> Option<&Rc<Message>> {
+        self.open.as_ref().and_then(|open| open.root.as_ref()).map(|(root, _)| root)
+    }
+
+    /// My own picture on this account.
     fn own_picture(&self, cx: &mut Context<Self>) -> Option<MediaFile> {
         let key = self.key()?;
         self.shell.upgrade().and_then(|shell| shell.read(cx).own_picture(key))
@@ -527,8 +527,8 @@ impl Pane {
             None => Avatar::named(&sender.name, size),
         };
         let file = match (&sender.avatar, message.outgoing) {
-            (Some(file), _) => Some(file.clone()),
-            (None, true) => self.own_picture(cx),
+            (_, true) => own_face(message, self.root(), &self.window.messages, &me, || self.own_picture(cx)),
+            (Some(file), false) => Some(file.clone()),
             // A tombstone names its sender and nothing else; their other messages have the picture.
             (None, false) => self.person(id).and_then(|key| self.who(&key, cx).1),
         };
@@ -1705,3 +1705,87 @@ fn natural(attachment: &Attachment) -> (u32, u32) {
         (still.width, still.height)
     }
 }
+
+/// The message that says most of who someone is here: their newest with a picture, else their
+/// newest. A tombstone keeps the name and loses the picture, so one that has it is looked for first.
+fn said_by<'a>(root: Option<&'a Rc<Message>>, messages: &'a [Rc<Message>], id: &str) -> Option<&'a Message> {
+    let theirs = || root.into_iter().chain(messages).rev().filter(|message| message.sender.id == id);
+    let found = theirs().find(|message| message.sender.avatar.is_some()).or_else(|| theirs().next());
+    found.map(|message| &**message)
+}
+
+/// The picture on a message of mine: the one it names, else the account's, which follows a
+/// change of picture. One I am sending is not from the network yet and names none. Without the
+/// account's (a network that keeps no row of me), my newest message here lends the picture it
+/// was stored with, which may since have changed.
+fn own_face(
+    message: &Message,
+    root: Option<&Rc<Message>>,
+    messages: &[Rc<Message>],
+    me: &str,
+    account: impl FnOnce() -> Option<MediaFile>,
+) -> Option<MediaFile> {
+    let shown = || match me {
+        // Without my id nothing here is known to be mine.
+        "" => None,
+        me => said_by(root, messages, me)?.sender.avatar.clone(),
+    };
+    message.sender.avatar.clone().or_else(account).or_else(shown)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use protocol::MediaKind;
+
+    fn picture(id: &str) -> MediaFile {
+        MediaFile::new(id, MediaKind::Avatar)
+    }
+
+    /// One of mine the network has, with the picture it was stored with.
+    fn mine(id: &str, avatar: Option<&str>) -> Rc<Message> {
+        let mut message = Message::plain(id, "Jo", "said", 10, true);
+        message.sender.id = "me".into();
+        message.sender.avatar = avatar.map(picture);
+        Rc::new(message)
+    }
+
+    /// One of mine still going: chatd names no sender on it.
+    fn sending() -> Message {
+        let mut message = Message::plain("c1", "You", "going", 20, true);
+        message.state = SendState::Pending;
+        message
+    }
+
+    fn face(message: &Message, messages: &[Rc<Message>], account: Option<&str>) -> Option<String> {
+        own_face(message, None, messages, "me", || account.map(picture)).map(|file| file.id)
+    }
+
+    #[test]
+    fn my_message_shows_its_own_picture_then_the_accounts_then_my_newest_here() {
+        let messages = [mine("1", Some("shown-here"))];
+        assert_eq!(face(&mine("2", Some("its-own")), &messages, Some("account")).as_deref(), Some("its-own"));
+        // One being sent names none.
+        assert_eq!(face(&sending(), &messages, Some("account")).as_deref(), Some("account"));
+        assert_eq!(face(&sending(), &messages, None).as_deref(), Some("shown-here"));
+        assert_eq!(face(&sending(), &[], None), None);
+    }
+
+    #[test]
+    fn a_picture_i_have_since_changed_does_not_come_back_from_my_earlier_messages() {
+        let messages = [mine("1", Some("before")), mine("2", Some("before"))];
+        assert_eq!(face(&sending(), &messages, Some("after")).as_deref(), Some("after"));
+    }
+
+    #[test]
+    fn only_my_newest_picture_here_is_lent_and_only_once_my_id_is_known() {
+        // A newer one of mine without a picture, a tombstone, does not lose it.
+        let messages = [mine("1", Some("older")), mine("2", Some("newer")), mine("3", None)];
+        assert_eq!(face(&sending(), &messages, None).as_deref(), Some("newer"));
+        let mut theirs = Message::plain("4", "mara", "hi", 30, false);
+        theirs.sender.id = "mara".into();
+        theirs.sender.avatar = Some(picture("mara"));
+        assert_eq!(face(&sending(), &[Rc::new(theirs)], None), None);
+        assert_eq!(own_face(&sending(), None, &messages, "", || None), None);
+    }
+}