Skip to content

Commit aa9a873

Browse files
committed
Complain if PRs don't reference regions correctly
We want to be able to rely on regions for filtering, e.g. when showing things in dashboards. Ideally we would have a source of truth of which region each trainee is in, but we don't, so in the mean time check that the title claims to be in some region. Hard-code the known regions for now - ideally an API will provide these in the future.
1 parent 4edd573 commit aa9a873

File tree

1 file changed

+53
-4
lines changed

1 file changed

+53
-4
lines changed

src/bin/pr-metadata-validator.rs

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::process::exit;
1+
use std::{collections::BTreeMap, process::exit};
22

33
use chrono::NaiveDate;
44
use indexmap::IndexMap;
@@ -39,6 +39,16 @@ async fn main() {
3939
}
4040
};
4141

42+
// TODO: Fetch this from classplanner or somewhere when we have access to a useful API.
43+
let known_region_aliases = KnownRegions(btreemap! {
44+
"Cape Town" => vec!["South Africa", "SouthAfrica", "ZA", "ZA Cape Town"],
45+
"Glasgow" => vec!["Scotland"],
46+
"London" => vec![],
47+
"North West" => vec!["NW", "Manchester"],
48+
"Sheffield" => vec![],
49+
"West Midlands" => vec!["WM", "WestMidlands", "West-Midlands", "Birmingham"],
50+
});
51+
4252
let github_token =
4353
std::env::var("GH_TOKEN").expect("GH_TOKEN wasn't set - must be set to a GitHub API token");
4454
let octocrab = octocrab_for_token(github_token).expect("Failed to get octocrab");
@@ -56,6 +66,7 @@ async fn main() {
5666
module_name.clone(),
5767
github_org_name.clone(),
5868
pr_number,
69+
&known_region_aliases,
5970
)
6071
.await
6172
.expect("Failed to validate PR");
@@ -79,6 +90,15 @@ async fn main() {
7990
.expect("Failed to create comment with validation error");
8091
exit(2);
8192
}
93+
ValidationResult::UnknownRegion => {
94+
eprintln!("Validation error: Could not find region in PR title");
95+
octocrab
96+
.issues(github_org_name, module_name.clone())
97+
.create_comment(pr_number, UNKNOWN_REGION_COMMENT)
98+
.await
99+
.expect("Failed to create comment with validation error");
100+
exit(2);
101+
}
82102
}
83103
}
84104

@@ -90,14 +110,19 @@ If this PR is not coursework, please add the NotCoursework label (and message on
90110

91111
const BAD_TITLE_COMMENT_PREFIX: &'static str = r#"Your PR's title isn't in the expected format.
92112
93-
Please check its title is in the correct format, and update it.
113+
Please check the expected title format, and update yours to match.
94114
95115
Reason: "#;
96116

117+
const UNKNOWN_REGION_COMMENT: &'static str = r#"Your PR's title didn't contain a known region.
118+
119+
Please check the expected title format, and make sure your region is in the correct place and spelled correctly."#;
120+
97121
enum ValidationResult {
98122
Ok,
99123
CouldNotMatch,
100124
BadTitleFormat { reason: String },
125+
UnknownRegion,
101126
}
102127

103128
async fn validate_pr(
@@ -106,6 +131,7 @@ async fn validate_pr(
106131
module_name: String,
107132
github_org_name: String,
108133
pr_number: u64,
134+
known_region_aliases: &KnownRegions,
109135
) -> Result<ValidationResult, Error> {
110136
let course = course_schedule
111137
.with_assignments(&octocrab, github_org_name.clone())
@@ -134,7 +160,7 @@ async fn validate_pr(
134160
&course.modules[&module_name],
135161
user_prs,
136162
Vec::new(),
137-
&Region("London".to_owned()),
163+
&ARBITRARY_REGION,
138164
)
139165
.map_err(|err| err.context("Failed to match PRs to assignments"))?;
140166

@@ -150,7 +176,11 @@ async fn validate_pr(
150176
reason: "Wrong number of parts separated by |s".to_owned(),
151177
});
152178
}
153-
// TODO: Validate Regions when they're known (0)
179+
180+
if !known_region_aliases.is_known_ignoring_case(&title_sections[0].trim()) {
181+
return Ok(ValidationResult::UnknownRegion);
182+
}
183+
154184
// TODO: Validate cohorts when they're known (1)
155185
let sprint_regex = Regex::new(r"^(S|s)print \d+$").unwrap();
156186
let sprint_section = title_sections[3].trim();
@@ -167,6 +197,25 @@ async fn validate_pr(
167197
Ok(ValidationResult::Ok)
168198
}
169199

200+
struct KnownRegions(BTreeMap<&'static str, Vec<&'static str>>);
201+
202+
impl KnownRegions {
203+
fn is_known_ignoring_case(&self, possible_region: &str) -> bool {
204+
let possible_region_lower = possible_region.to_ascii_lowercase();
205+
for (known_region, known_region_aliases) in &self.0 {
206+
if known_region.to_ascii_lowercase() == possible_region_lower {
207+
return true;
208+
}
209+
for known_region_alias in known_region_aliases {
210+
if known_region_alias.to_ascii_lowercase() == possible_region_lower {
211+
return true;
212+
}
213+
}
214+
}
215+
false
216+
}
217+
}
218+
170219
fn make_fake_course_schedule(module_name: String) -> CourseSchedule {
171220
let fixed_date = NaiveDate::from_ymd_opt(2030, 1, 1).unwrap();
172221
let mut sprints = IndexMap::new();

0 commit comments

Comments
 (0)