Skip to content

Commit b132752

Browse files
d0x2fDylanclaude
authored
Add routing unit tests and comprehensive integration tests (#39)
* Add comprehensive unit tests for actix routing logic Extract inline permission and validation checks from route handlers into named pure functions, then cover them with unit tests. Follows the existing pattern established in boards/routes.rs. - boards/mod.rs: extract check_cards_allowed / check_voting_allowed; 6 tests - cards/mod.rs: extract check_card_owner; 4 tests - cards/routes.rs: extract validate_card_text; 5 tests - columns/routes.rs: extract check_board_owner_permission; 5 tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Remove unnecessary extractions from purpose-built functions boards/mod.rs and cards/mod.rs already had named functions (assert_cards_allowed, assert_voting_allowed, assert_card_owner) for this logic — no extraction needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add comprehensive routing integration tests 34 tests covering the full HTTP request/response cycle through the actix middleware stack against a live Firestore emulator. Tests are #[ignore] and run with: FIRESTORE_EMULATOR_HOST=localhost:8080 cargo test -- --ignored Coverage: - Boards: create, list, get, update, delete — including owner/non-owner permission cases and not-found handling - Columns: create, list, get, update, delete — owner-only mutation checks - Cards: create (validation + cards-closed gate), list, get, update, delete, vote (including voting-closed gate), react, CSV export Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Reuse FirestoreDb connection for cleanup in integration tests Each test now clones the db before passing it to make_app!, keeping a handle for cleanup rather than opening a second emulator connection. Also removes a stray no-op emulator_db().await call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Split integration_tests.rs into per-domain submodules Move flat 1193-line file into src/integration_tests/ with board_tests.rs, column_tests.rs, card_tests.rs submodules. Shared infrastructure (emulator_db, test_config, make_app!, session_cookie, body_json, setup_board, setup_board_and_column) stays in mod.rs. Macro uses fully-qualified crate paths to work when expanded in child modules. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address review feedback: fix redundant unit tests and fill coverage gaps - Collapse board_owner_can_update_column + board_owner_can_delete_column (identical tests of the same function) into board_owner_is_permitted, and non_owner_cannot_update/delete_column into non_owner_is_forbidden - Add delete_as_non_owner_returns_403 integration test for cards - Add update_as_non_owner_with_anyone_is_owner_still_returns_403 integration test for columns (anyone_is_owner only applies to board settings) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Dylan <dylan@encapto.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent cb0ae57 commit b132752

7 files changed

Lines changed: 1431 additions & 14 deletions

File tree

src/cards/routes.rs

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,14 @@ use crate::columns::get_columns;
1212
use crate::error::Error;
1313
use crate::participants::models::Participant;
1414

15+
fn validate_card_text(card_message: &CardMessage) -> Result<(), Error> {
16+
match &card_message.text {
17+
Some(text) if text.is_empty() => Err(Error::BadRequest("Empty cards are not allowed.".into())),
18+
None => Err(Error::BadRequest("Card text must be provided.".into())),
19+
Some(_) => Ok(()),
20+
}
21+
}
22+
1523
#[post("boards/{board_id}/columns/{column_id}/cards")]
1624
pub async fn new(
1725
firestore: web::Data<FirestoreDb>,
@@ -29,14 +37,7 @@ pub async fn new(
2937
column_id
3038
));
3139

32-
// Empty cards are not allowed
33-
if let Some(text) = &card_message.text {
34-
if text.is_empty() {
35-
return Err(Error::BadRequest("Empty cards are not allowed.".into()));
36-
}
37-
} else {
38-
return Err(Error::BadRequest("Card text must be provided.".into()));
39-
}
40+
validate_card_text(&card_message)?;
4041

4142
let card = db::new(&firestore, &participant, &board_id, card_message).await?;
4243
Ok(
@@ -222,3 +223,43 @@ pub async fn csv(
222223
.body(String::from_utf8(csv_writer.into_inner()?)?),
223224
)
224225
}
226+
227+
#[cfg(test)]
228+
mod tests {
229+
use super::*;
230+
231+
fn msg(text: Option<&str>) -> CardMessage {
232+
CardMessage { author: None, text: text.map(|s| s.to_string()), column: None }
233+
}
234+
235+
#[test]
236+
fn non_empty_text_is_valid() {
237+
assert!(validate_card_text(&msg(Some("Hello"))).is_ok());
238+
}
239+
240+
#[test]
241+
fn empty_text_is_bad_request() {
242+
assert!(matches!(validate_card_text(&msg(Some(""))), Err(Error::BadRequest(_))));
243+
}
244+
245+
#[test]
246+
fn empty_text_error_message() {
247+
match validate_card_text(&msg(Some(""))) {
248+
Err(Error::BadRequest(s)) => assert_eq!(s, "Empty cards are not allowed."),
249+
other => panic!("expected BadRequest, got {other:?}"),
250+
}
251+
}
252+
253+
#[test]
254+
fn missing_text_is_bad_request() {
255+
assert!(matches!(validate_card_text(&msg(None)), Err(Error::BadRequest(_))));
256+
}
257+
258+
#[test]
259+
fn missing_text_error_message() {
260+
match validate_card_text(&msg(None)) {
261+
Err(Error::BadRequest(s)) => assert_eq!(s, "Card text must be provided."),
262+
other => panic!("expected BadRequest, got {other:?}"),
263+
}
264+
}
265+
}

src/columns/routes.rs

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,18 @@ use actix_web::{delete, get, patch, post, web, HttpResponse};
55
use super::db;
66
use super::models::ColumnMessage;
77
use crate::boards;
8+
use crate::boards::models::Board;
89
use crate::error::Error;
910
use crate::participants::models::Participant;
1011

12+
fn check_board_owner_permission(board: &Board, participant: &FirestoreReference) -> Result<(), Error> {
13+
if board.owner != *participant {
14+
Err(Error::Forbidden)
15+
} else {
16+
Ok(())
17+
}
18+
}
19+
1120
#[post("boards/{board_id}/columns")]
1221
pub async fn new(
1322
firestore: web::Data<FirestoreDb>,
@@ -55,9 +64,7 @@ pub async fn update(
5564
.unwrap()
5665
.into(),
5766
);
58-
if board.owner != participant_reference {
59-
return Err(Error::Forbidden);
60-
}
67+
check_board_owner_permission(&board, &participant_reference)?;
6168
let column = db::update(
6269
&firestore,
6370
&board_id,
@@ -82,9 +89,47 @@ pub async fn delete(
8289
.into(),
8390
);
8491
let board = boards::db::get(&firestore, &board_id).await?;
85-
if board.owner != participant_reference {
86-
return Err(Error::Forbidden);
87-
}
92+
check_board_owner_permission(&board, &participant_reference)?;
8893
db::delete(&firestore, &board_id, &column_id).await?;
8994
Ok(HttpResponse::Ok().finish())
9095
}
96+
97+
#[cfg(test)]
98+
mod tests {
99+
use super::*;
100+
use chrono::Utc;
101+
use serde_json::Map;
102+
103+
fn ref_(s: &str) -> FirestoreReference {
104+
FirestoreReference(s.to_string())
105+
}
106+
107+
fn make_board(owner: &str) -> Board {
108+
Board {
109+
id: "board1".to_string(),
110+
name: "Test".to_string(),
111+
cards_open: true,
112+
voting_open: true,
113+
ice_breaking: "".to_string(),
114+
created_at: Utc::now().timestamp(),
115+
owner: ref_(owner),
116+
anyone_is_owner: false,
117+
data: serde_json::Value::Object(Map::new()),
118+
}
119+
}
120+
121+
#[test]
122+
fn board_owner_is_permitted() {
123+
let board = make_board("participants/owner");
124+
assert!(check_board_owner_permission(&board, &ref_("participants/owner")).is_ok());
125+
}
126+
127+
#[test]
128+
fn non_owner_is_forbidden() {
129+
let board = make_board("participants/owner");
130+
assert!(matches!(
131+
check_board_owner_permission(&board, &ref_("participants/other")),
132+
Err(Error::Forbidden)
133+
));
134+
}
135+
}

0 commit comments

Comments
 (0)