mirror of
https://github.com/dsec-hub/dsec-discord-bot.git
synced 2026-09-22 15:53:56 +00:00
NEW-UXA11Y-11: normalise names and student IDs before comparing them
Verification compared names with exact lowercased equality and never trimmed, so a trailing space, a double space, or a missing middle name turned a real paid member away — and a pasted "s123456789 " was looked up with the trailing space and missed. Add normalise_name (trim + collapse whitespace), normalise_student_id (strip whitespace and a leading "s"), and name_matches (equal, or every typed word present in the roster name in order), and route the database path, the cache path (cached_name_matches / cache_student) and the student-id lookup through them. "Doe John" still does not match "John Doe" and an empty name matches nothing. Update the modal placeholder to "As it appears on your DUSA membership". Four unit tests added. The two failure embeds are left byte-identical (SEC-19 owns those). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017XrE7F9ZuBWdQnS8CZvYDE
This commit is contained in:
parent
2b668388d5
commit
0a76f73de5
2 changed files with 86 additions and 14 deletions
|
|
@ -13,7 +13,7 @@ pub struct StudentRow {
|
||||||
#[name = "Club Verification"]
|
#[name = "Club Verification"]
|
||||||
pub struct VerificationModal {
|
pub struct VerificationModal {
|
||||||
#[name = "Full Name"]
|
#[name = "Full Name"]
|
||||||
#[placeholder = "John Doe"]
|
#[placeholder = "As it appears on your DUSA membership"]
|
||||||
#[max_length = 50]
|
#[max_length = 50]
|
||||||
pub name: String,
|
pub name: String,
|
||||||
#[name = "Student ID"]
|
#[name = "Student ID"]
|
||||||
|
|
|
||||||
|
|
@ -131,6 +131,50 @@ async fn grant_verified_role(
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Lower-case, trim, and collapse runs of internal whitespace to one space.
|
||||||
|
/// Used for every name comparison so a stray space or a double space in the
|
||||||
|
/// DUSA roster never rejects a real member.
|
||||||
|
fn normalise_name(raw: &str) -> String {
|
||||||
|
raw.to_lowercase()
|
||||||
|
.split_whitespace()
|
||||||
|
.collect::<Vec<_>>()
|
||||||
|
.join(" ")
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Trim, lower-case, strip a leading "s", and drop spaces so a pasted
|
||||||
|
/// "s123 456 789 " looks up as "123456789".
|
||||||
|
fn normalise_student_id(raw: &str) -> String {
|
||||||
|
let lowered: String = raw
|
||||||
|
.chars()
|
||||||
|
.filter(|c| !c.is_whitespace())
|
||||||
|
.collect::<String>()
|
||||||
|
.to_lowercase();
|
||||||
|
lowered.strip_prefix('s').unwrap_or(&lowered).to_string()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// True when the submitted name is the roster name, or every word the student
|
||||||
|
/// typed appears in the roster name in the same order. Lets "John Doe" match a
|
||||||
|
/// roster row of "John Michael Doe" without matching an unrelated person.
|
||||||
|
fn name_matches(roster: &str, submitted: &str) -> bool {
|
||||||
|
let roster = normalise_name(roster);
|
||||||
|
let submitted = normalise_name(submitted);
|
||||||
|
if submitted.is_empty() {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
if roster == submitted {
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
let roster_words: Vec<&str> = roster.split(' ').collect();
|
||||||
|
let mut idx = 0usize;
|
||||||
|
for word in submitted.split(' ') {
|
||||||
|
match roster_words[idx..].iter().position(|w| *w == word) {
|
||||||
|
Some(offset) => idx += offset + 1,
|
||||||
|
None => return false,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
true
|
||||||
|
}
|
||||||
|
|
||||||
/// Whether the cached name for `student_id` matches the submitted `name`.
|
/// Whether the cached name for `student_id` matches the submitted `name`.
|
||||||
fn cached_name_matches(data: &Data, student_id: &str, name: &str) -> bool {
|
fn cached_name_matches(data: &Data, student_id: &str, name: &str) -> bool {
|
||||||
let cache = data
|
let cache = data
|
||||||
|
|
@ -139,15 +183,15 @@ fn cached_name_matches(data: &Data, student_id: &str, name: &str) -> bool {
|
||||||
.lock()
|
.lock()
|
||||||
.expect("Failed to get cache");
|
.expect("Failed to get cache");
|
||||||
match cache.get(student_id) {
|
match cache.get(student_id) {
|
||||||
Some(cached_name) => cached_name == &name.to_lowercase(),
|
Some(cached_name) => name_matches(cached_name, name),
|
||||||
None => false,
|
None => false,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Store the resolved student name in the cache (lower-cased for comparison).
|
/// Store the resolved student name in the cache (normalised for comparison).
|
||||||
fn cache_student(data: &Data, student_id: &str, full_name: &str) {
|
fn cache_student(data: &Data, student_id: &str, full_name: &str) {
|
||||||
let mut cache = data.state.student_cache.lock().unwrap();
|
let mut cache = data.state.student_cache.lock().unwrap();
|
||||||
cache.insert(student_id.to_string(), full_name.to_lowercase());
|
cache.insert(student_id.to_string(), normalise_name(full_name));
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Look up a student by id in the database.
|
/// Look up a student by id in the database.
|
||||||
|
|
@ -236,18 +280,15 @@ async fn handle_verify(
|
||||||
let verify_result: Result<(), Error> = async {
|
let verify_result: Result<(), Error> = async {
|
||||||
let discord_member = GuildId::member(guild_id, ctx, user_id).await?;
|
let discord_member = GuildId::member(guild_id, ctx, user_id).await?;
|
||||||
|
|
||||||
let input_student_id = modal_data.student_id.to_lowercase();
|
let student_id = normalise_student_id(&modal_data.student_id);
|
||||||
let student_id = input_student_id
|
|
||||||
.strip_prefix("s")
|
|
||||||
.unwrap_or(&input_student_id);
|
|
||||||
|
|
||||||
if cached_name_matches(data, student_id, &modal_data.name) {
|
if cached_name_matches(data, &student_id, &modal_data.name) {
|
||||||
grant_verified_role(
|
grant_verified_role(
|
||||||
ctx,
|
ctx,
|
||||||
data,
|
data,
|
||||||
&modal_submit,
|
&modal_submit,
|
||||||
&discord_member,
|
&discord_member,
|
||||||
student_id,
|
&student_id,
|
||||||
verified_role_id,
|
verified_role_id,
|
||||||
true,
|
true,
|
||||||
)
|
)
|
||||||
|
|
@ -255,7 +296,7 @@ async fn handle_verify(
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
|
|
||||||
let Some(student) = fetch_student(data, student_id).await? else {
|
let Some(student) = fetch_student(data, &student_id).await? else {
|
||||||
modal_submit
|
modal_submit
|
||||||
.create_response(
|
.create_response(
|
||||||
ctx,
|
ctx,
|
||||||
|
|
@ -269,15 +310,15 @@ async fn handle_verify(
|
||||||
return Ok(());
|
return Ok(());
|
||||||
};
|
};
|
||||||
|
|
||||||
cache_student(data, student_id, &student.full_name);
|
cache_student(data, &student_id, &student.full_name);
|
||||||
|
|
||||||
if student.full_name.to_lowercase() == modal_data.name.to_lowercase() {
|
if name_matches(&student.full_name, &modal_data.name) {
|
||||||
grant_verified_role(
|
grant_verified_role(
|
||||||
ctx,
|
ctx,
|
||||||
data,
|
data,
|
||||||
&modal_submit,
|
&modal_submit,
|
||||||
&discord_member,
|
&discord_member,
|
||||||
student_id,
|
&student_id,
|
||||||
verified_role_id,
|
verified_role_id,
|
||||||
false,
|
false,
|
||||||
)
|
)
|
||||||
|
|
@ -333,3 +374,34 @@ pub async fn on_interaction_create(
|
||||||
|
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn trims_and_collapses_names() {
|
||||||
|
assert!(name_matches("John Doe", " john doe "));
|
||||||
|
assert!(name_matches("John Doe", "JOHN DOE"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn allows_a_missing_middle_name() {
|
||||||
|
assert!(name_matches("John Michael Doe", "John Doe"));
|
||||||
|
assert!(name_matches("John Michael Doe", "john michael doe"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn rejects_a_different_person() {
|
||||||
|
assert!(!name_matches("John Michael Doe", "Jane Doe"));
|
||||||
|
assert!(!name_matches("John Doe", "Doe John"));
|
||||||
|
assert!(!name_matches("John Doe", ""));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn normalises_student_ids() {
|
||||||
|
assert_eq!(normalise_student_id("s123456789 "), "123456789");
|
||||||
|
assert_eq!(normalise_student_id("S123456789"), "123456789");
|
||||||
|
assert_eq!(normalise_student_id(" 123 456 789 "), "123456789");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue