Merge pull request #965 from rust-lang/require-zulip-id

rylev · web-flow · commit 03de88a2b11e · 2023-03-28T15:34:54.000+02:00
Require all people in Zulip user groups to have a Zulip id in their config
diff --git a/people/Aaron1011.toml b/people/Aaron1011.toml
@@ -2,3 +2,4 @@ name = 'Aaron Hill'
 email = "aa1ronham@gmail.com"
 github = 'Aaron1011'
 github-id = 1408859
+zulip-id = 125294
diff --git a/people/BurntSushi.toml b/people/BurntSushi.toml
@@ -3,6 +3,7 @@ github = "BurntSushi"
 github-id = 456674
 irc = "burntsushi"
 email = "jamslam@gmail.com"
+zulip-id = 222471
 
 [permissions]
 bors.regex.review = true
diff --git a/people/ChrisDenton.toml b/people/ChrisDenton.toml
@@ -2,3 +2,4 @@ name = 'Chris Denton'
 github = 'ChrisDenton'
 github-id = 4459874
 email = 'christophersdenton@gmail.com'
+zulip-id = 260325
diff --git a/people/JohnCSimon.toml b/people/JohnCSimon.toml
@@ -2,3 +2,4 @@ name = "John Simon"
 github = "JohnCSimon"
 github-id = 1977159
 email = "johncsimon@gmail.com"
+zulip-id = 260190
diff --git a/people/Muirrum.toml b/people/Muirrum.toml
@@ -2,3 +2,4 @@ name = 'Cara Salter'
 github = 'Muirrum'
 github-id = 8702646
 email = "cara@devcara.com"
+zulip-id = 310810
diff --git a/people/Shnatsel.toml b/people/Shnatsel.toml
@@ -2,3 +2,4 @@ name = "Sergey Davidoff"
 email = "shnatsel@gmail.com"
 github = "Shnatsel"
 github-id = 291257
+zulip-id = 127617
diff --git a/people/TaKO8Ki.toml b/people/TaKO8Ki.toml
@@ -2,3 +2,4 @@ name = 'Takayuki Maeda'
 github = 'TaKO8Ki'
 github-id = 41065217
 email = "takoyaki0316@gmail.com"
+zulip-id = 343125
diff --git a/people/alex.toml b/people/alex.toml
@@ -3,3 +3,4 @@ email = "alex.gaynor@gmail.com"
 github = "alex"
 github-id = 772
 irc = "Alex_Gaynor"
+zulip-id = 130046
diff --git a/people/alexcrichton.toml b/people/alexcrichton.toml
@@ -3,3 +3,4 @@ github = "alexcrichton"
 github-id = 64996
 irc = "acrichto"
 email = "alex@alexcrichton.com"
+zulip-id = 116015
diff --git a/people/alice-i-cecile.toml b/people/alice-i-cecile.toml
@@ -2,3 +2,4 @@ name = "Alice Cecile"
 github = "alice-i-cecile"
 github-id = 3579909
 email = "alice.i.cecile@gmail.com"
+zulip-id = 406353
diff --git a/people/aliemjay.toml b/people/aliemjay.toml
@@ -2,3 +2,4 @@ name = "Ali MJ Al-Nasrawy"
 github = "aliemjay"
 github-id = 28497461
 email = "alimjalnasrawy@gmail.com"
+zulip-id = 430838
diff --git a/people/aturon.toml b/people/aturon.toml
@@ -2,3 +2,4 @@ name = "Aaron Turon"
 github = "aturon"
 github-id = 709807
 email = "aturon@mozilla.com"
+zulip-id = 301292
diff --git a/people/bstrie.toml b/people/bstrie.toml
@@ -2,4 +2,4 @@ name = "Ben Striegel"
 github = "bstrie"
 github-id = 865233
 email = "ben.striegel@gmail.com"
-
+zulip-id = 256342
diff --git a/people/cramertj.toml b/people/cramertj.toml
@@ -3,3 +3,4 @@ github = "cramertj"
 github-id = 5963049
 email = "cramertaylorj@gmail.com"
 discord-id = 446773566478745611
+zulip-id = 127859
diff --git a/people/crlf0710.toml b/people/crlf0710.toml
@@ -2,3 +2,4 @@ name = 'Charles Lew'
 github = 'crlf0710'
 github-id = 451806
 email = "crlf0710@gmail.com"
+zulip-id = 116458
diff --git a/people/edmilsonefs.toml b/people/edmilsonefs.toml
@@ -2,3 +2,4 @@ name = "Edmilson Ferreira da Silva"
 github = "edmilsonefs"
 github-id = 498938
 email = "edmilson.efs@gmail.com"
