From 46ca96bcc6759dea0ae480c7a6952b5b1bdbd5d5 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 4 Nov 2024 13:55:32 +0000 Subject: [PATCH 1/3] zulip: sync stream membership --- docs/toml-schema.md | 31 ++++++ rust_team_data/src/v1.rs | 20 ++++ src/data.rs | 12 ++- src/schema.rs | 95 +++++++++++++++++++ src/static_api.rs | 33 +++++++ .../_expected/v1/zulip-streams.json | 15 +++ tests/static-api/teams/foo.toml | 3 + 7 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 tests/static-api/_expected/v1/zulip-streams.json diff --git a/docs/toml-schema.md b/docs/toml-schema.md index 520a4475e..dc670ff3b 100644 --- a/docs/toml-schema.md +++ b/docs/toml-schema.md @@ -209,6 +209,37 @@ excluded-people = [ "rylev", ] +# Define the Zulip streams used by the team +# It's optional, and there can be more than one. +# +# This will remove anyone who isn't in the team from the stream +# so it should only be used for private streams at the moment. +[[zulip-streams]] +# The name of the Zulip stream (required) +name = "t-overlords/private" +# This can be set to false to avoid including all the team members in the stream +# It's useful if you want to create the stream with a different set of members +# It's optional, and the default is `true`. +include-team-members = true +# Include the following extra people in the Zulip stream. Their email address +# or Zulip id will be fetched from their TOML in people/ (optional). +extra-people = [ + "alexcrichton", +] +# Include the following Zulip ids in the Zulip stream (optional). +extra-zulip-ids = [ + 1234 +] +# Include all the members of the following teams in the Zulip stream +# (optional). +extra-teams = [ + "bots-nursery", +] +# Exclude the following people in the Zulip stream (optional). +excluded-people = [ + "rylev", +] + # Roles to define in Discord. [[discord-roles]] # The name of the role. diff --git a/rust_team_data/src/v1.rs b/rust_team_data/src/v1.rs index 384d82e50..254c40c59 100644 --- a/rust_team_data/src/v1.rs +++ b/rust_team_data/src/v1.rs @@ -126,6 +126,26 @@ pub struct ZulipGroups { pub groups: IndexMap, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct ZulipStream { + pub name: String, + pub members: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "snake_case")] +pub enum ZulipStreamMember { + // TODO(rylev): this variant can be removed once + // it is verified that no one is relying on it + Email(String), + Id(u64), +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct ZulipStreams { + pub streams: IndexMap, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct Permission { pub people: Vec, diff --git a/src/data.rs b/src/data.rs index 64ad2aab3..b305daa3f 100644 --- a/src/data.rs +++ b/src/data.rs @@ -1,4 +1,4 @@ -use crate::schema::{Config, List, Person, Repo, Team, ZulipGroup}; +use crate::schema::{Config, List, Person, Repo, Team, ZulipGroup, ZulipStream}; use anyhow::{bail, Context as _, Error}; use serde::de::DeserializeOwned; use std::collections::{HashMap, HashSet}; @@ -140,6 +140,16 @@ impl Data { Ok(groups) } + pub(crate) fn zulip_streams(&self) -> Result, Error> { + let mut streams = HashMap::new(); + for team in self.teams() { + for stream in team.zulip_streams(self)? { + streams.insert(stream.name().to_string(), stream); + } + } + Ok(streams) + } + pub(crate) fn team(&self, name: &str) -> Option<&Team> { self.teams.get(name) } diff --git a/src/schema.rs b/src/schema.rs index be2a524da..26f834f13 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -179,6 +179,8 @@ pub(crate) struct Team { lists: Vec, #[serde(default)] zulip_groups: Vec, + #[serde(default)] + zulip_streams: Vec, discord_roles: Option>, } @@ -421,6 +423,60 @@ impl Team { Ok(groups) } + pub(crate) fn raw_zulip_streams(&self) -> &[RawZulipStream] { + &self.zulip_streams + } + + pub(crate) fn zulip_streams(&self, data: &Data) -> Result, Error> { + let mut streams = Vec::new(); + let zulip_streams = self.raw_zulip_streams(); + + for raw_stream in zulip_streams { + let mut stream = ZulipStream { + name: raw_stream.name.clone(), + members: Vec::new(), + }; + + let mut members = if raw_stream.include_team_members { + self.members(data)? + } else { + HashSet::new() + }; + for person in &raw_stream.extra_people { + members.insert(person.as_str()); + } + for team in &raw_stream.extra_teams { + let team = data + .team(team) + .ok_or_else(|| format_err!("team {} is missing", team))?; + members.extend(team.members(data)?); + } + for excluded in &raw_stream.excluded_people { + if !members.remove(excluded.as_str()) { + bail!("'{excluded}' was specifically excluded from the Zulip stream '{}' but they were already not included", raw_stream.name); + } + } + + for member in members.iter() { + let member = data.person(member).ok_or_else(|| { + format_err!("{} does not have a person configuration", member) + })?; + let member = match (member.github.clone(), member.zulip_id) { + (github, Some(zulip_id)) => { + ZulipStreamMember::MemberWithId { github, zulip_id } + } + (github, _) => ZulipStreamMember::MemberWithoutId { github }, + }; + stream.members.push(member); + } + for &extra in &raw_stream.extra_zulip_ids { + stream.members.push(ZulipStreamMember::JustId(extra)); + } + streams.push(stream); + } + Ok(streams) + } + pub(crate) fn permissions(&self) -> &Permissions { &self.permissions } @@ -691,6 +747,22 @@ pub(crate) struct RawZulipGroup { pub(crate) excluded_people: Vec, } +#[derive(serde_derive::Deserialize, Debug)] +#[serde(rename_all = "kebab-case", deny_unknown_fields)] +pub(crate) struct RawZulipStream { + pub(crate) name: String, + #[serde(default = "default_true")] + pub(crate) include_team_members: bool, + #[serde(default)] + pub(crate) extra_people: Vec, + #[serde(default)] + pub(crate) extra_zulip_ids: Vec, + #[serde(default)] + pub(crate) extra_teams: Vec, + #[serde(default)] + pub(crate) excluded_people: Vec, +} + #[derive(Debug)] pub(crate) struct List { address: String, @@ -736,6 +808,29 @@ pub(crate) enum ZulipGroupMember { MemberWithoutId { github: String }, } +#[derive(Debug)] +pub(crate) struct ZulipStream { + name: String, + members: Vec, +} + +impl ZulipStream { + pub(crate) fn name(&self) -> &str { + &self.name + } + + pub(crate) fn members(&self) -> &[ZulipStreamMember] { + &self.members + } +} + +#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] +pub(crate) enum ZulipStreamMember { + MemberWithId { github: String, zulip_id: u64 }, + JustId(u64), + MemberWithoutId { github: String }, +} + fn default_true() -> bool { true } diff --git a/src/static_api.rs b/src/static_api.rs index e71d93363..8d0d5ea67 100644 --- a/src/static_api.rs +++ b/src/static_api.rs @@ -1,6 +1,7 @@ use crate::data::Data; use crate::schema::{ Bot, Email, MergeBot, Permissions, RepoPermission, TeamKind, ZulipGroupMember, + ZulipStreamMember, }; use anyhow::{ensure, Context as _, Error}; use indexmap::IndexMap; @@ -30,6 +31,7 @@ impl<'a> Generator<'a> { self.generate_repos()?; self.generate_lists()?; self.generate_zulip_groups()?; + self.generate_zulip_streams()?; self.generate_permissions()?; self.generate_rfcbot()?; self.generate_zulip_map()?; @@ -303,6 +305,37 @@ impl<'a> Generator<'a> { Ok(()) } + fn generate_zulip_streams(&self) -> Result<(), Error> { + let mut streams = IndexMap::new(); + + for stream in self.data.zulip_streams()?.values() { + let mut members = stream.members().to_vec(); + members.sort(); + streams.insert( + stream.name().to_string(), + v1::ZulipStream { + name: stream.name().to_string(), + members: members + .into_iter() + .filter_map(|m| match m { + ZulipStreamMember::MemberWithId { zulip_id, .. } => { + Some(v1::ZulipStreamMember::Id(zulip_id)) + } + ZulipStreamMember::JustId(zulip_id) => { + Some(v1::ZulipStreamMember::Id(zulip_id)) + } + ZulipStreamMember::MemberWithoutId { .. } => None, + }) + .collect(), + }, + ); + } + + streams.sort_keys(); + self.add("v1/zulip-streams.json", &v1::ZulipStreams { streams })?; + Ok(()) + } + fn generate_permissions(&self) -> Result<(), Error> { for perm in &Permissions::available(self.data.config()) { let allowed = crate::permissions::allowed_people(self.data, perm)?; diff --git a/tests/static-api/_expected/v1/zulip-streams.json b/tests/static-api/_expected/v1/zulip-streams.json new file mode 100644 index 000000000..f87ddad63 --- /dev/null +++ b/tests/static-api/_expected/v1/zulip-streams.json @@ -0,0 +1,15 @@ +{ + "streams": { + "t-foo/private": { + "name": "t-foo/private", + "members": [ + { + "id": 1234 + }, + { + "id": 4321 + } + ] + } + } +} \ No newline at end of file diff --git a/tests/static-api/teams/foo.toml b/tests/static-api/teams/foo.toml index f2c1f7037..98b2d0bf7 100644 --- a/tests/static-api/teams/foo.toml +++ b/tests/static-api/teams/foo.toml @@ -49,3 +49,6 @@ extra-teams = ["wg-test"] [[zulip-groups]] name = "T-foo" + +[[zulip-streams]] +name = "t-foo/private" From ad5a1ec316dd6704127c6cca0c65e7cac8d3d273 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 4 Nov 2024 13:55:43 +0000 Subject: [PATCH 2/3] compiler: sync private stream --- teams/compiler.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/teams/compiler.toml b/teams/compiler.toml index dcb148609..c534a64b9 100644 --- a/teams/compiler.toml +++ b/teams/compiler.toml @@ -128,3 +128,6 @@ extra-people = [ "spastorino", "tmiasko" ] + +[[zulip-streams]] +name = "t-compiler/contrib-private" From 87644dc9c7c9109ebc0e8acb40095b9ec8e49ec1 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 11 Nov 2024 15:04:20 +0000 Subject: [PATCH 3/3] zulip: reduce code duplication --- src/schema.rs | 211 +++++++++++++++++++++------------------------- src/static_api.rs | 17 ++-- src/validate.rs | 94 ++++++++++++++++++--- 3 files changed, 183 insertions(+), 139 deletions(-) diff --git a/src/schema.rs b/src/schema.rs index 26f834f13..34ab63598 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -370,6 +370,51 @@ impl Team { Ok(lists) } + /// `on_exclude_not_included` is a function that is returned when an excluded member + /// wasn't included. + fn expand_zulip_membership( + &self, + data: &Data, + common: &RawZulipCommon, + on_exclude_not_included: impl Fn(&str) -> Error, + ) -> Result, Error> { + let mut members = if common.include_team_members { + self.members(data)? + } else { + HashSet::new() + }; + for person in &common.extra_people { + members.insert(person.as_str()); + } + for team in &common.extra_teams { + let team = data + .team(team) + .ok_or_else(|| format_err!("team {} is missing", team))?; + members.extend(team.members(data)?); + } + for excluded in &common.excluded_people { + if !members.remove(excluded.as_str()) { + return Err(on_exclude_not_included(excluded)); + } + } + + let mut final_members = Vec::new(); + for member in members.iter() { + let member = data + .person(member) + .ok_or_else(|| format_err!("{} does not have a person configuration", member))?; + let member = match (member.github.clone(), member.zulip_id) { + (github, Some(zulip_id)) => ZulipMember::MemberWithId { github, zulip_id }, + (github, _) => ZulipMember::MemberWithoutId { github }, + }; + final_members.push(member); + } + for &extra in &common.extra_zulip_ids { + final_members.push(ZulipMember::JustId(extra)); + } + Ok(final_members) + } + pub(crate) fn raw_zulip_groups(&self) -> &[RawZulipGroup] { &self.zulip_groups } @@ -379,46 +424,17 @@ impl Team { let zulip_groups = &self.zulip_groups; for raw_group in zulip_groups { - let mut group = ZulipGroup { - name: raw_group.name.clone(), - includes_team_members: raw_group.include_team_members, - members: Vec::new(), - }; - - let mut members = if raw_group.include_team_members { - self.members(data)? - } else { - HashSet::new() - }; - for person in &raw_group.extra_people { - members.insert(person.as_str()); - } - for team in &raw_group.extra_teams { - let team = data - .team(team) - .ok_or_else(|| format_err!("team {} is missing", team))?; - members.extend(team.members(data)?); - } - for excluded in &raw_group.excluded_people { - if !members.remove(excluded.as_str()) { - bail!("'{excluded}' was specifically excluded from the Zulip group '{}' but they were already not included", raw_group.name); - } - } - - for member in members.iter() { - let member = data.person(member).ok_or_else(|| { - format_err!("{} does not have a person configuration", member) - })?; - 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::JustId(extra)); - } - groups.push(group); + groups.push(ZulipGroup(ZulipCommon { + name: raw_group.common.name.clone(), + includes_team_members: raw_group.common.include_team_members, + members: self.expand_zulip_membership( + data, + &raw_group.common, + |excluded| { + format_err!("'{excluded}' was specifically excluded from the Zulip group '{}' but they were already not included", raw_group.common.name) + }, + )?, + })); } Ok(groups) } @@ -432,47 +448,17 @@ impl Team { let zulip_streams = self.raw_zulip_streams(); for raw_stream in zulip_streams { - let mut stream = ZulipStream { - name: raw_stream.name.clone(), - members: Vec::new(), - }; - - let mut members = if raw_stream.include_team_members { - self.members(data)? - } else { - HashSet::new() - }; - for person in &raw_stream.extra_people { - members.insert(person.as_str()); - } - for team in &raw_stream.extra_teams { - let team = data - .team(team) - .ok_or_else(|| format_err!("team {} is missing", team))?; - members.extend(team.members(data)?); - } - for excluded in &raw_stream.excluded_people { - if !members.remove(excluded.as_str()) { - bail!("'{excluded}' was specifically excluded from the Zulip stream '{}' but they were already not included", raw_stream.name); - } - } - - for member in members.iter() { - let member = data.person(member).ok_or_else(|| { - format_err!("{} does not have a person configuration", member) - })?; - let member = match (member.github.clone(), member.zulip_id) { - (github, Some(zulip_id)) => { - ZulipStreamMember::MemberWithId { github, zulip_id } - } - (github, _) => ZulipStreamMember::MemberWithoutId { github }, - }; - stream.members.push(member); - } - for &extra in &raw_stream.extra_zulip_ids { - stream.members.push(ZulipStreamMember::JustId(extra)); - } - streams.push(stream); + streams.push(ZulipStream(ZulipCommon { + name: raw_stream.common.name.clone(), + includes_team_members: raw_stream.common.include_team_members, + members: self.expand_zulip_membership( + data, + &raw_stream.common, + |excluded| { + format_err!("'{excluded}' was specifically excluded from the Zulip stream '{}' but they were already not included", raw_stream.common.name) + }, + )?, + })); } Ok(streams) } @@ -733,7 +719,7 @@ pub(crate) struct TeamList { #[derive(serde_derive::Deserialize, Debug)] #[serde(rename_all = "kebab-case", deny_unknown_fields)] -pub(crate) struct RawZulipGroup { +pub(crate) struct RawZulipCommon { pub(crate) name: String, #[serde(default = "default_true")] pub(crate) include_team_members: bool, @@ -747,20 +733,18 @@ pub(crate) struct RawZulipGroup { pub(crate) excluded_people: Vec, } +#[derive(serde_derive::Deserialize, Debug)] +#[serde(rename_all = "kebab-case", deny_unknown_fields)] +pub(crate) struct RawZulipGroup { + #[serde(flatten)] + pub(crate) common: RawZulipCommon, +} + #[derive(serde_derive::Deserialize, Debug)] #[serde(rename_all = "kebab-case", deny_unknown_fields)] pub(crate) struct RawZulipStream { - pub(crate) name: String, - #[serde(default = "default_true")] - pub(crate) include_team_members: bool, - #[serde(default)] - pub(crate) extra_people: Vec, - #[serde(default)] - pub(crate) extra_zulip_ids: Vec, - #[serde(default)] - pub(crate) extra_teams: Vec, - #[serde(default)] - pub(crate) excluded_people: Vec, + #[serde(flatten)] + pub(crate) common: RawZulipCommon, } #[derive(Debug)] @@ -780,52 +764,49 @@ impl List { } #[derive(Debug)] -pub(crate) struct ZulipGroup { +pub(crate) struct ZulipCommon { name: String, includes_team_members: bool, - members: Vec, + members: Vec, } -impl ZulipGroup { +impl ZulipCommon { pub(crate) fn name(&self) -> &str { &self.name } - /// Whether the group includes the members of the team its associated + /// Whether the group/stream includes the members of the associated team? pub(crate) fn includes_team_members(&self) -> bool { self.includes_team_members } - pub(crate) fn members(&self) -> &[ZulipGroupMember] { + pub(crate) fn members(&self) -> &[ZulipMember] { &self.members } } -#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] -pub(crate) enum ZulipGroupMember { - MemberWithId { github: String, zulip_id: u64 }, - JustId(u64), - MemberWithoutId { github: String }, -} - #[derive(Debug)] -pub(crate) struct ZulipStream { - name: String, - members: Vec, -} +pub(crate) struct ZulipGroup(ZulipCommon); -impl ZulipStream { - pub(crate) fn name(&self) -> &str { - &self.name +impl std::ops::Deref for ZulipGroup { + type Target = ZulipCommon; + fn deref(&self) -> &Self::Target { + &self.0 } +} - pub(crate) fn members(&self) -> &[ZulipStreamMember] { - &self.members +#[derive(Debug)] +pub(crate) struct ZulipStream(ZulipCommon); + +impl std::ops::Deref for ZulipStream { + type Target = ZulipCommon; + fn deref(&self) -> &Self::Target { + &self.0 } } #[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)] -pub(crate) enum ZulipStreamMember { +pub(crate) enum ZulipMember { MemberWithId { github: String, zulip_id: u64 }, JustId(u64), MemberWithoutId { github: String }, diff --git a/src/static_api.rs b/src/static_api.rs index 8d0d5ea67..7e3ad7b84 100644 --- a/src/static_api.rs +++ b/src/static_api.rs @@ -1,8 +1,5 @@ use crate::data::Data; -use crate::schema::{ - Bot, Email, MergeBot, Permissions, RepoPermission, TeamKind, ZulipGroupMember, - ZulipStreamMember, -}; +use crate::schema::{Bot, Email, MergeBot, Permissions, RepoPermission, TeamKind, ZulipMember}; use anyhow::{ensure, Context as _, Error}; use indexmap::IndexMap; use log::info; @@ -287,13 +284,13 @@ impl<'a> Generator<'a> { members: members .into_iter() .filter_map(|m| match m { - ZulipGroupMember::MemberWithId { zulip_id, .. } => { + ZulipMember::MemberWithId { zulip_id, .. } => { Some(v1::ZulipGroupMember::Id(zulip_id)) } - ZulipGroupMember::JustId(zulip_id) => { + ZulipMember::JustId(zulip_id) => { Some(v1::ZulipGroupMember::Id(zulip_id)) } - ZulipGroupMember::MemberWithoutId { .. } => None, + ZulipMember::MemberWithoutId { .. } => None, }) .collect(), }, @@ -318,13 +315,13 @@ impl<'a> Generator<'a> { members: members .into_iter() .filter_map(|m| match m { - ZulipStreamMember::MemberWithId { zulip_id, .. } => { + ZulipMember::MemberWithId { zulip_id, .. } => { Some(v1::ZulipStreamMember::Id(zulip_id)) } - ZulipStreamMember::JustId(zulip_id) => { + ZulipMember::JustId(zulip_id) => { Some(v1::ZulipStreamMember::Id(zulip_id)) } - ZulipStreamMember::MemberWithoutId { .. } => None, + ZulipMember::MemberWithoutId { .. } => None, }) .collect(), }, diff --git a/src/validate.rs b/src/validate.rs index bf30253cd..de8d43fc8 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -1,8 +1,6 @@ use crate::data::Data; use crate::github::GitHubApi; -use crate::schema::{ - Bot, Email, MergeBot, Permissions, Team, TeamKind, TeamPeople, ZulipGroupMember, -}; +use crate::schema::{Bot, Email, MergeBot, Permissions, Team, TeamKind, TeamPeople, ZulipMember}; use crate::zulip::ZulipApi; use anyhow::{bail, Error}; use log::{error, warn}; @@ -47,6 +45,9 @@ static CHECKS: &[Check)>] = checks![ validate_unique_zulip_groups, validate_zulip_group_ids, validate_zulip_group_extra_people, + validate_unique_zulip_streams, + validate_zulip_stream_ids, + validate_zulip_stream_extra_people, validate_repos, validate_branch_protections, validate_member_roles, @@ -335,9 +336,10 @@ fn validate_inactive_members(data: &Data, errors: &mut Vec) { .values() .flat_map(|z| z.members()) .filter_map(|m| match m { - ZulipGroupMember::MemberWithId { github, .. } - | ZulipGroupMember::MemberWithoutId { github } => Some(github.as_str()), - ZulipGroupMember::JustId(_) => None, + ZulipMember::MemberWithId { github, .. } | ZulipMember::MemberWithoutId { github } => { + Some(github.as_str()) + } + ZulipMember::JustId(_) => None, }) .collect::>(); wrapper( @@ -688,15 +690,13 @@ fn validate_zulip_users(data: &Data, zulip: &ZulipApi, errors: &mut Vec) .members() .iter() .filter_map(|m| match m { - ZulipGroupMember::MemberWithId { github, zulip_id } - if !by_id.contains(zulip_id) => - { + ZulipMember::MemberWithId { github, zulip_id } if !by_id.contains(zulip_id) => { Some(github.clone()) } - ZulipGroupMember::JustId(zulip_id) if !by_id.contains(zulip_id) => { + ZulipMember::JustId(zulip_id) if !by_id.contains(zulip_id) => { Some(format!("ID: {zulip_id}")) } - ZulipGroupMember::MemberWithoutId { github } => Some(github.clone()), + ZulipMember::MemberWithoutId { github } => Some(github.clone()), _ => None, }) .collect::>(); @@ -711,7 +711,7 @@ fn validate_zulip_users(data: &Data, zulip: &ZulipApi, errors: &mut Vec) }) } -/// Ensure every member of a team that has a Zulip group either has a Zulip id +/// Ensure team members in Zulip groups have a Zulip id fn validate_zulip_group_ids(data: &Data, errors: &mut Vec) { wrapper(data.teams(), errors, |team, errors| { let groups = team.zulip_groups(data)?; @@ -735,6 +735,30 @@ fn validate_zulip_group_ids(data: &Data, errors: &mut Vec) { }); } +/// Ensure team members in Zulip streams have a Zulip id +fn validate_zulip_stream_ids(data: &Data, errors: &mut Vec) { + wrapper(data.teams(), errors, |team, errors| { + let streams = team.zulip_streams(data)?; + // Returns if stream is empty or all the streams don't include the team members + if streams.is_empty() || streams.iter().all(|s| !s.includes_team_members()) { + return Ok(()); + } + wrapper(team.members(data)?.iter(), errors, |member, _| { + if let Some(member) = data.person(member) { + if member.zulip_id().is_none() { + bail!( + "person `{}` in '{}' is a member of a Zulip stream but has no Zulip id", + member.github(), + team.name() + ); + } + } + Ok(()) + }); + Ok(()) + }); +} + /// Ensure there is at most one definition for any given Zulip group fn validate_unique_zulip_groups(data: &Data, errors: &mut Vec) { let mut groups = HashMap::new(); @@ -758,16 +782,58 @@ fn validate_unique_zulip_groups(data: &Data, errors: &mut Vec) { }); } +/// Ensure there is at most one definition for any given Zulip group +fn validate_unique_zulip_streams(data: &Data, errors: &mut Vec) { + let mut streams = HashMap::new(); + wrapper(data.teams(), errors, |team, errors| { + wrapper( + team.zulip_streams(data).iter().flatten(), + errors, + |stream, _| { + if let Some(other_team) = streams.insert(stream.name().to_owned(), team.name()) { + bail!( + "the Zulip stream `{}` is defined in both `{}` and `{}` team definitions", + stream.name(), + team.name(), + other_team + ); + } + Ok(()) + }, + ); + Ok(()) + }); +} + /// Ensure members of extra-people in a Zulip user group are real people fn validate_zulip_group_extra_people(data: &Data, errors: &mut Vec) { wrapper(data.teams(), errors, |team, errors| { wrapper(team.raw_zulip_groups().iter(), errors, |group, _| { - for person in &group.extra_people { + for person in &group.common.extra_people { if data.person(person).is_none() { bail!( "person `{}` does not exist (in Zulip group `{}`)", person, - group.name + group.common.name + ); + } + } + Ok(()) + }); + Ok(()) + }); +} + +/// Ensure members of extra-people in a Zulip user group are real people +fn validate_zulip_stream_extra_people(data: &Data, errors: &mut Vec) { + wrapper(data.teams(), errors, |team, errors| { + wrapper(team.raw_zulip_streams().iter(), errors, |stream, _| { + for person in &stream.common.extra_people { + if data.person(person).is_none() { + bail!( + "person `{}` does not exist (in Zulip stream `{}`)", + person, + stream.common.name ); } }