From f8f18def90d28f992e73a24c52d99297b2a092c2 Mon Sep 17 00:00:00 2001 From: philipp Date: Fri, 11 Oct 2024 09:29:26 +0200 Subject: [PATCH 1/9] create cluster in userrole --- migration.sql | 16 ++++++++- src/model/family.rs | 13 ++++++- src/model/role.rs | 38 ++++++++++++++++++--- src/model/user.rs | 77 ++++++++++++++++++++++++++++++++++-------- src/tera/admin/user.rs | 22 ++++++++---- staging-diff.sql | 32 ++++++++++++++++++ 6 files changed, 170 insertions(+), 28 deletions(-) diff --git a/migration.sql b/migration.sql index ad652b5..c2b105d 100644 --- a/migration.sql +++ b/migration.sql @@ -27,7 +27,8 @@ CREATE TABLE IF NOT EXISTS "family" ( CREATE TABLE IF NOT EXISTS "role" ( "id" integer NOT NULL PRIMARY KEY AUTOINCREMENT, - "name" text NOT NULL UNIQUE + "name" text NOT NULL UNIQUE, + "cluster" text ); CREATE TABLE IF NOT EXISTS "user_role" ( @@ -220,3 +221,16 @@ CREATE TABLE IF NOT EXISTS "distance" ( "distance_in_km" integer NOT NULL ); + +CREATE UNIQUE INDEX one_role_per_group_per_user ON user_role +WHERE EXISTS ( + SELECT 1 + FROM role r1 + JOIN role r2 ON r1.id = user_role.role_id + WHERE r1."group" = r2."group" + AND r2.id IN ( + SELECT role_id + FROM user_role ur2 + WHERE ur2.user_id = user_role.user_id + ) +); diff --git a/src/model/family.rs b/src/model/family.rs index 894b082..6821d33 100644 --- a/src/model/family.rs +++ b/src/model/family.rs @@ -1,5 +1,7 @@ +use std::ops::DerefMut; + use serde::Serialize; -use sqlx::{sqlite::SqliteQueryResult, FromRow, SqlitePool}; +use sqlx::{sqlite::SqliteQueryResult, FromRow, Sqlite, SqlitePool, Transaction}; use super::user::User; @@ -22,6 +24,15 @@ impl Family { .unwrap() } + pub async fn insert_tx(db: &mut Transaction<'_, Sqlite>) -> i64 { + let result: SqliteQueryResult = sqlx::query("INSERT INTO family DEFAULT VALUES") + .execute(db.deref_mut()) + .await + .unwrap(); + + result.last_insert_rowid() + } + pub async fn insert(db: &SqlitePool) -> i64 { let result: SqliteQueryResult = sqlx::query("INSERT INTO family DEFAULT VALUES") .execute(db) diff --git a/src/model/role.rs b/src/model/role.rs index 8904860..df95157 100644 --- a/src/model/role.rs +++ b/src/model/role.rs @@ -7,11 +7,12 @@ use sqlx::{FromRow, Sqlite, SqlitePool, Transaction}; pub struct Role { pub(crate) id: i64, pub(crate) name: String, + pub(crate) cluster: Option, } impl Role { pub async fn all(db: &SqlitePool) -> Vec { - sqlx::query_as!(Role, "SELECT id, name FROM role") + sqlx::query_as!(Role, "SELECT id, name, cluster FROM role") .fetch_all(db) .await .unwrap() @@ -21,7 +22,7 @@ impl Role { sqlx::query_as!( Self, " -SELECT id, name +SELECT id, name, cluster FROM role WHERE id like ? ", @@ -31,12 +32,41 @@ WHERE id like ? .await .ok() } + pub async fn find_by_id_tx(db: &mut Transaction<'_, Sqlite>, name: i32) -> Option { + sqlx::query_as!( + Self, + " +SELECT id, name, cluster +FROM role +WHERE id like ? + ", + name + ) + .fetch_one(db.deref_mut()) + .await + .ok() + } + + pub async fn find_by_cluster_tx(db: &mut Transaction<'_, Sqlite>, name: i32) -> Option { + sqlx::query_as!( + Self, + " +SELECT id, name, cluster +FROM role +WHERE cluster = ? + ", + name + ) + .fetch_one(db.deref_mut()) + .await + .ok() + } pub async fn find_by_name(db: &SqlitePool, name: &str) -> Option { sqlx::query_as!( Self, " -SELECT id, name +SELECT id, name, cluster FROM role WHERE name like ? ", @@ -51,7 +81,7 @@ WHERE name like ? sqlx::query_as!( Self, " -SELECT id, name +SELECT id, name, cluster FROM role WHERE name like ? ", diff --git a/src/model/user.rs b/src/model/user.rs index c2bc150..9e9b8a3 100644 --- a/src/model/user.rs +++ b/src/model/user.rs @@ -490,6 +490,17 @@ ASKÖ Ruderverein Donau Linz", self.name), _ => true, } } + pub async fn has_membership_pdf_tx(&self, db: &mut Transaction<'_, Sqlite>) -> bool { + match sqlx::query_scalar!("SELECT membership_pdf FROM user WHERE id = ?", self.id) + .fetch_one(db.deref_mut()) + .await + .unwrap() + { + Some(a) if a.is_empty() => false, + None => false, + _ => true, + } + } pub async fn roles(&self, db: &SqlitePool) -> Vec { sqlx::query!( @@ -697,14 +708,16 @@ ORDER BY last_access DESC .is_ok() } - pub async fn update(&self, db: &SqlitePool, data: UserEditForm<'_>) { + pub async fn update(&self, db: &SqlitePool, data: UserEditForm<'_>) -> Result<(), String> { + let mut db = db.begin().await.map_err(|e| e.to_string())?; + let mut family_id = data.family_id; if family_id.is_some_and(|x| x == -1) { - family_id = Some(Family::insert(db).await) + family_id = Some(Family::insert_tx(&mut db).await) } - if !self.has_membership_pdf(db).await { + if !self.has_membership_pdf_tx(&mut db).await { if let Some(membership_pdf) = data.membership_pdf { let mut stream = membership_pdf.open().await.unwrap(); let mut buffer = Vec::new(); @@ -714,7 +727,7 @@ ORDER BY last_access DESC buffer, self.id ) - .execute(db) + .execute(db.deref_mut()) .await .unwrap(); //Okay, because we can only create a User of a valid id } @@ -735,28 +748,29 @@ ORDER BY last_access DESC family_id, self.id ) - .execute(db) + .execute(db.deref_mut()) .await .unwrap(); //Okay, because we can only create a User of a valid id // handle roles sqlx::query!("DELETE FROM user_role WHERE user_id = ?", self.id) - .execute(db) + .execute(db.deref_mut()) .await .unwrap(); for role_id in data.roles.into_keys() { - self.add_role( - db, - &Role::find_by_id(db, role_id.parse::().unwrap()) - .await - .unwrap(), - ) - .await; + let role = Role::find_by_id_tx(&mut db, role_id.parse::().unwrap()) + .await + .unwrap(); + self.add_role_tx(&mut db, &role).await?; } + + db.commit().await.map_err(|e| e.to_string())?; + + Ok(()) } - pub async fn add_role(&self, db: &SqlitePool, role: &Role) { + pub async fn add_role(&self, db: &SqlitePool, role: &Role) -> Result<(), String> { sqlx::query!( "INSERT INTO user_role(user_id, role_id) VALUES (?, ?)", self.id, @@ -764,7 +778,40 @@ ORDER BY last_access DESC ) .execute(db) .await - .unwrap(); + .map_err(|_| { + format!( + "User already has a role in the cluster '{}'", + role.cluster + .clone() + .expect("db trigger can't activate on empty string") + ) + })?; + + Ok(()) + } + + pub async fn add_role_tx( + &self, + db: &mut Transaction<'_, Sqlite>, + role: &Role, + ) -> Result<(), String> { + sqlx::query!( + "INSERT INTO user_role(user_id, role_id) VALUES (?, ?)", + self.id, + role.id + ) + .execute(db.deref_mut()) + .await + .map_err(|_| { + format!( + "User already has a role in the cluster '{}'", + role.cluster + .clone() + .expect("db trigger can't activate on empty string") + ) + })?; + + Ok(()) } pub async fn remove_role(&self, db: &SqlitePool, role: &Role) { diff --git a/src/tera/admin/user.rs b/src/tera/admin/user.rs index 9731054..3c7564d 100644 --- a/src/tera/admin/user.rs +++ b/src/tera/admin/user.rs @@ -200,7 +200,8 @@ async fn fees_paid( ) .await; user.add_role(db, &Role::find_by_name(db, "paid").await.unwrap()) - .await; + .await + .expect("paid role has no group"); } } @@ -305,9 +306,10 @@ async fn update( ); }; - user.update(db, data.into_inner()).await; - - Flash::success(Redirect::to("/admin/user"), "Successfully updated user") + match user.update(db, data.into_inner()).await { + Ok(_) => Flash::success(Redirect::to("/admin/user"), "Successfully updated user"), + Err(e) => Flash::error(Redirect::to("/admin/user"), e), + } } #[get("/user//membership")] @@ -394,7 +396,9 @@ async fn create_scheckbuch( // 4. Add 'scheckbuch' role let scheckbuch = Role::find_by_name(db, "scheckbuch").await.unwrap(); - user.add_role(db, &scheckbuch).await; + user.add_role(db, &scheckbuch) + .await + .expect("new user has no roles yet"); // 4. Send welcome mail (+ notification) user.send_welcome_email(db, &config.smtp_pw).await.unwrap(); @@ -434,10 +438,14 @@ async fn schnupper_to_scheckbuch( user.remove_role(db, &paid).await; let scheckbuch = Role::find_by_name(db, "scheckbuch").await.unwrap(); - user.add_role(db, &scheckbuch).await; + user.add_role(db, &scheckbuch) + .await + .expect("just removed 'schnupperant' thus can't have a role with that group"); if let Some(no_einschreibgebuehr) = Role::find_by_name(db, "no-einschreibgebuehr").await { - user.add_role(db, &no_einschreibgebuehr).await; + user.add_role(db, &no_einschreibgebuehr) + .await + .expect("role doesn't have a group"); } user.send_welcome_email(db, &config.smtp_pw).await.unwrap(); diff --git a/staging-diff.sql b/staging-diff.sql index 6fb21fc..09e7a30 100644 --- a/staging-diff.sql +++ b/staging-diff.sql @@ -3,3 +3,35 @@ INSERT INTO user(name) VALUES('Marie'); INSERT INTO "user_role" (user_id, role_id) VALUES((SELECT id from user where name = 'Marie'),(SELECT id FROM role where name = 'Donau Linz')); INSERT INTO user(name) VALUES('Philipp'); INSERT INTO "user_role" (user_id, role_id) VALUES((SELECT id from user where name = 'Philipp'),(SELECT id FROM role where name = 'Donau Linz')); + +ALTER TABLE "role" ADD COLUMN "cluster" text; +CREATE TRIGGER prevent_multiple_roles_same_cluster +BEFORE INSERT ON user_role +BEGIN + SELECT CASE + WHEN EXISTS ( + SELECT 1 + FROM user_role ur + JOIN role r1 ON ur.role_id = r1.id + JOIN role r2 ON r1."cluster" = r2."cluster" + WHERE ur.user_id = NEW.user_id + AND r2.id = NEW.role_id + AND r1.id != NEW.role_id + ) + THEN RAISE(ABORT, 'User already has a role in this cluster') + END; +END; + + +UPDATE role SET 'cluster'='skill' WHERE id=2; +UPDATE role SET 'cluster'='membership_type' WHERE id=3; +UPDATE role SET 'cluster'='skill' WHERE id=5; +UPDATE role SET 'cluster'='skill' WHERE id=6; +UPDATE role SET 'cluster'='membership_type' WHERE id=7; +UPDATE role SET 'cluster'='financial' WHERE id=8; +UPDATE role SET 'cluster'='membership_type' WHERE id=9; +UPDATE role SET 'cluster'='membership_type' WHERE id=14; +UPDATE role SET 'cluster'='financial' WHERE id=17; +UPDATE role SET 'cluster'='financial' WHERE id=18; +UPDATE role SET 'cluster'='membership_type' WHERE id=20; +UPDATE role SET 'cluster'='membership_type' WHERE id=22; -- 2.45.2 From 91bf71cf0027b6de3cb1438f45cd48bcb1d9f244 Mon Sep 17 00:00:00 2001 From: philipp Date: Fri, 11 Oct 2024 09:58:53 +0200 Subject: [PATCH 2/9] start working on frontend --- src/model/role.rs | 4 ++-- src/model/user.rs | 17 ++++++++++++++++- src/tera/admin/user.rs | 2 ++ templates/admin/user/index.html.tera | 20 ++++++++++++++++++-- 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/model/role.rs b/src/model/role.rs index df95157..c8accad 100644 --- a/src/model/role.rs +++ b/src/model/role.rs @@ -1,9 +1,9 @@ use std::ops::DerefMut; -use serde::Serialize; +use serde::{Deserialize, Serialize}; use sqlx::{FromRow, Sqlite, SqlitePool, Transaction}; -#[derive(FromRow, Serialize, Clone)] +#[derive(FromRow, Serialize, Clone, Deserialize, Debug)] pub struct Role { pub(crate) id: i64, pub(crate) name: String, diff --git a/src/model/user.rs b/src/model/user.rs index 9e9b8a3..72e801c 100644 --- a/src/model/user.rs +++ b/src/model/user.rs @@ -513,6 +513,21 @@ ASKÖ Ruderverein Donau Linz", self.name), .into_iter().map(|r| r.name).collect() } + pub async fn real_roles(&self, db: &SqlitePool) -> Vec { + sqlx::query_as!( + Role, + "SELECT r.id, r.name, r.cluster + FROM role r + JOIN user_role ur ON r.id = ur.role_id + JOIN user u ON u.id = ur.user_id + WHERE ur.user_id = ? AND u.deleted = 0;", + self.id + ) + .fetch_all(db) + .await + .unwrap() + } + pub async fn has_role_tx(&self, db: &mut Transaction<'_, Sqlite>, role: &str) -> bool { if sqlx::query!( "SELECT * FROM user_role WHERE user_id=? AND role_id = (SELECT id FROM role WHERE name = ?)", @@ -1154,7 +1169,7 @@ pub struct UserWithRolesAndMembershipPdf { #[serde(flatten)] pub user: User, pub membership_pdf: bool, - pub roles: Vec, + pub roles: Vec, // TODO: get rid of this... } impl UserWithRolesAndMembershipPdf { diff --git a/src/tera/admin/user.rs b/src/tera/admin/user.rs index 3c7564d..3cd9337 100644 --- a/src/tera/admin/user.rs +++ b/src/tera/admin/user.rs @@ -293,6 +293,8 @@ async fn update( data: Form>, admin: ManageUserUser, ) -> Flash { + println!("{data:#?}"); + let user = User::find_by_id(db, data.id).await; Log::create( db, diff --git a/templates/admin/user/index.html.tera b/templates/admin/user/index.html.tera index adf1026..82c2ebf 100644 --- a/templates/admin/user/index.html.tera +++ b/templates/admin/user/index.html.tera @@ -79,9 +79,25 @@
- {% for role in roles %} - {{ macros::checkbox(label=role.name, name="roles[" ~ role.id ~ "]", id=loop.index , checked=role.name in user.roles, disabled=allowed_to_edit == false) }} + {% for cluster, r in roles | group_by(attribute="cluster") %} + {{ cluster }} + {% endfor %} + + + {% for role in roles %} + {% if not role.cluster %} + {{ macros::checkbox(label=role.name, name="roles[" ~ role.id ~ "]", id=loop.index , checked=role.name in user.roles, disabled=allowed_to_edit == false) }} + {% endif %} + {% endfor %}
{% if user.membership_pdf %} Date: Fri, 11 Oct 2024 10:18:53 +0200 Subject: [PATCH 3/9] make it work with changed db model, but no select yet --- src/tera/admin/user.rs | 2 -- templates/admin/user/index.html.tera | 23 +++++------------------ 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/src/tera/admin/user.rs b/src/tera/admin/user.rs index 3cd9337..3c7564d 100644 --- a/src/tera/admin/user.rs +++ b/src/tera/admin/user.rs @@ -293,8 +293,6 @@ async fn update( data: Form>, admin: ManageUserUser, ) -> Flash { - println!("{data:#?}"); - let user = User::find_by_id(db, data.id).await; Log::create( db, diff --git a/templates/admin/user/index.html.tera b/templates/admin/user/index.html.tera index 82c2ebf..05cd9e5 100644 --- a/templates/admin/user/index.html.tera +++ b/templates/admin/user/index.html.tera @@ -79,25 +79,12 @@
- {% for cluster, r in roles | group_by(attribute="cluster") %} - {{ cluster }} - - {% endfor %} - - - {% for role in roles %} - {% if not role.cluster %} + {# for cluster, r in roles | group_by(attribute="cluster") #} + {# cluster #} + {% for role in roles %} {{ macros::checkbox(label=role.name, name="roles[" ~ role.id ~ "]", id=loop.index , checked=role.name in user.roles, disabled=allowed_to_edit == false) }} - {% endif %} - {% endfor %} + {% endfor %} + {# endfor #}
{% if user.membership_pdf %} Date: Fri, 11 Oct 2024 10:29:13 +0200 Subject: [PATCH 4/9] fix backend tests --- migration.sql | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/migration.sql b/migration.sql index c2b105d..37bc668 100644 --- a/migration.sql +++ b/migration.sql @@ -222,15 +222,19 @@ CREATE TABLE IF NOT EXISTS "distance" ( ); -CREATE UNIQUE INDEX one_role_per_group_per_user ON user_role -WHERE EXISTS ( - SELECT 1 - FROM role r1 - JOIN role r2 ON r1.id = user_role.role_id - WHERE r1."group" = r2."group" - AND r2.id IN ( - SELECT role_id - FROM user_role ur2 - WHERE ur2.user_id = user_role.user_id - ) -); +CREATE TRIGGER prevent_multiple_roles_same_cluster +BEFORE INSERT ON user_role +BEGIN + SELECT CASE + WHEN EXISTS ( + SELECT 1 + FROM user_role ur + JOIN role r1 ON ur.role_id = r1.id + JOIN role r2 ON r1."cluster" = r2."cluster" + WHERE ur.user_id = NEW.user_id + AND r2.id = NEW.role_id + AND r1.id != NEW.role_id + ) + THEN RAISE(ABORT, 'User already has a role in this cluster') + END; +END; -- 2.45.2 From 24a874f888f3f5185c689769a9261b6893b80cf7 Mon Sep 17 00:00:00 2001 From: philipp Date: Fri, 11 Oct 2024 10:30:40 +0200 Subject: [PATCH 5/9] be more thorough in tests --- src/model/user.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/model/user.rs b/src/model/user.rs index 72e801c..711bdbf 100644 --- a/src/model/user.rs +++ b/src/model/user.rs @@ -1296,7 +1296,8 @@ mod test { membership_pdf: None, }, ) - .await; + .await + .unwrap(); let user = User::find_by_id(&pool, 1).await.unwrap(); -- 2.45.2 From ca51ba420c0cf260a196e01cf2590d4b83643505 Mon Sep 17 00:00:00 2001 From: philipp Date: Fri, 11 Oct 2024 10:34:27 +0200 Subject: [PATCH 6/9] update docs to account for new db --- doc/db/README.md | 1 + doc/db/user.mermaid | 1 + 2 files changed, 2 insertions(+) diff --git a/doc/db/README.md b/doc/db/README.md index dc2ac23..90ffc4d 100644 --- a/doc/db/README.md +++ b/doc/db/README.md @@ -9,6 +9,7 @@ Thus, here is the current (October '24) model and the reasoning behind it: - All user-relevant fields are stored in `User`. - `Role` (and its associative table `UserRole`) map current roles the user has. This is used for e.g. permissions (`Vorstand`, `Admin`, `cox`, ... roles) and fee calculation (`Donau Linz`, `scheckbuch`, `Rennjugend`). - `Family` specifies, well, a family. Currently only used for fee calculation. +- `cluster` in `Role` groups roles together. There is a db check to only allow for at most 1 role of the same cluster (e.g. either `cox` or `bootsfuehrer`, but not both). ## Planned rowing adventures :-) ![](./planned.svg) diff --git a/doc/db/user.mermaid b/doc/db/user.mermaid index 85dbac6..31d3200 100644 --- a/doc/db/user.mermaid +++ b/doc/db/user.mermaid @@ -29,6 +29,7 @@ classDiagram class Role { +int id +string name + +string cluster } class UserRole { -- 2.45.2 From 0057a58eff75ea5a66386b8f86209a8468f18267 Mon Sep 17 00:00:00 2001 From: philipp Date: Fri, 11 Oct 2024 10:35:08 +0200 Subject: [PATCH 7/9] add new db plot --- doc/db/user.svg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/db/user.svg b/doc/db/user.svg index f635599..b1c18f8 100644 --- a/doc/db/user.svg +++ b/doc/db/user.svg @@ -1 +1 @@ -
1
*
1
*
1
0..1
User
+int id
+string name
+string pw
+bool deleted
+datetime last_access
+string dob
+string weight
+string sex
+string dirty_thirty
+string dirty_dozen
+string member_since_date
+string birthdate
+string mail
+string nickname
+string notes
+string phone
+string address
+int family_id
+blob membership_pdf
+string user_token
Family
+int id
Role
+int id
+string name
UserRole
+int user_id
+int role_id
 \ No newline at end of file +
1
*
1
*
1
0..1
User
+int id
+string name
+string pw
+bool deleted
+datetime last_access
+string dob
+string weight
+string sex
+string dirty_thirty
+string dirty_dozen
+string member_since_date
+string birthdate
+string mail
+string nickname
+string notes
+string phone
+string address
+int family_id
+blob membership_pdf
+string user_token
Family
+int id
Role
+int id
+string name
+string cluster
UserRole
+int user_id
+int role_id
 \ No newline at end of file -- 2.45.2 From aae206d8d9823694cc6ccf886f3135496b446edd Mon Sep 17 00:00:00 2001 From: philipp Date: Fri, 11 Oct 2024 10:36:03 +0200 Subject: [PATCH 8/9] remove unnecessary comment --- src/model/user.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/model/user.rs b/src/model/user.rs index 711bdbf..24127a9 100644 --- a/src/model/user.rs +++ b/src/model/user.rs @@ -1169,7 +1169,7 @@ pub struct UserWithRolesAndMembershipPdf { #[serde(flatten)] pub user: User, pub membership_pdf: bool, - pub roles: Vec, // TODO: get rid of this... + pub roles: Vec, } impl UserWithRolesAndMembershipPdf { -- 2.45.2 From 10d220773bacbabb81d017dd712937b44345a985 Mon Sep 17 00:00:00 2001 From: philipp Date: Fri, 11 Oct 2024 11:09:23 +0200 Subject: [PATCH 9/9] fix ci --- migration.sql | 2 +- staging-diff.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/migration.sql b/migration.sql index 37bc668..365aa72 100644 --- a/migration.sql +++ b/migration.sql @@ -222,7 +222,7 @@ CREATE TABLE IF NOT EXISTS "distance" ( ); -CREATE TRIGGER prevent_multiple_roles_same_cluster +CREATE TRIGGER IF NOT EXISTS prevent_multiple_roles_same_cluster BEFORE INSERT ON user_role BEGIN SELECT CASE diff --git a/staging-diff.sql b/staging-diff.sql index 09e7a30..e4387f2 100644 --- a/staging-diff.sql +++ b/staging-diff.sql @@ -5,7 +5,7 @@ INSERT INTO user(name) VALUES('Philipp'); INSERT INTO "user_role" (user_id, role_id) VALUES((SELECT id from user where name = 'Philipp'),(SELECT id FROM role where name = 'Donau Linz')); ALTER TABLE "role" ADD COLUMN "cluster" text; -CREATE TRIGGER prevent_multiple_roles_same_cluster +CREATE TRIGGER IF NOT EXISTS prevent_multiple_roles_same_cluster BEFORE INSERT ON user_role BEGIN SELECT CASE -- 2.45.2