+zulip-id = 220075
diff --git a/people/estebank.toml b/people/estebank.toml
@@ -3,3 +3,4 @@ github = "estebank"
 github-id = 1606434
 email = "esteban@kuber.com.ar"
 discord-id = 449245991216873475
+zulip-id = 119031
diff --git a/people/guswynn.toml b/people/guswynn.toml
@@ -2,3 +2,4 @@ name = 'Gus Wynn'
 github = 'guswynn'
 github-id = 5404303
 email = 'guswynn@gmail.com'
+zulip-id = 257428
diff --git a/people/joelpalmer.toml b/people/joelpalmer.toml
@@ -2,3 +2,4 @@ name = "Joel Parmer"
 github = "joelpalmer"
 github-id = 8049061
 email = "joelpalmer@gmail.com"
+zulip-id = 223076
diff --git a/people/kennytm.toml b/people/kennytm.toml
@@ -3,3 +3,4 @@ github = "kennytm"
 github-id = 103023
 email = "kennytm@gmail.com"
 discord-id = 382886245069488129
+zulip-id = 125254
diff --git a/people/lcnr.toml b/people/lcnr.toml
@@ -2,3 +2,4 @@ name = 'lcnr'
 email = "rust@lcnr.de"
 github = 'lcnr'
 github-id = 29864074
+zulip-id = 216206
diff --git a/people/llogiq.toml b/people/llogiq.toml
@@ -2,3 +2,4 @@ name = "Andre Bogus"
 github = "llogiq"
 github-id = 4200835
 email = "bogusandre@gmail.com"
+zulip-id = 248599
diff --git a/people/m-ou-se.toml b/people/m-ou-se.toml
@@ -2,3 +2,4 @@ name = 'Mara Bos'
 github = 'm-ou-se'
 github-id = 783247
 email = 'm-ou.se@m-ou.se'
+zulip-id = 310399
diff --git a/people/matthiaskrgr.toml b/people/matthiaskrgr.toml
@@ -1,4 +1,5 @@
 name = "Matthias Krüger"
 github = "matthiaskrgr"
 github-id = 476013
-email = "matthias.krueger@famsik.de"
+email = "matthias.krueger@famsik.de"
+zulip-id = 217864
diff --git a/people/nellshamrell.toml b/people/nellshamrell.toml
@@ -3,3 +3,4 @@ github = "nellshamrell"
 github-id = 813007
 email = "nellshamrell@gmail.com"
 discord-id = 244302461718757376
+zulip-id = 225192
diff --git a/people/nnethercote.toml b/people/nnethercote.toml
@@ -2,3 +2,4 @@ name = "Nicholas Nethercote"
 github = "nnethercote"
 github-id = 1940286
 email = "n.nethercote@gmail.com"
+zulip-id = 120989
diff --git a/people/nrc.toml b/people/nrc.toml
@@ -2,6 +2,7 @@ name = "Nick Cameron"
 github = "nrc"
 github-id = 762626
 email = "nrc@ncameron.org"
+zulip-id = 256841
 
 [permissions]
 bors.rustup-rs.review = true
diff --git a/people/varkor.toml b/people/varkor.toml
@@ -2,3 +2,4 @@ name = "varkor"
 github = "varkor"
 github-id = 3943692
 email = "github@varkor.com"
+zulip-id = 121053
diff --git a/people/vertexclique.toml b/people/vertexclique.toml
@@ -1,4 +1,5 @@
 name = 'Mahmut Bulut'
 github = 'vertexclique'
 github-id = 578559
-email = "vertexclique@gmail.com"
+email = "vertexclique@gmail.com"
+zulip-id = 223879
diff --git a/people/workingjubilee.toml b/people/workingjubilee.toml
@@ -2,3 +2,4 @@ name = 'Jubilee'
 github = 'workingjubilee'
 github-id = 46493976
 email = 'workingjubilee@gmail.com'
+zulip-id = 281757
diff --git a/rust_team_data/src/v1.rs b/rust_team_data/src/v1.rs
@@ -103,6 +103,8 @@ pub struct ZulipGroup {
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "snake_case")]
 pub enum ZulipGroupMember {
+    // TODO(rylev): this variant can be removed once
+    // it is verified that noone is relying on it
     Email(String),
     Id(usize),
 }
