feature_flags: Fix issue where staff is not automatically signed into collab (#54332)

Bennet Bo Fenner created 1 week ago

Follow up to #54206

`on_flags_ready` relied on the fact that the `FeatureFlagStore` was only
set once the flags had been received from the server.
However, after #54206 the global gets instantiated earlier, without the
flags being resolved.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- N/A

Change summary

crates/feature_flags/src/feature_flags.rs | 14 +++++----
crates/feature_flags/src/store.rs         | 34 ++++++++++++++++++++++++
2 files changed, 42 insertions(+), 6 deletions(-)

Detailed changes

crates/feature_flags/src/feature_flags.rs

@@ -259,12 +259,14 @@ impl FeatureFlagAppExt for App {
     {
         self.observe_global::<FeatureFlagStore>(move |cx| {
             let store = cx.global::<FeatureFlagStore>();
-            callback(
-                OnFlagsReady {
-                    is_staff: store.is_staff(),
-                },
-                cx,
-            );
+            if store.server_flags_received() {
+                callback(
+                    OnFlagsReady {
+                        is_staff: store.is_staff(),
+                    },
+                    cx,
+                );
+            }
         })
     }
 

crates/feature_flags/src/store.rs

@@ -70,6 +70,7 @@ macro_rules! register_feature_flag {
 pub struct FeatureFlagStore {
     staff: bool,
     server_flags: HashMap<String, String>,
+    server_flags_received: bool,
 
     _settings_subscription: Option<Subscription>,
 }
@@ -95,12 +96,17 @@ impl FeatureFlagStore {
         self.staff
     }
 
+    pub fn server_flags_received(&self) -> bool {
+        self.server_flags_received
+    }
+
     pub fn set_staff(&mut self, staff: bool) {
         self.staff = staff;
     }
 
     pub fn update_server_flags(&mut self, staff: bool, flags: Vec<String>) {
         self.staff = staff;
+        self.server_flags_received = true;
         self.server_flags.clear();
         for flag in flags {
             self.server_flags.insert(flag.clone(), flag);
@@ -371,4 +377,32 @@ mod tests {
         assert_eq!(store.try_flag_value::<DemoFlag>(cx), None);
         assert_eq!(PresenceFlag::default(), PresenceFlag::Off);
     }
+
+    #[gpui::test]
+    fn on_flags_ready_waits_for_server_flags(cx: &mut gpui::TestAppContext) {
+        use crate::FeatureFlagAppExt;
+        use std::cell::Cell;
+        use std::rc::Rc;
+
+        cx.update(|cx| {
+            init_settings_store(cx);
+            FeatureFlagStore::init(cx);
+        });
+
+        let fired = Rc::new(Cell::new(false));
+        cx.update({
+            let fired = fired.clone();
+            |cx| cx.on_flags_ready(move |_, _| fired.set(true)).detach()
+        });
+
+        // Settings-triggered no-op touch must not fire on_flags_ready.
+        cx.update(|cx| cx.update_default_global::<FeatureFlagStore, _>(|_, _| {}));
+        cx.run_until_parked();
+        assert!(!fired.get());
+
+        // Server flags arrive — now it should fire.
+        cx.update(|cx| cx.update_flags(true, vec![]));
+        cx.run_until_parked();
+        assert!(fired.get());
+    }
 }