diff --git a/Cargo.lock b/Cargo.lock index 7e0eedb..72da197 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -442,6 +442,41 @@ version = "0.8.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28" +[[package]] +name = "darling" +version = "0.21.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9cdf337090841a411e2a7f3deb9187445851f91b309c0c0a29e05f74a00a48c0" +dependencies = [ + "darling_core", + "darling_macro", +] + +[[package]] +name = "darling_core" +version = "0.21.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1247195ecd7e3c85f83c8d2a366e4210d588e802133e1e355180a9870b517ea4" +dependencies = [ + "fnv", + "ident_case", + "proc-macro2", + "quote", + "strsim", + "syn 2.0.102", +] + +[[package]] +name = "darling_macro" +version = "0.21.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d38308df82d1080de0afee5d069fa14b0326a88c14f15c5ccda35b4a6c414c81" +dependencies = [ + "darling_core", + "quote", + "syn 2.0.102", +] + [[package]] name = "dashmap" version = "6.1.0" @@ -742,6 +777,29 @@ version = "0.31.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "07e28edb80900c19c28f1072f2e8aeca7fa06b23cd4169cefe1af5aa3260783f" +[[package]] +name = "google-apis-common" +version = "7.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7530ee92a7e9247c3294ae1b84ea98474dbc27563c49a14d3938e816499bf38f" +dependencies = [ + "base64 0.22.1", + "chrono", + "http 1.3.1", + "http-body-util", + "hyper 1.6.0", + "hyper-util", + "itertools 0.13.0", + "mime", + "percent-encoding", + "serde", + "serde_json", + "serde_with", + "tokio", + "url", + "yup-oauth2 11.0.0", +] + [[package]] name = "google-drive" version = "0.7.0" @@ -766,7 +824,7 @@ dependencies = [ "reqwest-retry", "reqwest-tracing", "ring 0.16.20", - "schemars", + "schemars 0.8.22", "serde", "serde_json", "serde_urlencoded", @@ -774,7 +832,27 @@ dependencies = [ "tokio", "url", "uuid", - "yup-oauth2", + "yup-oauth2 8.3.2", +] + +[[package]] +name = "google-sheets4" +version = "6.0.0+20240621" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e4f8ccfc6418e81d1e2ed66fad49d0487526281505b8a0ed8ee770dc7d6bb1e5" +dependencies = [ + "chrono", + "google-apis-common", + "hyper 1.6.0", + "hyper-rustls 0.27.7", + "hyper-util", + "mime", + "serde", + "serde_json", + "serde_with", + "tokio", + "url", + "yup-oauth2 11.0.0", ] [[package]] @@ -824,7 +902,7 @@ dependencies = [ "reqwest-retry", "reqwest-tracing", "ring 0.16.20", - "schemars", + "schemars 0.8.22", "serde", "serde_json", "serde_urlencoded", @@ -832,14 +910,14 @@ dependencies = [ "tokio", "url", "uuid", - "yup-oauth2", + "yup-oauth2 8.3.2", ] [[package]] name = "h2" -version = "0.3.26" +version = "0.3.27" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81fe527a889e1532da5c525686d96d4c2e74cdd345badf8dfef9f6b39dd5f5e8" +checksum = "0beca50380b1fc32983fc1cb4587bfa4bb9e78fc259aad4a0032d2080309222d" dependencies = [ "bytes", "fnv", @@ -847,7 +925,7 @@ dependencies = [ "futures-sink", "futures-util", "http 0.2.12", - "indexmap", + "indexmap 2.9.0", "slab", "tokio", "tokio-util", @@ -866,13 +944,19 @@ dependencies = [ "futures-core", "futures-sink", "http 1.3.1", - "indexmap", + "indexmap 2.9.0", "slab", "tokio", "tokio-util", "tracing", ] +[[package]] +name = "hashbrown" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" + [[package]] name = "hashbrown" version = "0.14.5" @@ -896,6 +980,12 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" +[[package]] +name = "hex" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" + [[package]] name = "http" version = "0.2.12" @@ -990,7 +1080,7 @@ dependencies = [ "futures-channel", "futures-core", "futures-util", - "h2 0.3.26", + "h2 0.3.27", "http 0.2.12", "http-body 0.4.6", "httparse", @@ -1207,6 +1297,12 @@ dependencies = [ "zerovec", ] +[[package]] +name = "ident_case" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" + [[package]] name = "idna" version = "1.0.3" @@ -1228,6 +1324,17 @@ dependencies = [ "icu_properties", ] +[[package]] +name = "indexmap" +version = "1.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99" +dependencies = [ + "autocfg", + "hashbrown 0.12.3", + "serde", +] + [[package]] name = "indexmap" version = "2.9.0" @@ -1287,6 +1394,15 @@ dependencies = [ "either", ] +[[package]] +name = "itertools" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "413ee7dfc52ee1a4949ceeb7dbc8a33f2d6c088194d9f922fb8318faf1f01186" +dependencies = [ + "either", +] + [[package]] name = "itertools" version = "0.14.0" @@ -2058,6 +2174,26 @@ dependencies = [ "thiserror 2.0.12", ] +[[package]] +name = "ref-cast" +version = "1.0.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f354300ae66f76f1c85c5f84693f0ce81d747e2c3f21a45fef496d89c960bf7d" +dependencies = [ + "ref-cast-impl", +] + +[[package]] +name = "ref-cast-impl" +version = "1.0.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7186006dcb21920990093f30e3dea63b7d6e977bf1256be20c3563a5db070da" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.102", +] + [[package]] name = "regex" version = "1.11.1" @@ -2113,7 +2249,7 @@ dependencies = [ "encoding_rs", "futures-core", "futures-util", - "h2 0.3.26", + "h2 0.3.27", "http 0.2.12", "http-body 0.4.6", "hyper 0.14.32", @@ -2127,7 +2263,7 @@ dependencies = [ "percent-encoding", "pin-project-lite", "rustls 0.21.12", - "rustls-pemfile", + "rustls-pemfile 1.0.4", "serde", "serde_json", "serde_urlencoded", @@ -2357,7 +2493,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a9aace74cb666635c918e9c12bc0d348266037aa8eb599b5cba565709a8dff00" dependencies = [ "openssl-probe", - "rustls-pemfile", + "rustls-pemfile 1.0.4", "schannel", "security-framework 2.11.1", ] @@ -2383,6 +2519,15 @@ dependencies = [ "base64 0.21.7", ] +[[package]] +name = "rustls-pemfile" +version = "2.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dce314e5fee3f39953d46bb63bb8a46d40c2f8fb7cc5a3b6cab2bde9721d6e50" +dependencies = [ + "rustls-pki-types", +] + [[package]] name = "rustls-pki-types" version = "1.12.0" @@ -2462,6 +2607,30 @@ dependencies = [ "uuid", ] +[[package]] +name = "schemars" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4cd191f9397d57d581cddd31014772520aa448f65ef991055d7f61582c65165f" +dependencies = [ + "dyn-clone", + "ref-cast", + "serde", + "serde_json", +] + +[[package]] +name = "schemars" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9558e172d4e8533736ba97870c4b2cd63f84b382a3d6eb063da41b91cce17289" +dependencies = [ + "dyn-clone", + "ref-cast", + "serde", + "serde_json", +] + [[package]] name = "schemars_derive" version = "0.8.22" @@ -2653,6 +2822,38 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_with" +version = "3.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c522100790450cf78eeac1507263d0a350d4d5b30df0c8e1fe051a10c22b376e" +dependencies = [ + "base64 0.22.1", + "chrono", + "hex", + "indexmap 1.9.3", + "indexmap 2.9.0", + "schemars 0.9.0", + "schemars 1.1.0", + "serde", + "serde_derive", + "serde_json", + "serde_with_macros", + "time", +] + +[[package]] +name = "serde_with_macros" +version = "3.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "327ada00f7d64abaac1e55a6911e90cf665aa051b9a561c7006c157f4633135e" +dependencies = [ + "darling", + "proc-macro2", + "quote", + "syn 2.0.102", +] + [[package]] name = "sharded-slab" version = "0.1.7" @@ -2686,7 +2887,7 @@ dependencies = [ "reqwest-retry", "reqwest-tracing", "ring 0.16.20", - "schemars", + "schemars 0.8.22", "serde", "serde_json", "serde_urlencoded", @@ -2694,7 +2895,7 @@ dependencies = [ "tokio", "url", "uuid", - "yup-oauth2", + "yup-oauth2 8.3.2", ] [[package]] @@ -2823,6 +3024,12 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" +[[package]] +name = "strsim" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" + [[package]] name = "subtle" version = "2.6.1" @@ -3281,12 +3488,13 @@ dependencies = [ "email_address 0.2.9 (git+https://github.com/illicitonion/rust-email_address.git?rev=12cd9762a166b79a227beaa90b2f60a768d7c55c)", "futures", "google-drive", + "google-sheets4", "gsuite-api", "http 1.3.1", "http-serde", "hyper-rustls 0.27.7", "hyper-util", - "indexmap", + "indexmap 2.9.0", "itertools 0.14.0", "maplit", "moka", @@ -3891,7 +4099,7 @@ dependencies = [ "log", "percent-encoding", "rustls 0.22.4", - "rustls-pemfile", + "rustls-pemfile 1.0.4", "seahash", "serde", "serde_json", @@ -3901,6 +4109,33 @@ dependencies = [ "url", ] +[[package]] +name = "yup-oauth2" +version = "11.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4ed5f19242090128c5809f6535cc7b8d4e2c32433f6c6005800bbc20a644a7f0" +dependencies = [ + "anyhow", + "async-trait", + "base64 0.22.1", + "futures", + "http 1.3.1", + "http-body-util", + "hyper 1.6.0", + "hyper-rustls 0.27.7", + "hyper-util", + "log", + "percent-encoding", + "rustls 0.23.27", + "rustls-pemfile 2.2.0", + "seahash", + "serde", + "serde_json", + "time", + "tokio", + "url", +] + [[package]] name = "zerocopy" version = "0.8.26" diff --git a/Cargo.toml b/Cargo.toml index 88c6286..b2a9a40 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ dotenv = "0.15.0" email_address = { git = "https://github.com/illicitonion/rust-email_address.git", rev = "12cd9762a166b79a227beaa90b2f60a768d7c55c" } futures = "0.3.31" google-drive = "0.7.0" +google-sheets4 = "6.0.0" gsuite-api = "0.7.0" http = "1.3.1" http-serde = "2.1.1" diff --git a/src/auth.rs b/src/auth.rs index 282faa8..1e9e6a3 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -6,7 +6,6 @@ use axum::{ }; use http::Uri; use serde::Deserialize; -use sheets::Client; use tower_sessions::Session; use uuid::Uuid; @@ -112,7 +111,8 @@ pub async fn handle_google_oauth_callback( server_state.config.public_base_url ); - let mut client = Client::new( + // TODO: Replace this with some other way of getting the token (either a request ourselves, or using another library) so we can drop the sheets dep. + let mut client = ::sheets::Client::new( server_state.config.google_apis_client_id.clone(), (*server_state.config.google_apis_client_secret).clone(), redirect_uri, diff --git a/src/github_accounts.rs b/src/github_accounts.rs index bc9f1ab..a21f89d 100644 --- a/src/github_accounts.rs +++ b/src/github_accounts.rs @@ -3,12 +3,11 @@ use std::collections::BTreeMap; use anyhow::Context; use email_address::EmailAddress; use serde::{Deserialize, Serialize}; -use sheets::types::Sheet; use crate::{ Error, newtypes::{GithubLogin, Region, new_case_insensitive_email_address}, - sheets::{SheetsClient, cell_string}, + sheets::{Sheet, SheetsClient, cell_string}, }; // TODO: Replace this with a serde implementation from a Google Sheet. @@ -17,7 +16,7 @@ pub(crate) async fn get_trainees( sheet_id: &str, ) -> Result, Error> { const EXPECTED_SHEET_NAME: &str = "Form responses 1"; - let data = client.get(sheet_id, true, &[]).await.map_err(|err| { + let data = client.get(sheet_id).await.map_err(|err| { err.with_context(|| { format!( "Failed to get trainees github accounts sheet with id {}", @@ -25,24 +24,11 @@ pub(crate) async fn get_trainees( ) }) })?; - let sheet = data.body.sheets.into_iter().find(|sheet| { - if let Some(properties) = &sheet.properties { - properties.title == EXPECTED_SHEET_NAME - } else { - false - } - }); + let sheet = data.get(EXPECTED_SHEET_NAME); if let Some(sheet) = sheet { let data = trainees_from_sheet(&sheet).map_err(|err| { err.with_context(|| { - format!( - "Failed to read trainees from sheet {}", - sheet - .properties - .map(|properties| properties.title) - .as_deref() - .unwrap_or("") - ) + format!("Failed to read trainees from sheet {}", EXPECTED_SHEET_NAME,) }) })?; Ok(data) @@ -65,41 +51,31 @@ pub struct Trainee { fn trainees_from_sheet(sheet: &Sheet) -> Result, Error> { let mut trainees = BTreeMap::new(); - for data in &sheet.data { - if data.start_column != 0 || data.start_row != 0 { + for (row_index, cells) in sheet.rows.iter().enumerate() { + if row_index == 0 { + continue; + } + if cells.len() < 5 { return Err(Error::Fatal(anyhow::anyhow!( - "Reading data from Google Sheets API - got data chunk that didn't start at row=0,column=0 - got row={},column={}", - data.start_row, - data.start_column + "Reading trainee data from Google Sheets API, row {} didn't have at least 5 columns", + row_index ))); } - for (row_index, row) in data.row_data.iter().enumerate() { - if row_index == 0 { - continue; - } - let cells = &row.values; - if cells.len() < 5 { - return Err(Error::Fatal(anyhow::anyhow!( - "Reading trainee data from Google Sheets API, row {} didn't have at least 5 columns", - row_index - ))); - } - let github_login = GithubLogin::from(cell_string(&cells[3])); + let github_login = GithubLogin::from(cell_string(&cells[3])); - let email = cell_string(&cells[4]); + let email = cell_string(&cells[4]); - trainees.insert( - github_login.clone(), - Trainee { - name: cell_string(&cells[1]), - region: Region(cell_string(&cells[2])), - github_login, - email: new_case_insensitive_email_address(&email) - .with_context(|| format!("Failed to parse trainee email {}", email))?, - }, - ); - } + trainees.insert( + github_login.clone(), + Trainee { + name: cell_string(&cells[1]), + region: Region(cell_string(&cells[2])), + github_login, + email: new_case_insensitive_email_address(&email) + .with_context(|| format!("Failed to parse trainee email {}", email))?, + }, + ); } Ok(trainees) diff --git a/src/mentoring.rs b/src/mentoring.rs index 6d38ebf..87c080e 100644 --- a/src/mentoring.rs +++ b/src/mentoring.rs @@ -2,8 +2,8 @@ use std::collections::{BTreeMap, btree_map::Entry}; use anyhow::Context; use chrono::{NaiveDate, Utc}; +use google_sheets4::api::CellData; use serde::Serialize; -use sheets::types::GridData; use tracing::warn; use crate::{ @@ -44,55 +44,49 @@ pub async fn get_mentoring_records( records: BTreeMap::new(), }; - for sheet_data in sheet_data { - if sheet_data.start_column != 0 || sheet_data.start_row != 0 { - return Err(Error::Fatal(anyhow::anyhow!( - "Start column and row were {} and {}, expected 0 and 0", - sheet_data.start_column, - sheet_data.start_row - ))); + for (row_number, cells) in sheet_data.into_iter().enumerate() { + if cells.is_empty() { + continue; } - - for (row_number, row) in sheet_data.row_data.into_iter().enumerate() { - let cells = row.values; - if cells.is_empty() { - continue; + if cells.len() < 6 && !cell_string(&cells[0]).is_empty() { + warn!( + "Parsing mentoring data from Google Sheet with ID {}: Not enough columns for row {} - expected at least 6, got {} containing: {}", + mentoring_records_sheet_id, + row_number, + cells.len(), + format!("{:#?}", cells), + ); + continue; + } + if row_number == 0 { + let headings = cells.iter().take(6).map(cell_string).collect::>(); + if headings != ["Name", "Region", "Date", "Staff", "Status", "Notes"] { + return Err(Error::Fatal(anyhow::anyhow!( + "Mentoring data sheet contained wrong headings: {}", + headings.join(", ") + ))); } - if cells.len() < 6 && !cell_string(&cells[0]).is_empty() { - warn!( - "Parsing mentoring data from Google Sheet with ID {}: Not enough columns for row {} - expected at least 6, got {} containing: {}", - mentoring_records_sheet_id, - row_number, - cells.len(), - format!("{:#?}", cells), - ); - continue; + } else { + if cells[0].effective_value.is_none() { + break; } - if row_number == 0 { - let headings = cells.iter().take(6).map(cell_string).collect::>(); - if headings != ["Name", "Region", "Date", "Staff", "Status", "Notes"] { - return Err(Error::Fatal(anyhow::anyhow!( - "Mentoring data sheet contained wrong headings: {}", - headings.join(", ") - ))); - } - } else { - if cells[0].effective_value.is_none() { - break; + let name = cell_string(&cells[0]); + let date = cell_date(&cells[2]).with_context(|| { + format!( + "Failed to parse date from row {} in sheet ID {}", + row_number + 1, + mentoring_records_sheet_id + ) + })?; + let entry = mentoring_records.records.entry(name); + match entry { + Entry::Vacant(entry) => { + entry.insert(MentoringRecord { last_date: date }); } - let name = cell_string(&cells[0]); - let date = cell_date(&cells[2]) - .with_context(|| format!("Failed to parse date from row {}", row_number + 1))?; - let entry = mentoring_records.records.entry(name); - match entry { - Entry::Vacant(entry) => { + Entry::Occupied(mut entry) => { + if entry.get().last_date < date { entry.insert(MentoringRecord { last_date: date }); } - Entry::Occupied(mut entry) => { - if entry.get().last_date < date { - entry.insert(MentoringRecord { last_date: date }); - } - } } } } @@ -103,9 +97,10 @@ pub async fn get_mentoring_records( async fn get_mentoring_records_grid_data( client: SheetsClient, mentoring_records_sheet_id: &str, -) -> Result, Error> { - let data_result = client.get(mentoring_records_sheet_id, true, &[]).await; - let data = match data_result { +) -> Result>, Error> { + let expected_sheet_title = "Feedback"; + let data_result = client.get(mentoring_records_sheet_id).await; + let mut data = match data_result { Ok(data) => data, Err(Error::PotentiallyIgnorablePermissions(_)) => { return Ok(Vec::new()); @@ -120,24 +115,12 @@ async fn get_mentoring_records_grid_data( return Err(err); } }; - let expected_sheet_title = "Feedback"; - let sheet = data - .body - .sheets - .into_iter() - .find(|sheet| { - sheet - .properties - .as_ref() - .map(|properties| properties.title.as_str()) - == Some(expected_sheet_title) - }) - .ok_or_else(|| { - Error::Fatal(anyhow::anyhow!( - "Couldn't find sheet '{}' in spreadsheet with ID {}", - expected_sheet_title, - mentoring_records_sheet_id - )) - })?; - Ok(sheet.data) + let sheet = data.remove(expected_sheet_title).ok_or_else(|| { + Error::Fatal(anyhow::anyhow!( + "Couldn't find sheet '{}' in spreadsheet with ID {}", + expected_sheet_title, + mentoring_records_sheet_id + )) + })?; + Ok(sheet.rows) } diff --git a/src/prs.rs b/src/prs.rs index 48bbdc6..f726925 100644 --- a/src/prs.rs +++ b/src/prs.rs @@ -207,14 +207,14 @@ pub(crate) async fn fill_in_reviewers( .collect()) } -#[derive(PartialEq, Eq, Serialize)] +#[derive(Debug, PartialEq, Eq, Serialize)] pub(crate) enum CheckStatus { CheckedAndOk, CheckedAndCheckAgain, Unchecked, } -#[derive(PartialEq, Eq, Serialize)] +#[derive(Debug, PartialEq, Eq, Serialize)] pub(crate) struct ReviewerStaffOnlyDetails { pub(crate) name: String, pub(crate) attended_training: bool, diff --git a/src/register.rs b/src/register.rs index a79c3e6..4cc2f37 100644 --- a/src/register.rs +++ b/src/register.rs @@ -1,9 +1,9 @@ use anyhow::Context; use chrono::{DateTime, NaiveDate, Utc}; use email_address::EmailAddress; +use google_sheets4::api::CellData; use indexmap::IndexMap; use serde::Serialize; -use sheets::types::{CellData, GridData}; use tracing::warn; use crate::{ @@ -61,124 +61,100 @@ pub(crate) async fn get_register( ) -> Result { let mut modules: IndexMap = IndexMap::new(); - let data = client - .get(®ister_sheet_id, true, &[]) - .await - .map_err(|err| { - err.with_context(|| format!("Failed to get spreadsheet with ID {}", register_sheet_id)) - })?; - for sheet in data.body.sheets { - if let Some(properties) = &sheet.properties { - let title = properties.title.clone(); - if modules.contains_key(&title) { - return Err(Error::Fatal(anyhow::anyhow!( - "Failed to read register sheet ID {} - duplicate sheets {}", - register_sheet_id, - title - ))); - } - let register_url = format!( - "{}{}gid={}", - data.body.spreadsheet_url, - if data.body.spreadsheet_url.contains("?") { - "&" - } else { - "?" - }, - properties.sheet_id - ); - let attendance = read_module(sheet.data, register_url.clone(), start_date, end_date) - .with_context(|| { - format!( - "Failed to read register sheet ID {} sheet {}", - register_sheet_id, title - ) - })?; - let module = ModuleAttendance { - register_url, - attendance, - }; - // TODO: Unify module names across sources (repo has Module-prefix, register does not) - modules.insert(format!("Module-{}", title.replace(' ', "-")), module); - } else { - warn!("Ignoring sheet in {} with no properties", register_sheet_id); + let data = client.get(®ister_sheet_id).await.map_err(|err| { + err.with_context(|| format!("Failed to get spreadsheet with ID {}", register_sheet_id)) + })?; + for (title, sheet) in data.into_iter() { + if modules.contains_key(&title) { + return Err(Error::Fatal(anyhow::anyhow!( + "Failed to read register sheet ID {} - duplicate sheets {}", + register_sheet_id, + title + ))); } + let register_url = format!( + "{}{}gid={}", + sheet.url, + if sheet.url.contains("?") { "&" } else { "?" }, + sheet.id + ); + let attendance = read_module(sheet.rows, register_url.clone(), start_date, end_date) + .with_context(|| { + format!( + "Failed to read register sheet ID {} sheet {}", + register_sheet_id, title + ) + })?; + let module = ModuleAttendance { + register_url, + attendance, + }; + // TODO: Unify module names across sources (repo has Module-prefix, register does not) + modules.insert(format!("Module-{}", title.replace(' ', "-")), module); } Ok(Register { modules }) } fn read_module( - sheet_data: Vec, + sheet_data: Vec>, register_url: String, start_date: NaiveDate, end_date: NaiveDate, ) -> Result>, anyhow::Error> { let mut sprints = Vec::new(); - 'sheet: for data in sheet_data { - if data.start_column != 0 || data.start_row != 0 { + for (row_number, cells) in sheet_data.into_iter().enumerate() { + // Some sheets have documentation or pivot table + if row_number == 0 && !cells.is_empty() && cell_string(&cells[0]) != "Name" { + continue; + } + if cells.len() < 7 { return Err(anyhow::anyhow!( - "Start column and row were {} and {}, expected 0 and 0", - data.start_column, - data.start_row + "Not enough columns for row {} - expected at least 7, got {} containing: {}", + row_number, + cells.len(), + format!("{:#?}", cells), )); } - for (row_number, row) in data.row_data.into_iter().enumerate() { - let cells = row.values; - // Some sheets have documentation or pivot table - if row_number == 0 && !cells.is_empty() && cell_string(&cells[0]) != "Name" { - continue 'sheet; - } - if cells.len() < 7 { + if row_number == 0 { + let headings = cells.iter().take(7).map(cell_string).collect::>(); + if headings + != [ + "Name", + "Email", + "Timestamp", + "Course", + "Module", + "Day", + "Location", + ] + { return Err(anyhow::anyhow!( - "Not enough columns for row {} - expected at least 7, got {} containing: {}", - row_number, - cells.len(), - format!("{:#?}", cells), + "Register sheet contained wrong headings: {}", + headings.join(", ") )); } - if row_number == 0 { - let headings = cells.iter().take(7).map(cell_string).collect::>(); - if headings - != [ - "Name", - "Email", - "Timestamp", - "Course", - "Module", - "Day", - "Location", - ] - { - return Err(anyhow::anyhow!( - "Register sheet contained wrong headings: {}", - headings.join(", ") - )); - } + } else { + if cells[0].effective_value.is_none() { + break; + } + let (sprint_number, attendance) = read_row(&cells, register_url.clone()) + .with_context(|| format!("Failed to read attendance from row {}", row_number))?; + if attendance.timestamp.date_naive() <= start_date + || attendance.timestamp.date_naive() >= end_date + { + continue; + } + let sprint_index = sprint_number - 1; + while sprints.len() < sprint_number { + sprints.push(IndexMap::new()); + } + if sprints[sprint_index].contains_key(&attendance.email) { + warn!( + "Register sheet contained duplicate entry for sprint {} trainee {}", + sprint_number, attendance.email + ); } else { - if cells[0].effective_value.is_none() { - break; - } - let (sprint_number, attendance) = read_row(&cells, register_url.clone()) - .with_context(|| { - format!("Failed to read attendance from row {}", row_number) - })?; - if attendance.timestamp.date_naive() <= start_date - || attendance.timestamp.date_naive() >= end_date - { - continue; - } - let sprint_index = sprint_number - 1; - while sprints.len() < sprint_number { - sprints.push(IndexMap::new()); - } - if sprints[sprint_index].contains_key(&attendance.email) { - warn!( - "Register sheet contained duplicate entry for sprint {} trainee {}", - sprint_number, attendance.email - ); - } else { - sprints[sprint_index].insert(attendance.email.clone(), attendance); - } + sprints[sprint_index].insert(attendance.email.clone(), attendance); } } } diff --git a/src/reviewer_staff_info.rs b/src/reviewer_staff_info.rs index afbfbaf..21f66cc 100644 --- a/src/reviewer_staff_info.rs +++ b/src/reviewer_staff_info.rs @@ -1,12 +1,10 @@ use std::collections::BTreeMap; -use sheets::types::Sheet; - use crate::{ Error, newtypes::GithubLogin, prs::{CheckStatus, ReviewerStaffOnlyDetails}, - sheets::{SheetsClient, cell_bool, cell_string}, + sheets::{Sheet, SheetsClient, cell_bool, cell_string}, }; pub(crate) async fn get_reviewer_staff_info( @@ -14,7 +12,7 @@ pub(crate) async fn get_reviewer_staff_info( sheet_id: &str, ) -> Result, Error> { const EXPECTED_SHEET_NAME: &str = "Sheet1"; - let data = client.get(sheet_id, true, &[]).await.map_err(|err| { + let mut data = client.get(sheet_id).await.map_err(|err| { err.with_context(|| { format!( "Failed to get reviewer staff detail sheet with id {}", @@ -22,23 +20,13 @@ pub(crate) async fn get_reviewer_staff_info( ) }) })?; - let sheet = data.body.sheets.into_iter().find(|sheet| { - if let Some(properties) = &sheet.properties { - properties.title == EXPECTED_SHEET_NAME - } else { - false - } - }); + let sheet = data.remove(EXPECTED_SHEET_NAME); if let Some(sheet) = sheet { let data = reviewer_staff_detail_from_sheet(&sheet).map_err(|err| { err.with_context(|| { format!( "Failed to read reviewer staff details from sheet {}", - sheet - .properties - .map(|properties| properties.title) - .as_deref() - .unwrap_or("") + EXPECTED_SHEET_NAME ) }) })?; @@ -56,47 +44,38 @@ fn reviewer_staff_detail_from_sheet( sheet: &Sheet, ) -> Result, Error> { let mut reviewers = BTreeMap::new(); - for data in &sheet.data { - if data.start_column != 0 || data.start_row != 0 { - return Err(Error::Fatal(anyhow::anyhow!( - "Reading data from Google Sheets API - got data chunk that didn't start at row=0,column=0 - got row={},column={}", - data.start_row, - data.start_column - ))); + + for (row_index, cells) in sheet.rows.iter().enumerate() { + if row_index == 0 { + continue; + } + if cells.len() < 6 { + continue; } - for (row_index, row) in data.row_data.iter().enumerate() { - if row_index == 0 { - continue; - } - let cells = &row.values; - if cells.len() < 6 { - continue; - } - let github_login = GithubLogin::from(cell_string(&cells[0])); + let github_login = GithubLogin::from(cell_string(&cells[0])); - let notes = match cells.get(6) { - Some(cell) => cell_string(cell), - None => String::new(), - }; + let notes = match cells.get(6) { + Some(cell) => cell_string(cell), + None => String::new(), + }; - let checked = match (cell_bool(&cells[3]), cell_bool(&cells[4])) { - (true, false) => CheckStatus::CheckedAndOk, - (true, true) => CheckStatus::CheckedAndCheckAgain, - (false, _) => CheckStatus::Unchecked, - }; + let checked = match (cell_bool(&cells[3]), cell_bool(&cells[4])) { + (true, false) => CheckStatus::CheckedAndOk, + (true, true) => CheckStatus::CheckedAndCheckAgain, + (false, _) => CheckStatus::Unchecked, + }; - reviewers.insert( - github_login.clone(), - ReviewerStaffOnlyDetails { - name: cell_string(&cells[1]), - attended_training: cell_bool(&cells[2]), - checked, - quality: cell_string(&cells[5]), - notes, - }, - ); - } + reviewers.insert( + github_login.clone(), + ReviewerStaffOnlyDetails { + name: cell_string(&cells[1]), + attended_training: cell_bool(&cells[2]), + checked, + quality: cell_string(&cells[5]), + notes, + }, + ); } Ok(reviewers) diff --git a/src/sheets.rs b/src/sheets.rs index c284803..55c6639 100644 --- a/src/sheets.rs +++ b/src/sheets.rs @@ -1,35 +1,87 @@ +use std::collections::BTreeMap; + use anyhow::Context; +use chrono::Days; +use google_sheets4::{ + Sheets, + api::{CellData, ErrorValue}, +}; use http::{HeaderMap, Uri}; -use sheets::{spreadsheets::Spreadsheets, types::CellData}; +use hyper_rustls::HttpsConnector; +use hyper_util::client::legacy::connect::HttpConnector; +use serde_json::Value; use tower_sessions::Session; +use tracing::warn; use crate::{ Error, ServerState, google_auth::{GoogleScope, make_redirect_uri, redirect_endpoint}, }; +// This is documented as a union where at most one value is set, per https://developers.google.com/workspace/sheets/api/reference/rest/v4/spreadsheets/other#ExtendedValue +#[allow(unused)] +enum ExtendedValue { + String(String), + Number(f64), + Bool(bool), + Formula(String), + Error(ErrorValue), + None, +} + +impl From<&CellData> for ExtendedValue { + fn from(value: &CellData) -> Self { + if let Some(value) = &value.effective_value { + if let Some(value) = &value.string_value { + ExtendedValue::String(value.clone()) + } else if let Some(value) = value.number_value { + ExtendedValue::Number(value) + } else if let Some(value) = &value.error_value { + ExtendedValue::Error(value.clone()) + } else if let Some(value) = &value.formula_value { + ExtendedValue::Formula(value.clone()) + } else if let Some(value) = value.bool_value { + ExtendedValue::Bool(value) + } else { + ExtendedValue::None + } + } else { + ExtendedValue::None + } + } +} + pub(crate) fn cell_string(cell: &CellData) -> String { - let value = cell.effective_value.clone(); - if let Some(value) = value { - value.string_value + if let ExtendedValue::String(value) = ExtendedValue::from(cell) { + value } else { String::new() } } pub(crate) fn cell_bool(cell: &CellData) -> bool { - let value = cell.effective_value.clone(); - if let Some(value) = value { - value.bool_value + if let ExtendedValue::Bool(value) = ExtendedValue::from(cell) { + value } else { false } } pub(crate) fn cell_date(cell: &CellData) -> Result { - let date_string = &cell.formatted_value; - chrono::NaiveDate::parse_from_str(date_string, "%Y-%m-%d") - .with_context(|| format!("Failed to parse {} as a date", date_string)) + if let ExtendedValue::Number(value) = ExtendedValue::from(cell) { + // UNWRAP: Statically known valid date. + let epoch = chrono::NaiveDate::from_ymd_opt(1899, 12, 30).unwrap(); + // AS: Hopefully this is ok, Google Sheets claims it will be valid... + let days = value as u64; + epoch + .checked_add_days(Days::new(days)) + .ok_or_else(|| anyhow::anyhow!("Failed to parse {} as a date", value)) + } else { + Err(anyhow::anyhow!( + "Failed to parse cell containing {} as a date", + cell.formatted_value.clone().unwrap_or_default() + )) + } } pub(crate) async fn sheets_client( @@ -60,13 +112,19 @@ pub(crate) async fn sheets_client( let redirect_endpoint = redirect_endpoint(&server_state); if let Some(token) = maybe_token { - let client = ::sheets::Client::new( - server_state.config.google_apis_client_id.clone(), - server_state.config.google_apis_client_secret.to_string(), - &redirect_endpoint, - token, - "", - ); + let client = + hyper_util::client::legacy::Client::builder(hyper_util::rt::TokioExecutor::new()) + .build( + hyper_rustls::HttpsConnectorBuilder::new() + .with_native_roots() + .unwrap() + .https_only() + .enable_http1() + .enable_http2() + .build(), + ); + + let client = Sheets::new(client, token); Ok(SheetsClient { client, original_uri, @@ -87,42 +145,134 @@ pub(crate) async fn sheets_client( #[derive(Clone)] pub struct SheetsClient { - client: ::sheets::Client, + client: Sheets>, original_uri: Uri, server_state: ServerState, } +pub struct Sheet { + pub title: String, + pub rows: Vec>, + pub id: String, + pub url: String, +} + impl SheetsClient { pub async fn get( self, sheet_id: &str, - include_grid_data: bool, - ranges: &[String], - ) -> Result<::sheets::Response<::sheets::types::Spreadsheet>, Error> { - let result = Spreadsheets { - client: self.client, - } - .get(sheet_id, include_grid_data, ranges) - .await; + // ) -> Result<::sheets::Response<::sheets::types::Spreadsheet>, Error> { + ) -> Result, Error> { + let result = self + .client + .spreadsheets() + .get(sheet_id) + .include_grid_data(true) + .doit() + .await; match result { - Ok(value) => Ok(value), - Err(::sheets::ClientError::HttpError { status, .. }) if status.as_u16() == 401 => { - Err(Error::Redirect( - make_redirect_uri( - &self.server_state, - self.original_uri, - &redirect_endpoint(&self.server_state), - GoogleScope::Sheets, - ) - .await?, - )) + Ok((_, spreadsheet)) => { + let mut sheets = BTreeMap::new(); + let Some(url) = spreadsheet.spreadsheet_url else { + warn!( + "Fetching spreadsheet with ID {} didn't have URL metadata", + sheet_id + ); + return Ok(sheets); + }; + let Some(id) = spreadsheet.spreadsheet_id else { + warn!( + "Fetching spreadsheet with ID {} didn't have ID metadata", + sheet_id + ); + return Ok(sheets); + }; + for (sheet_index, sheet) in spreadsheet + .sheets + .unwrap_or_default() + .into_iter() + .enumerate() + { + let Some(properties) = sheet.properties else { + warn!( + "Fetching spreadsheet with ID {} - sheet with index {} didn't have properties", + sheet_id, sheet_index + ); + continue; + }; + let Some(title) = properties.title else { + warn!( + "Fetching spreadsheet with ID {} - sheet with index {} didn't have title", + sheet_id, sheet_index + ); + continue; + }; + if let Some(data) = sheet.data { + if data.is_empty() { + warn!( + "Fetching spreadsheet with ID {} - sheet with index {} and title {} didn't have data", + sheet_id, sheet_index, title + ); + continue; + } + for data in data { + if data.start_column.unwrap_or(0) != 0 + || data.start_row.unwrap_or(0) != 0 + { + return Err(Error::Fatal(anyhow::anyhow!( + "Error reading spreadsheet ID {} sheet {}: Start column and row were {:?} and {:?}, expected 0 and 0", + sheet_id, + title, + data.start_column, + data.start_row + ))); + } + if let Some(row_data) = data.row_data { + let rows = + row_data.into_iter().filter_map(|row| row.values).collect(); + sheets.insert( + title.clone(), + Sheet { + rows, + title: title.clone(), + url: url.clone(), + id: id.clone(), + }, + ); + } else { + warn!( + "Fetching spreadsheet with ID {} - sheet with index {} and title {} didn't have row_data", + sheet_id, sheet_index, title + ); + } + } + } + } + Ok(sheets) } - Err(err @ ::sheets::ClientError::HttpError { status, .. }) - if status.as_u16() == 403 => - { - Err(Error::PotentiallyIgnorablePermissions(err.into())) + Err( + ::google_sheets4::Error::MissingAPIKey | ::google_sheets4::Error::MissingToken(..), + ) => Err(Error::Redirect( + make_redirect_uri( + &self.server_state, + self.original_uri, + &redirect_endpoint(&self.server_state), + GoogleScope::Sheets, + ) + .await?, + )), + Err(err) => { + // TODO: Upgrade to a let guard when https://github.com/rust-lang/rust/issues/51114 stabilises. + if let ::google_sheets4::Error::BadRequest(ref details) = err + && let Value::Object(object) = details + && object.get("error").and_then(|error| error.get("code")) + == Some(&Value::Number(serde_json::Number::from_u128(403).unwrap())) + { + Err(Error::PotentiallyIgnorablePermissions(err.into())) + } else { + Err(Error::Fatal(err.into())) + } } - Err(err) => Err(Error::Fatal(err.into())), } } }