From 6251dea6a1c331b9ab8536f84f0e71db2a6b477e Mon Sep 17 00:00:00 2001 From: Simon Bruder Date: Thu, 11 Jul 2024 13:25:09 +0200 Subject: [PATCH] Simplify item class model Whether an item class is generic or specific can be deduced from whether a parent exists or not. While the SQL migration (especially the down direction) is quite complex, it simplifies the handling quite a bit. --- .../down.sql | 42 ++++++++++++ .../up.sql | 34 ++++++++++ src/frontend/item_class.rs | 67 +++++-------------- src/frontend/templates/helpers.rs | 1 + src/models.rs | 28 -------- src/schema.rs | 10 --- 6 files changed, 92 insertions(+), 90 deletions(-) create mode 100644 migrations/2024-07-11-110525_item_class_simplify/down.sql create mode 100644 migrations/2024-07-11-110525_item_class_simplify/up.sql diff --git a/migrations/2024-07-11-110525_item_class_simplify/down.sql b/migrations/2024-07-11-110525_item_class_simplify/down.sql new file mode 100644 index 0000000..35808a9 --- /dev/null +++ b/migrations/2024-07-11-110525_item_class_simplify/down.sql @@ -0,0 +1,42 @@ +-- SPDX-FileCopyrightText: 2024 Simon Bruder +-- +-- SPDX-License-Identifier: AGPL-3.0-or-later + +DROP TRIGGER prevent_item_class_recursion ON item_classes; +DROP FUNCTION check_item_class_recursion_depth; + +CREATE TYPE item_class_type AS ENUM ('generic', 'specific'); + +ALTER TABLE item_classes + ADD type item_class_type; + +UPDATE item_classes SET type='generic' WHERE parent IS NULL; +UPDATE item_classes SET type='specific' WHERE parent IS NOT NULL; + +ALTER TABLE item_classes + ALTER type SET NOT NULL; +ALTER TABLE item_classes + ALTER type SET DEFAULT 'generic'; + +ALTER TABLE item_classes + ADD CONSTRAINT parent_only_for_specific CHECK (type = 'generic' AND parent IS NULL OR type = 'specific' AND parent IS NOT NULL); + +CREATE FUNCTION check_item_class_parent() +RETURNS TRIGGER AS $$ +BEGIN + IF NEW.parent IS NULL THEN + RETURN NEW; + END IF; + + IF (SELECT type FROM item_classes WHERE id = NEW.parent) = 'generic' THEN + RETURN NEW; + END IF; + + RAISE EXCEPTION 'Specific item classes may only have a generic parent (to avoid recursion)'; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER prevent_item_class_recursion +BEFORE INSERT OR UPDATE ON item_classes +FOR EACH ROW +EXECUTE FUNCTION check_item_class_parent(); diff --git a/migrations/2024-07-11-110525_item_class_simplify/up.sql b/migrations/2024-07-11-110525_item_class_simplify/up.sql new file mode 100644 index 0000000..207ee3f --- /dev/null +++ b/migrations/2024-07-11-110525_item_class_simplify/up.sql @@ -0,0 +1,34 @@ +-- SPDX-FileCopyrightText: 2024 Simon Bruder +-- +-- SPDX-License-Identifier: AGPL-3.0-or-later + +DROP TRIGGER prevent_item_class_recursion ON item_classes; +DROP FUNCTION check_item_class_parent; + +ALTER TABLE item_classes + DROP CONSTRAINT parent_only_for_specific; + +ALTER TABLE item_classes + DROP type; + +DROP TYPE item_class_type; + +CREATE FUNCTION check_item_class_recursion_depth() +RETURNS TRIGGER AS $$ +BEGIN + IF NEW.parent IS NULL THEN + RETURN NEW; + END IF; + + IF (SELECT parent FROM item_classes WHERE id = NEW.parent) IS NULL THEN + RETURN NEW; + END IF; + + RAISE EXCEPTION 'Item classes may only be nested one level deep'; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER prevent_item_class_recursion +BEFORE INSERT OR UPDATE ON item_classes +FOR EACH ROW +EXECUTE FUNCTION check_item_class_recursion_depth(); diff --git a/src/frontend/item_class.rs b/src/frontend/item_class.rs index f4cda33..5a9c3bf 100644 --- a/src/frontend/item_class.rs +++ b/src/frontend/item_class.rs @@ -11,26 +11,6 @@ use crate::manage; use crate::models::*; use crate::DbPool; -const FORM_ENSURE_PARENT: templates::helpers::Js = templates::helpers::Js::Inline( - r#" -(() => { - document.getElementById("type").addEventListener("change", e => { - let parentInput = document.getElementById("parent") - switch (e.target.value) { - case "generic": - parentInput.disabled = true - parentInput.value = "" - break - case "specific": - parentInput.disabled = false - break - default: - console.error("invalid type!") - } - }) -})()"#, -); - pub fn config(cfg: &mut web::ServiceConfig) { cfg.service(show_item_class) .service(list_item_classes) @@ -86,10 +66,6 @@ async fn show_item_class( th { "Name" } td { (item_class.name) } } - tr { - th { "Type" } - td { (item_class.r#type) } - } @if let Some(parent) = parent { tr { th { "Parent" } @@ -155,7 +131,6 @@ async fn add_item_class() -> actix_web::Result { path: "/items-classes/add", title: Some("Add Item Class"), page_title: Some(Box::new("Add Item Class")), - extra_js: vec![FORM_ENSURE_PARENT], ..Default::default() }, html! { @@ -167,19 +142,13 @@ async fn add_item_class() -> actix_web::Result { required: true, ..Default::default() }) - // TODO: drop type in favour of determining it on whether parent is set - .mb-3 { - label .form-label for="type" { "Type" } - select .form-select #type name="type" required { - @for variant in ItemClassType::VARIANTS { - option { (variant) } - } - } - } - .mb-3 { - label .form-label for="parent" { "Parent" } - input .form-control #parent type="text" name="parent" disabled; - } + (forms::InputGroup { + r#type: forms::InputType::Text, + name: "parent", + title: "Parent", + disabled: true, + ..Default::default() + }) button .btn.btn-primary type="submit" { "Add" } } @@ -217,7 +186,6 @@ async fn edit_item_class( path: &format!("/items-class/{}/add", id), title: Some(&title), page_title: Some(Box::new(item_class.name.clone())), - extra_js: vec![FORM_ENSURE_PARENT], ..Default::default() }, html! { @@ -238,19 +206,14 @@ async fn edit_item_class( value: Some(&item_class.name), ..Default::default() }) - // TODO: drop type in favour of determining it on whether parent is set - .mb-3 { - label .form-label for="type" { "Type" } - select .form-select #type name="type" required { - @for variant in ItemClassType::VARIANTS { - option selected[variant == item_class.r#type] { (variant) } - } - } - } - .mb-3 { - label .form-label for="parent" { "Parent" } - input .form-control #parent type="text" name="parent" disabled[item_class.parent.is_none()] value=[item_class.parent]; - } + (forms::InputGroup { + r#type: forms::InputType::Text, + name: "parent", + title: "Parent", + disabled: item_class.parent.is_none(), + value: item_class.parent.map(|id| id.to_string()).as_deref(), + ..Default::default() + }) button .btn.btn-primary type="submit" { "Edit" } } diff --git a/src/frontend/templates/helpers.rs b/src/frontend/templates/helpers.rs index 0b7238f..212cd42 100644 --- a/src/frontend/templates/helpers.rs +++ b/src/frontend/templates/helpers.rs @@ -32,6 +32,7 @@ impl Render for Css<'_> { pub enum Js<'a> { File(&'a str), + #[allow(dead_code)] Inline(&'a str), } diff --git a/src/models.rs b/src/models.rs index 6fe80b4..5a662b2 100644 --- a/src/models.rs +++ b/src/models.rs @@ -2,10 +2,7 @@ // // SPDX-License-Identifier: AGPL-3.0-or-later -use std::fmt; - use diesel::prelude::*; -use diesel_derive_enum::DbEnum; use serde::{Deserialize, Serialize}; use uuid::Uuid; @@ -33,36 +30,12 @@ pub struct NewItem { pub class: Uuid, } -#[derive(Clone, Debug, DbEnum, PartialEq, Deserialize, Serialize)] -#[ExistingTypePath = "sql_types::ItemClassType"] -#[serde(rename_all = "snake_case")] -pub enum ItemClassType { - Generic, - Specific, -} - -impl ItemClassType { - pub const VARIANTS: [ItemClassType; 2] = [Self::Generic, Self::Specific]; -} - -impl fmt::Display for ItemClassType { - // FIXME: currently, the HTML form relies on this matching the serde serializsation. - // This should not be the case. - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Generic => write!(f, "generic"), - Self::Specific => write!(f, "specific"), - } - } -} - #[derive(Clone, Debug, Queryable, Selectable, Insertable, Serialize)] #[diesel(table_name = item_classes)] #[diesel(check_for_backend(diesel::pg::Pg))] pub struct ItemClass { pub id: Uuid, pub name: String, - pub r#type: ItemClassType, #[serde(skip_serializing_if = "Option::is_none")] pub parent: Option, } @@ -73,7 +46,6 @@ pub struct ItemClass { #[diesel(treat_none_as_null = true)] pub struct NewItemClass { pub name: String, - pub r#type: ItemClassType, #[serde(default)] pub parent: Option, } diff --git a/src/schema.rs b/src/schema.rs index 9803c17..61e7195 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -2,12 +2,6 @@ // // SPDX-License-Identifier: AGPL-3.0-or-later -pub mod sql_types { - #[derive(diesel::query_builder::QueryId, diesel::sql_types::SqlType)] - #[diesel(postgres_type(name = "item_class_type"))] - pub struct ItemClassType; -} - diesel::table! { items (id) { id -> Uuid, @@ -25,13 +19,9 @@ diesel::table! { } diesel::table! { - use diesel::sql_types::*; - use super::sql_types::ItemClassType; - item_classes (id) { id -> Uuid, name -> Varchar, - r#type -> ItemClassType, parent -> Nullable, } }