diff --git a/src/check_synced.rs b/src/check_synced.rs
@@ -24,11 +24,6 @@ fn check_zulip(data: &Data) -> Result<(), failure::Error> {
         .map(|g| (g.name.clone(), g))
         .collect::<HashMap<_, _>>();
     let remote_users = zulip.get_users()?;
-    let email_to_zulip_id = remote_users
-        .iter()
-        .cloned()
-        .map(|u| (u.email, u.user_id))
-        .collect::<HashMap<_, _>>();
     let zulip_id_to_name = remote_users
         .into_iter()
         .map(|u| (u.user_id, u.name))
@@ -44,16 +39,10 @@ fn check_zulip(data: &Data) -> Result<(), failure::Error> {
                 let mut remote_members = rg.members.iter().collect::<HashSet<_>>();
                 for local_member in local_group.members() {
                     let i = match local_member {
-                        ZulipGroupMember::Id(i) => *i,
-                        ZulipGroupMember::Email(e) => match email_to_zulip_id.get(e) {
-                            Some(i) => *i,
-                            None => {
-                                error!("No user on Zulip uses the email '{e}'");
-                                continue;
-                            }
-                        },
-                        ZulipGroupMember::Missing => {
-                            error!("Member of Zulip user group '{}' does not have an email or Zulip id", local_group.name());
+                        ZulipGroupMember::MemberWithId { zulip_id, .. } => *zulip_id,
+                        ZulipGroupMember::JustId(zulip_id) => *zulip_id,
+                        ZulipGroupMember::MemberWithoutId { github } => {
+                            error!("Member '{github}' of Zulip user group '{}' does not have a Zulip id", local_group.name());
                             continue;
                         }
                     };
diff --git a/src/schema.rs b/src/schema.rs
@@ -369,15 +369,14 @@ impl Team {
                 let member = data.person(member).ok_or_else(|| {
                     err_msg(format!("{} does not have a person configuration", member))
                 })?;
-                let member = match (member.zulip_id, member.email()) {
-                    (Some(zulip_id), _) => ZulipGroupMember::Id(zulip_id),
-                    (_, Email::Present(email)) => ZulipGroupMember::Email(email.to_string()),
-                    _ => ZulipGroupMember::Missing,
+                let member = match (member.github.clone(), member.zulip_id) {
+                    (github, Some(zulip_id)) => ZulipGroupMember::MemberWithId { github, zulip_id },
+                    (github, _) => ZulipGroupMember::MemberWithoutId { github },
                 };
                 group.members.push(member);
             }
             for &extra in &raw_group.extra_zulip_ids {
-                group.members.push(ZulipGroupMember::Id(extra));
+                group.members.push(ZulipGroupMember::JustId(extra));
             }
             groups.push(group);
         }
@@ -654,9 +653,9 @@ impl ZulipGroup {
 
 #[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
 pub(crate) enum ZulipGroupMember {
-    Id(usize),
-    Email(String),
-    Missing,
+    MemberWithId { github: String, zulip_id: usize },
+    JustId(usize),
+    MemberWithoutId { github: String },
 }
 
 fn default_true() -> bool {
diff --git a/src/static_api.rs b/src/static_api.rs
@@ -237,9 +237,13 @@ impl<'a> Generator<'a> {
                     members: members
                         .into_iter()
                         .filter_map(|m| match m {
-                            ZulipGroupMember::Email(e) => Some(v1::ZulipGroupMember::Email(e)),
-                            ZulipGroupMember::Id(i) => Some(v1::ZulipGroupMember::Id(i)),
-                            ZulipGroupMember::Missing => None,
+                            ZulipGroupMember::MemberWithId { zulip_id, .. } => {
+                                Some(v1::ZulipGroupMember::Id(zulip_id))
+                            }
+                            ZulipGroupMember::JustId(zulip_id) => {
+                                Some(v1::ZulipGroupMember::Id(zulip_id))
+                            }
+                            ZulipGroupMember::MemberWithoutId { .. } => None,
                         })
                         .collect(),
                 },
diff --git a/src/validate.rs b/src/validate.rs
@@ -609,13 +609,10 @@ fn validate_discord_team_members_have_discord_ids(data: &Data, errors: &mut Vec<
     });
 }
 
-/// Ensure every member of a team that has a Zulip group either has a Zulip id or an email address
+/// Ensure every member of a team that has a Zulip group has a Zulip id
 fn validate_zulip_users(data: &Data, zulip: &ZulipApi, errors: &mut Vec<String>) {
-    let (by_id, by_email) = match zulip.get_users() {
-        Ok(u) => (
-            u.iter().map(|u| u.user_id).collect::<HashSet<_>>(),
-            u.into_iter().map(|u| u.email).collect::<HashSet<_>>(),
-        ),
+    let by_id = match zulip.get_users() {
+        Ok(u) => u.iter().map(|u| u.user_id).collect::<HashSet<_>>(),
         Err(err) => {
             errors.push(format!("couldn't verify Zulip users: {}", err));
             return;
@@ -633,8 +630,15 @@ fn validate_zulip_users(data: &Data, zulip: &ZulipApi, errors: &mut Vec<String>)
             .members()
             .iter()
             .filter_map(|m| match m {
-                ZulipGroupMember::Id(i) if !by_id.contains(i) => Some(format!("{}", i)),
-                ZulipGroupMember::Email(e) if !by_email.contains(e) => Some(e.clone()),
+                ZulipGroupMember::MemberWithId { github, zulip_id }
+                    if !by_id.contains(zulip_id) =>
+                {
+                    Some(github.clone())
+                }
+                ZulipGroupMember::JustId(zulip_id) if !by_id.contains(zulip_id) => {
+                    Some(format!("ID: {zulip_id}"))
+                }
+                ZulipGroupMember::MemberWithoutId { github } => Some(github.clone()),
                 _ => None,
             })
             .collect::<HashSet<_>>();
@@ -649,7 +653,7 @@ fn validate_zulip_users(data: &Data, zulip: &ZulipApi, errors: &mut Vec<String>)
     })
 }
 
-/// Ensure every member of a team that has a Zulip group either has a Zulip id or an email address
+/// Ensure every member of a team that has a Zulip group either has a Zulip id
 fn validate_zulip_group_ids(data: &Data, errors: &mut Vec<String>) {
     wrapper(data.teams(), errors, |team, errors| {
         let groups = team.zulip_groups(data)?;
@@ -659,11 +663,9 @@ fn validate_zulip_group_ids(data: &Data, errors: &mut Vec<String>) {
         }
         wrapper(team.members(data).iter(), errors, |member, _| {
             if let Some(member) = data.person(member) {
-                if member.zulip_id().is_none()
-                    && matches!(member.email(), Email::Missing | Email::Disabled)
-                {
+                if member.zulip_id().is_none() {
                     bail!(
-                        "person `{}` in '{}' is a member of a Zulip user group but has no Zulip id nor an enabled email address",
+                        "person `{}` in '{}' is a member of a Zulip user group but has no Zulip id",
                         member.github(),
                         team.name()
                     );
diff --git a/src/zulip.rs b/src/zulip.rs
@@ -98,8 +98,6 @@ struct ZulipUsers {
 /// A single Zulip user
 #[derive(Clone, Deserialize, PartialEq, Eq, Hash)]
 pub(crate) struct ZulipUser {
-    #[serde(rename = "delivery_email")]
-    pub(crate) email: String,
     pub(crate) user_id: usize,
     #[serde(rename = "full_name")]
     pub(crate) name: String,
diff --git a/tests/static-api/_expected/v1/zulip-groups.json b/tests/static-api/_expected/v1/zulip-groups.json
@@ -7,7 +7,7 @@
           "id": 1234
         },
         {
-          "email": "user1@example.com"
+          "id": 4321
         }
       ]
     }
diff --git a/tests/static-api/_expected/v1/zulip-map.json b/tests/static-api/_expected/v1/zulip-map.json
@@ -1,6 +1,7 @@
 {
   "users": {
     "2": 2,
-    "1234": 0
+    "1234": 0,
+    "4321": 0
   }
 }
diff --git a/tests/static-api/people/user-1.toml b/tests/static-api/people/user-1.toml
@@ -3,3 +3,4 @@ github = 'user-1'
 github-id = 0
 email = "user1@example.com"
 discord-id = 1
+zulip-id = 4321

-Original file line number
+Diff line change
@@ @@ -1,4 +1,5 @@ @@
 name = "Matthias Krüger"
 github = "matthiaskrgr"
 github-id = 476013
 -email = "[email protected]"
 +email = "[email protected]"
 +zulip-id = 217864
Original file line number	Diff line number	Diff line change
`@@ -103,6 +103,8 @@ pub struct ZulipGroup {`
`103`	`103`	`#[derive(Debug, Clone, Serialize, Deserialize)]`
`104`	`104`	`#[serde(rename_all = "snake_case")]`
`105`	`105`	`pub enum ZulipGroupMember {`
	`106`	`+ // TODO(rylev): this variant can be removed once`
	`107`	`+ // it is verified that noone is relying on it`
`106`	`108`	`Email(String),`
`107`	`109`	`Id(usize),`
`108`	`110`	`}`
Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@`
`7`	`7`	`"id": 1234`
`8`	`8`	`},`
`9`	`9`	`{`
`10`		`- "email": "[email protected]"`
	`10`	`+ "id": 4321`
`11`	`11`	`}`
`12`	`12`	`]`
`13`	`13`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,7 @@`
`1`	`1`	`{`
`2`	`2`	`"users": {`
`3`	`3`	`"2": 2,`
`4`		`- "1234": 0`
	`4`	`+ "1234": 0,`
	`5`	`+ "4321": 0`
`5`	`6`	`}`
`6`	`7`	`}`