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.
This commit is contained in:
Simon Bruder 2024-07-11 13:25:09 +02:00
parent 4a2dc721d4
commit de65452a01
Signed by: simon
GPG key ID: 347FF8699CDA0776
6 changed files with 92 additions and 90 deletions

View file

@ -0,0 +1,42 @@
-- SPDX-FileCopyrightText: 2024 Simon Bruder <simon@sbruder.de>
--
-- 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();

View file

@ -0,0 +1,34 @@
-- SPDX-FileCopyrightText: 2024 Simon Bruder <simon@sbruder.de>
--
-- 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();

View file

@ -11,26 +11,6 @@ use crate::manage;
use crate::models::*; use crate::models::*;
use crate::DbPool; 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) { pub fn config(cfg: &mut web::ServiceConfig) {
cfg.service(show_item_class) cfg.service(show_item_class)
.service(list_item_classes) .service(list_item_classes)
@ -86,10 +66,6 @@ async fn show_item_class(
th { "Name" } th { "Name" }
td { (item_class.name) } td { (item_class.name) }
} }
tr {
th { "Type" }
td { (item_class.r#type) }
}
@if let Some(parent) = parent { @if let Some(parent) = parent {
tr { tr {
th { "Parent" } th { "Parent" }
@ -155,7 +131,6 @@ async fn add_item_class() -> actix_web::Result<impl Responder> {
path: "/items-classes/add", path: "/items-classes/add",
title: Some("Add Item Class"), title: Some("Add Item Class"),
page_title: Some(Box::new("Add Item Class")), page_title: Some(Box::new("Add Item Class")),
extra_js: vec![FORM_ENSURE_PARENT],
..Default::default() ..Default::default()
}, },
html! { html! {
@ -167,19 +142,13 @@ async fn add_item_class() -> actix_web::Result<impl Responder> {
required: true, required: true,
..Default::default() ..Default::default()
}) })
// TODO: drop type in favour of determining it on whether parent is set (forms::InputGroup {
.mb-3 { r#type: forms::InputType::Text,
label .form-label for="type" { "Type" } name: "parent",
select .form-select #type name="type" required { title: "Parent",
@for variant in ItemClassType::VARIANTS { disabled: true,
option { (variant) } ..Default::default()
} })
}
}
.mb-3 {
label .form-label for="parent" { "Parent" }
input .form-control #parent type="text" name="parent" disabled;
}
button .btn.btn-primary type="submit" { "Add" } button .btn.btn-primary type="submit" { "Add" }
} }
@ -217,7 +186,6 @@ async fn edit_item_class(
path: &format!("/items-class/{}/add", id), path: &format!("/items-class/{}/add", id),
title: Some(&title), title: Some(&title),
page_title: Some(Box::new(item_class.name.clone())), page_title: Some(Box::new(item_class.name.clone())),
extra_js: vec![FORM_ENSURE_PARENT],
..Default::default() ..Default::default()
}, },
html! { html! {
@ -238,19 +206,14 @@ async fn edit_item_class(
value: Some(&item_class.name), value: Some(&item_class.name),
..Default::default() ..Default::default()
}) })
// TODO: drop type in favour of determining it on whether parent is set (forms::InputGroup {
.mb-3 { r#type: forms::InputType::Text,
label .form-label for="type" { "Type" } name: "parent",
select .form-select #type name="type" required { title: "Parent",
@for variant in ItemClassType::VARIANTS { disabled: item_class.parent.is_none(),
option selected[variant == item_class.r#type] { (variant) } value: item_class.parent.map(|id| id.to_string()).as_deref(),
} ..Default::default()
} })
}
.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];
}
button .btn.btn-primary type="submit" { "Edit" } button .btn.btn-primary type="submit" { "Edit" }
} }

View file

@ -32,6 +32,7 @@ impl Render for Css<'_> {
pub enum Js<'a> { pub enum Js<'a> {
File(&'a str), File(&'a str),
#[allow(dead_code)]
Inline(&'a str), Inline(&'a str),
} }

View file

@ -2,10 +2,7 @@
// //
// SPDX-License-Identifier: AGPL-3.0-or-later // SPDX-License-Identifier: AGPL-3.0-or-later
use std::fmt;
use diesel::prelude::*; use diesel::prelude::*;
use diesel_derive_enum::DbEnum;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use uuid::Uuid; use uuid::Uuid;
@ -33,36 +30,12 @@ pub struct NewItem {
pub class: Uuid, 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)] #[derive(Clone, Debug, Queryable, Selectable, Insertable, Serialize)]
#[diesel(table_name = item_classes)] #[diesel(table_name = item_classes)]
#[diesel(check_for_backend(diesel::pg::Pg))] #[diesel(check_for_backend(diesel::pg::Pg))]
pub struct ItemClass { pub struct ItemClass {
pub id: Uuid, pub id: Uuid,
pub name: String, pub name: String,
pub r#type: ItemClassType,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
pub parent: Option<Uuid>, pub parent: Option<Uuid>,
} }
@ -73,7 +46,6 @@ pub struct ItemClass {
#[diesel(treat_none_as_null = true)] #[diesel(treat_none_as_null = true)]
pub struct NewItemClass { pub struct NewItemClass {
pub name: String, pub name: String,
pub r#type: ItemClassType,
#[serde(default)] #[serde(default)]
pub parent: Option<Uuid>, pub parent: Option<Uuid>,
} }

View file

@ -2,12 +2,6 @@
// //
// SPDX-License-Identifier: AGPL-3.0-or-later // 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! { diesel::table! {
items (id) { items (id) {
id -> Uuid, id -> Uuid,
@ -25,13 +19,9 @@ diesel::table! {
} }
diesel::table! { diesel::table! {
use diesel::sql_types::*;
use super::sql_types::ItemClassType;
item_classes (id) { item_classes (id) {
id -> Uuid, id -> Uuid,
name -> Varchar, name -> Varchar,
r#type -> ItemClassType,
parent -> Nullable<Uuid>, parent -> Nullable<Uuid>,
} }
} }