From c4a73204aa99c5897b06ac9a90ea4e6f5b729c76 Mon Sep 17 00:00:00 2001 From: Simon Bruder Date: Wed, 10 Jul 2024 01:14:49 +0200 Subject: [PATCH] Make item name optional This vastly changes how item names are displayed. To make this more ergonomic, it adds some helper macros. This also aids in adhering to DRY. --- .../down.sql | 13 +++++++ .../up.sql | 13 +++++++ src/frontend/item.rs | 39 ++++++++++++++++++- src/models.rs | 12 +++--- src/schema.rs | 2 +- templates/item_add.html | 7 +++- templates/item_details.html | 14 +++---- templates/item_edit.html | 10 ++++- templates/item_list.html | 17 +++----- templates/macros.html | 36 +++++++++++++++++ 10 files changed, 131 insertions(+), 32 deletions(-) create mode 100644 migrations/2024-07-09-133113_item_make_name_optional/down.sql create mode 100644 migrations/2024-07-09-133113_item_make_name_optional/up.sql create mode 100644 templates/macros.html diff --git a/migrations/2024-07-09-133113_item_make_name_optional/down.sql b/migrations/2024-07-09-133113_item_make_name_optional/down.sql new file mode 100644 index 0000000..995d10e --- /dev/null +++ b/migrations/2024-07-09-133113_item_make_name_optional/down.sql @@ -0,0 +1,13 @@ +-- SPDX-FileCopyrightText: 2024 Simon Bruder +-- +-- SPDX-License-Identifier: AGPL-3.0-or-later + +ALTER TABLE items + DROP CONSTRAINT name_not_empty; + +UPDATE items + SET name = '' + WHERE NAME IS NULL; + +ALTER TABLE items + ALTER name SET NOT NULL; diff --git a/migrations/2024-07-09-133113_item_make_name_optional/up.sql b/migrations/2024-07-09-133113_item_make_name_optional/up.sql new file mode 100644 index 0000000..079f952 --- /dev/null +++ b/migrations/2024-07-09-133113_item_make_name_optional/up.sql @@ -0,0 +1,13 @@ +-- SPDX-FileCopyrightText: 2024 Simon Bruder +-- +-- SPDX-License-Identifier: AGPL-3.0-or-later + +ALTER TABLE items + ALTER name DROP NOT NULL; + +UPDATE items + SET name = NULL + WHERE name = ''; + +ALTER TABLE items + ADD CONSTRAINT name_not_empty CHECK(name <> ''); diff --git a/src/frontend/item.rs b/src/frontend/item.rs index 61e6f85..3a34360 100644 --- a/src/frontend/item.rs +++ b/src/frontend/item.rs @@ -27,6 +27,7 @@ struct ItemDetails { req: HttpRequest, item: Item, item_class: ItemClass, + item_classes: HashMap, parents: Vec, children: Vec, } @@ -43,10 +44,13 @@ async fn show_item( .await .map_err(error::ErrorInternalServerError)?; - let item_class = manage::item_class::get(&mut pool.get().await.unwrap(), item.class) + let item_classes = manage::item_class::get_all_as_map(&mut pool.get().await.unwrap()) .await .map_err(error::ErrorInternalServerError)?; + // TODO: remove clone (should be possible without it) + let item_class = item_classes.get(&item.class).unwrap().clone(); + let parents = manage::item::get_parents_details(&mut pool.get().await.unwrap(), item.id) .await .map_err(error::ErrorInternalServerError)?; @@ -59,6 +63,7 @@ async fn show_item( req, item, item_class, + item_classes, parents, children, }) @@ -71,9 +76,13 @@ struct ItemList { // Both a Vec and a HashMap are used to have both the natural order, // as well as arbitrary access capabilities. item_list: Vec, + #[allow(dead_code)] // remove once item_parents can be constructed in the template items: HashMap, item_classes: HashMap, + #[allow(dead_code)] // remove once item_parents can be constructed in the template item_tree: HashMap>, + // to overcome askama’s lack of support for closures + item_parents: HashMap>, } #[get("/items")] @@ -97,12 +106,29 @@ async fn list_items( .await .map_err(error::ErrorInternalServerError)?; + // TODO: remove clone (should be possible without it) + let item_parents = items + .clone() + .into_iter() + .map(|(id, item)| { + (id, { + item_tree + .get(&item.id) + .unwrap() + .iter() + .map(|is| items.get(is).unwrap().clone()) + .collect() + }) + }) + .collect(); + Ok(ItemList { req, item_list, items, item_classes, item_tree, + item_parents, }) } @@ -134,6 +160,7 @@ async fn add_item_post( struct ItemEditForm { req: HttpRequest, item: Item, + item_class: ItemClass, } #[get("/item/{id}/edit")] @@ -148,7 +175,15 @@ async fn edit_item( .await .map_err(error::ErrorInternalServerError)?; - Ok(ItemEditForm { req, item }) + let item_class = manage::item_class::get(&mut pool.get().await.unwrap(), item.class) + .await + .map_err(error::ErrorInternalServerError)?; + + Ok(ItemEditForm { + req, + item, + item_class, + }) } #[post("/item/{id}/edit")] diff --git a/src/models.rs b/src/models.rs index 7949e88..8d74f87 100644 --- a/src/models.rs +++ b/src/models.rs @@ -11,12 +11,12 @@ use uuid::Uuid; use crate::schema::*; -#[derive(Debug, Queryable, Selectable, Insertable, Serialize)] +#[derive(Clone, Debug, Queryable, Selectable, Insertable, Serialize)] #[diesel(table_name = items)] #[diesel(check_for_backend(diesel::pg::Pg))] pub struct Item { pub id: Uuid, - pub name: String, + pub name: Option, pub parent: Option, pub class: Uuid, } @@ -24,14 +24,16 @@ pub struct Item { #[derive(Debug, Insertable, Deserialize, AsChangeset)] #[diesel(table_name = items)] #[diesel(check_for_backend(diesel::pg::Pg))] +#[diesel(treat_none_as_null = true)] pub struct NewItem { - pub name: String, + #[serde(default)] + pub name: Option, #[serde(default)] pub parent: Option, pub class: Uuid, } -#[derive(Debug, DbEnum, PartialEq, Deserialize, Serialize)] +#[derive(Clone, Debug, DbEnum, PartialEq, Deserialize, Serialize)] #[ExistingTypePath = "sql_types::ItemClassType"] #[serde(rename_all = "snake_case")] pub enum ItemClassType { @@ -54,7 +56,7 @@ impl fmt::Display for ItemClassType { } } -#[derive(Debug, Queryable, Selectable, Insertable, Serialize)] +#[derive(Clone, Debug, Queryable, Selectable, Insertable, Serialize)] #[diesel(table_name = item_classes)] #[diesel(check_for_backend(diesel::pg::Pg))] pub struct ItemClass { diff --git a/src/schema.rs b/src/schema.rs index ab62ec5..9803c17 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -11,7 +11,7 @@ pub mod sql_types { diesel::table! { items (id) { id -> Uuid, - name -> Varchar, + name -> Nullable, parent -> Nullable, class -> Uuid, } diff --git a/templates/item_add.html b/templates/item_add.html index c31cb52..1fdf1d8 100644 --- a/templates/item_add.html +++ b/templates/item_add.html @@ -10,7 +10,12 @@ SPDX-License-Identifier: AGPL-3.0-or-later
- +
+
+ +
+ +
diff --git a/templates/item_details.html b/templates/item_details.html index 67f4263..b8ca08b 100644 --- a/templates/item_details.html +++ b/templates/item_details.html @@ -4,8 +4,9 @@ SPDX-FileCopyrightText: 2024 Simon Bruder SPDX-License-Identifier: AGPL-3.0-or-later #} +{%- import "macros.html" as macros -%} {% extends "base.html" %} -{% block title %}{% block page_title %}{{ item.name }}{% endblock %} – Item Details – {{ branding }}{% endblock %} +{% block title %}{% block page_title %}{% call macros::item_name(item, item_class, false) %}{% endblock %} – Item Details – {{ branding }}{% endblock %} {% block page_actions %} Edit {% endblock %} @@ -23,7 +24,7 @@ SPDX-License-Identifier: AGPL-3.0-or-later Name - {{ item.name }} + {% call macros::item_name_terse(item, true) %} @@ -39,12 +40,7 @@ SPDX-License-Identifier: AGPL-3.0-or-later Parents -
    - {%- for parent in parents %} -
  1. {{ parent.name }}
    2. - {%- endfor %} -
  2. {{ item.name }}
    3. -
+ {%- call macros::parents_breadcrumb(item, parents, item_classes, full=true) %} @@ -55,7 +51,7 @@ SPDX-License-Identifier: AGPL-3.0-or-later {% endif %} diff --git a/templates/item_edit.html b/templates/item_edit.html index 5c1efdc..791e37e 100644 --- a/templates/item_edit.html +++ b/templates/item_edit.html @@ -4,8 +4,9 @@ SPDX-FileCopyrightText: 2024 Simon Bruder SPDX-License-Identifier: AGPL-3.0-or-later #} +{%- import "macros.html" as macros -%} {% extends "base.html" %} -{% block title %}{% block page_title %}{{ item.name }}{% endblock %} – Edit Item – {{ branding }}{% endblock %} +{% block title %}{% block page_title %}{% call macros::item_name(item, item_class, false) %}{% endblock %} – Edit Item – {{ branding }}{% endblock %} {% block main %}
@@ -14,7 +15,12 @@ SPDX-License-Identifier: AGPL-3.0-or-later
- +
+
+ +
+ +
diff --git a/templates/item_list.html b/templates/item_list.html index 0792a44..08a1ddf 100644 --- a/templates/item_list.html +++ b/templates/item_list.html @@ -4,6 +4,7 @@ SPDX-FileCopyrightText: 2024 Simon Bruder SPDX-License-Identifier: AGPL-3.0-or-later #} +{%- import "macros.html" as macros -%} {% extends "base.html" %} {% block title %}{% block page_title %}Item List{% endblock %} – {{ branding }}{% endblock %} {% block page_actions %} @@ -21,21 +22,13 @@ SPDX-License-Identifier: AGPL-3.0-or-later {% for item in item_list -%} {% let class = item_classes.get(item.class).unwrap() %} + {# inlining this breaks? #} + {%- let parents = item_parents.get(item.id).unwrap() %} - {{ item.name }} + {% call macros::item_name_terse(item, true) %} {{ class.name }} -
    - {%- let parents = item_tree.get(item.id).unwrap() -%} - {%- if parents.len() > 3 %} -
  1. …
    2. - {%- endif %} - {%- for parent in parents.iter().rev().take(3).rev() %} - {%- let parent = items.get(parent).unwrap() %} -
  2. {{ parent.name }}
    3. - {%- endfor %} -
  3. {{ item.name }}
    4. -
+ {%- call macros::parents_breadcrumb(item, parents, item_classes, full=false) %} {% endfor -%} diff --git a/templates/macros.html b/templates/macros.html new file mode 100644 index 0000000..e2c90ec --- /dev/null +++ b/templates/macros.html @@ -0,0 +1,36 @@ +{# +SPDX-FileCopyrightText: 2024 Simon Bruder + +SPDX-License-Identifier: AGPL-3.0-or-later +#} + +{% macro emphasize(text, html) %} +{%- if html %}{{ text }}{% else %}*{{ text }}*{% endif %} +{%- endmacro %} + +{% macro item_name_generic(name, fallback, html) %} +{%- if let Some(name) = name %}{{ name }}{% else %}{% call emphasize(fallback, html) %}{% endif %} +{%- endmacro %} + +{% macro item_name(item, item_class, html) %} +{%- call item_name_generic(item.name, item_class.name, html) %} +{%- endmacro %} + +{% macro item_name_terse(item, html) %} +{%- call item_name_generic(item.name, "[no name]", html) %} +{%- endmacro %} + +{% macro parents_breadcrumb(item, parents, parents_item_classes, full) %} +{%- let limit = 3 %} +{%- let limited = parents.len() > limit && !full %} +{%- let parents = parents.iter().rev().take(limit.into()).rev() %} +
    + {%- if limited %} +
  1. …
    2. + {%- endif %} + {%- for parent in parents %} +
  2. {% call item_name(parent, parents_item_classes.get(parent.class).unwrap(), true) %}
    3. + {%- endfor %} +
  3. {% call item_name(item, item_classes.get(item.class).unwrap(), true) %}
    4. +
+{%- endmacro %}