diff --git a/config.toml b/config.toml index b6dce8d2d..877c6a6ad 100644 --- a/config.toml +++ b/config.toml @@ -52,9 +52,32 @@ special-org-members = [ "rust-log-analyzer", "rust-timer", "rustbot", - # Ferrous systems employees who manage the github sponsors # for rust-analyzer. "MissHolmes", "skade", ] + +members-without-zulip-id = [ + "arshiamufti", + "bnchi", + "brotzeit", + "celaus", + "chris-morgan", + "codesections", + "da-x", + "ecstatic-morse", + "gdr-at-ms", + "kennykerr", + "mathk", + "nico-abram", + "opeolluwa", + "patchfx", + "pvdrz", + "reitermarkus", + "ryankurte", + "Stammark", + "therealprof", + "U007D", + "zeenix" +] diff --git a/docs/toml-schema.md b/docs/toml-schema.md index 00b10e0b8..fce4ae222 100644 --- a/docs/toml-schema.md +++ b/docs/toml-schema.md @@ -9,7 +9,7 @@ The file structure is this: name = "John Doe" # Display name of the person for the website (required) github = "johndoe" # GitHub username of the person (required) github-id = 123456 # GitHub ID of the person (required) -zulip-id = 123456 # Zulip ID of the person (optional) +zulip-id = 123456 # Zulip ID of the person (required) discord-id = 123456 # Discord ID of the person (optional) # You can also set `email = false` to explicitly disable the email for the user. # This will, for example, avoid adding the person to the mailing lists. diff --git a/src/schema.rs b/src/schema.rs index 399034c00..c9e983546 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -15,6 +15,7 @@ pub(crate) struct Config { permissions_bools: HashSet, // Use a BTreeSet for consistent ordering in tests special_org_members: BTreeSet, + members_without_zulip_id: BTreeSet, } impl Config { @@ -41,6 +42,10 @@ impl Config { pub(crate) fn special_org_members(&self) -> &BTreeSet { &self.special_org_members } + + pub(crate) fn members_without_zulip_id(&self) -> &BTreeSet { + &self.members_without_zulip_id + } } // This is an enum to allow two kinds of values for the email field: diff --git a/src/validate.rs b/src/validate.rs index 5b38dd9aa..3272042a1 100644 --- a/src/validate.rs +++ b/src/validate.rs @@ -49,6 +49,8 @@ static CHECKS: &[Check)>] = checks![ validate_zulip_group_extra_people, validate_unique_zulip_streams, validate_unique_zulip_user_ids, + validate_present_zulip_id, + validate_zulip_id_allowlist, validate_zulip_stream_ids, validate_zulip_stream_extra_people, validate_repos, @@ -715,6 +717,53 @@ fn validate_unique_zulip_user_ids(data: &Data, errors: &mut Vec) { }) } +/// Ensure that every active member except a few remaining people on an allowlist have a Zulip ID. +fn validate_present_zulip_id(data: &Data, errors: &mut Vec) { + wrapper( + data.active_members().unwrap().iter(), + errors, + |person, _| { + let person = data.person(person).expect("Person not found"); + if person.zulip_id().is_none() + && !data + .config() + .members_without_zulip_id() + .contains(person.github()) + { + return Err(anyhow::anyhow!( + "User {} does not have a Zulip ID", + person.github() + )); + } + Ok(()) + }, + ) +} + +/// Ensure people in the missing Zulip ID allowlist do not actually have Zulip ID configured. +fn validate_zulip_id_allowlist(data: &Data, errors: &mut Vec) { + // Sort for deterministic output + let mut members = data + .config() + .members_without_zulip_id() + .iter() + .collect::>(); + members.sort(); + for member in members { + let Some(person) = data.person(member) else { + errors.push(format!( + "Person {member} in Zulip ID allowlist does not exist" + )); + continue; + }; + if person.zulip_id().is_some() { + errors.push(format!( + "Person {member} in Zulip ID allowlist has Zulip ID configured. Please remove them from the `members-without-zulip-id` list in `config.toml`" + )); + } + } +} + /// 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| { @@ -731,15 +780,17 @@ fn validate_zulip_group_ids(data: &Data, errors: &mut Vec) { // Okay, have zulip IDs. } ZulipMember::MemberWithoutId { github } => { - // Bad, only github handle, no zulip ID, even though included in zulip user - // group - bail!( - "person `{}` in '{}' is a member of a Zulip user group '{}' but has no \ - Zulip id", - github, - team.name(), - group.name() - ); + if !data.config().members_without_zulip_id().contains(github) { + // Bad, only github handle, no zulip ID, even though included in zulip user + // group + bail!( + "person `{}` in '{}' is a member of a Zulip user group '{}' but has no \ + Zulip id", + github, + team.name(), + group.name() + ); + } } } Ok(()) @@ -764,11 +815,13 @@ fn validate_zulip_stream_ids(data: &Data, errors: &mut Vec) { match member { ZulipMember::MemberWithId { .. } | ZulipMember::JustId(_) => {} ZulipMember::MemberWithoutId { github } => { - bail!( - "person `{github}` is a member of a Zulip stream `{}` defined in team `{}`, but has no Zulip id", - stream.name(), - team.name() - ); + if !data.config().members_without_zulip_id().contains(github) { + bail!( + "person `{github}` is a member of a Zulip stream `{}` defined in team `{}`, but has no Zulip id", + stream.name(), + team.name() + ); + } } } Ok(()) diff --git a/teams/all.toml b/teams/all.toml index ad8d49683..e0358554d 100644 --- a/teams/all.toml +++ b/teams/all.toml @@ -31,26 +31,7 @@ extra-emails = [ [[zulip-groups]] name = "T-all" -# Exclude the following people from the Zulip group for grandfathering purposes, -# where previously we didn't require all project team members to have Zulip IDs -# (since there wasn't a `T-all` Zulip group previously). -excluded-people = [ - "U007D", - "andrewpollack", - "bnchi", - "celaus", - "opeolluwa", -] # Private channel with all team members, so that we can have an easy way of reaching them. [[zulip-streams]] name = "all/private" -# Exclude the following people from the Zulip stream for grandfathering purposes, -# where previously we didn't require all project team members to have Zulip IDs. -excluded-people = [ - "U007D", - "andrewpollack", - "bnchi", - "celaus", - "opeolluwa", -] diff --git a/tests/static-api/_expected/v1/zulip-map.json b/tests/static-api/_expected/v1/zulip-map.json index 8dbaf6680..fd8ed9ca1 100644 --- 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, + "6": 6, "1234": 0, "4321": 0 } diff --git a/tests/static-api/config.toml b/tests/static-api/config.toml index e93673baa..f8d46930b 100644 --- a/tests/static-api/config.toml +++ b/tests/static-api/config.toml @@ -22,3 +22,9 @@ permissions-bools = [ special-org-members = [ "test-bot", ] + +members-without-zulip-id = [ + "test-admin", + "user-3", + "user-4" +] diff --git a/tests/static-api/people/user-6.toml b/tests/static-api/people/user-6.toml index 51bdbe598..0f33eb42f 100644 --- a/tests/static-api/people/user-6.toml +++ b/tests/static-api/people/user-6.toml @@ -2,3 +2,4 @@ name = 'Sixth user' github = 'user-6' github-id = 6 email = "user6@example.com" +zulip-id = 6