From 77ac9d6a5c645224e81ee0681c4bd0b6d7d96038 Mon Sep 17 00:00:00 2001 From: Donkie Date: Sat, 6 Jan 2024 16:22:35 +0100 Subject: [PATCH 01/18] Added custom extra fields to backend No integration tests yet, TBD --- ...04_2209-b8881bdb716c_added_extra_fields.py | 74 +++++++ spoolman/api/v1/field.py | 98 +++++++++ spoolman/api/v1/filament.py | 41 +++- spoolman/api/v1/models.py | 21 ++ spoolman/api/v1/router.py | 3 +- spoolman/api/v1/spool.py | 23 +- spoolman/api/v1/vendor.py | 41 +++- spoolman/database/filament.py | 6 +- spoolman/database/models.py | 45 +++- spoolman/database/spool.py | 6 +- spoolman/database/vendor.py | 9 +- spoolman/extra_fields.py | 199 ++++++++++++++++++ spoolman/settings.py | 4 + tests_integration/tests/setting/test_get.py | 11 +- 14 files changed, 556 insertions(+), 25 deletions(-) create mode 100644 migrations/versions/2024_01_04_2209-b8881bdb716c_added_extra_fields.py create mode 100644 spoolman/api/v1/field.py create mode 100644 spoolman/extra_fields.py diff --git a/migrations/versions/2024_01_04_2209-b8881bdb716c_added_extra_fields.py b/migrations/versions/2024_01_04_2209-b8881bdb716c_added_extra_fields.py new file mode 100644 index 0000000..50a2b79 --- /dev/null +++ b/migrations/versions/2024_01_04_2209-b8881bdb716c_added_extra_fields.py @@ -0,0 +1,74 @@ +"""Added extra fields. + +Revision ID: b8881bdb716c +Revises: ccbb17aeda7c +Create Date: 2024-01-04 22:09:34.417527 +""" +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "b8881bdb716c" +down_revision = "ccbb17aeda7c" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + """Perform the upgrade.""" + op.create_table( + "vendor_field", + sa.Column("vendor_id", sa.Integer(), nullable=False), + sa.Column("key", sa.String(length=64), nullable=False), + sa.Column("value", sa.Text(), nullable=False), + sa.ForeignKeyConstraint( + ["vendor_id"], + ["vendor.id"], + ), + sa.PrimaryKeyConstraint("vendor_id", "key"), + ) + op.create_index(op.f("ix_vendor_field_key"), "vendor_field", ["key"], unique=False) + op.create_index(op.f("ix_vendor_field_vendor_id"), "vendor_field", ["vendor_id"], unique=False) + + op.create_table( + "filament_field", + sa.Column("filament_id", sa.Integer(), nullable=False), + sa.Column("key", sa.String(length=64), nullable=False), + sa.Column("value", sa.Text(), nullable=False), + sa.ForeignKeyConstraint( + ["filament_id"], + ["filament.id"], + ), + sa.PrimaryKeyConstraint("filament_id", "key"), + ) + op.create_index(op.f("ix_filament_field_filament_id"), "filament_field", ["filament_id"], unique=False) + op.create_index(op.f("ix_filament_field_key"), "filament_field", ["key"], unique=False) + + op.create_table( + "spool_field", + sa.Column("spool_id", sa.Integer(), nullable=False), + sa.Column("key", sa.String(length=64), nullable=False), + sa.Column("value", sa.Text(), nullable=False), + sa.ForeignKeyConstraint( + ["spool_id"], + ["spool.id"], + ), + sa.PrimaryKeyConstraint("spool_id", "key"), + ) + op.create_index(op.f("ix_spool_field_key"), "spool_field", ["key"], unique=False) + op.create_index(op.f("ix_spool_field_spool_id"), "spool_field", ["spool_id"], unique=False) + + +def downgrade() -> None: + """Perform the downgrade.""" + op.drop_index(op.f("ix_spool_field_spool_id"), table_name="spool_field") + op.drop_index(op.f("ix_spool_field_key"), table_name="spool_field") + op.drop_table("spool_field") + + op.drop_index(op.f("ix_filament_field_key"), table_name="filament_field") + op.drop_index(op.f("ix_filament_field_filament_id"), table_name="filament_field") + op.drop_table("filament_field") + + op.drop_index(op.f("ix_vendor_field_vendor_id"), table_name="vendor_field") + op.drop_index(op.f("ix_vendor_field_key"), table_name="vendor_field") + op.drop_table("vendor_field") diff --git a/spoolman/api/v1/field.py b/spoolman/api/v1/field.py new file mode 100644 index 0000000..0f1504c --- /dev/null +++ b/spoolman/api/v1/field.py @@ -0,0 +1,98 @@ +"""Vendor related endpoints.""" + +import logging +from typing import Annotated, Union + +from fastapi import APIRouter, Depends, Path +from fastapi.responses import JSONResponse +from sqlalchemy.ext.asyncio import AsyncSession + +from spoolman.api.v1.models import Message +from spoolman.database.database import get_db_session +from spoolman.exceptions import ItemNotFoundError +from spoolman.extra_fields import ( + EntityType, + ExtraField, + ExtraFieldParameters, + add_or_update_extra_field, + delete_extra_field, + get_extra_fields, +) + +router = APIRouter( + prefix="/field", + tags=["field"], +) + +# ruff: noqa: D103,B008 + +logger = logging.getLogger(__name__) + + +@router.get( + "/{entity_type}", + name="Get extra fields", + description="Get all extra fields for a specific entity type.", + response_model_exclude_none=True, +) +async def get( + db: Annotated[AsyncSession, Depends(get_db_session)], + entity_type: Annotated[EntityType, Path(description="Entity type this field is for")], +) -> list[ExtraField]: + return await get_extra_fields(db, entity_type) + + +@router.post( + "/{entity_type}/{key}", + name="Add or update extra field", + description=( + "Add or update an extra field for a specific entity type. " + "Returns the full list of extra fields for the entity type." + ), + response_model_exclude_none=True, + response_model=list[ExtraField], + responses={400: {"model": Message}}, +) +async def update( + db: Annotated[AsyncSession, Depends(get_db_session)], + entity_type: Annotated[EntityType, Path(description="Entity type this field is for")], + key: Annotated[str, Path(min_length=1, max_length=64, regex="^[a-z0-9_]+$")], + body: ExtraFieldParameters, +) -> Union[list[ExtraField], JSONResponse]: + body_with_key = ExtraField.parse_obj(body.copy(update={"key": key})) + + try: + await add_or_update_extra_field(db, entity_type, body_with_key) + except ValueError as e: + return JSONResponse(status_code=400, content=Message(message=str(e)).dict()) + + return await get_extra_fields(db, entity_type) + + +@router.delete( + "/{entity_type}/{key}", + name="Delete extra field", + description=( + "Delete an extra field for a specific entity type. " + "Returns the full list of extra fields for the entity type." + ), + response_model_exclude_none=True, + response_model=list[ExtraField], + responses={404: {"model": Message}}, +) +async def delete( + db: Annotated[AsyncSession, Depends(get_db_session)], + entity_type: Annotated[EntityType, Path(description="Entity type this field is for")], + key: Annotated[str, Path(min_length=1, max_length=64, regex="^[a-z0-9_]+$")], +) -> Union[list[ExtraField], JSONResponse]: + try: + await delete_extra_field(db, entity_type, key) + except ItemNotFoundError: + return JSONResponse( + status_code=404, + content=Message( + message=f"Extra field with key {key} does not exist for entity type {entity_type.name}", + ).dict(), + ) + + return await get_extra_fields(db, entity_type) diff --git a/spoolman/api/v1/filament.py b/spoolman/api/v1/filament.py index 451f446..a414a18 100644 --- a/spoolman/api/v1/filament.py +++ b/spoolman/api/v1/filament.py @@ -17,6 +17,7 @@ from spoolman.database import filament from spoolman.database.database import get_db_session from spoolman.database.utils import SortOrder from spoolman.exceptions import ItemDeleteError +from spoolman.extra_fields import EntityType, get_extra_fields, validate_extra_field_dict from spoolman.ws import websocket_manager logger = logging.getLogger(__name__) @@ -81,6 +82,10 @@ class FilamentParameters(BaseModel): description="Hexadecimal color code of the filament, e.g. FF0000 for red. Supports alpha channel at the end.", example="FF0000", ) + extra: Optional[dict[str, str]] = Field( + None, + description="Extra fields for this filament.", + ) @validator("color_hex") @classmethod @@ -323,11 +328,20 @@ async def notify( name="Add filament", description="Add a new filament to the database.", response_model_exclude_none=True, + response_model=Filament, + responses={400: {"model": Message}}, ) -async def create( +async def create( # noqa: ANN201 db: Annotated[AsyncSession, Depends(get_db_session)], body: FilamentParameters, -) -> Filament: +): + if body.extra: + all_fields = await get_extra_fields(db, EntityType.filament) + try: + validate_extra_field_dict(all_fields, body.extra) + except ValueError as e: + return JSONResponse(status_code=400, content=Message(message=str(e)).dict()) + db_item = await filament.create( db=db, density=body.density, @@ -343,6 +357,7 @@ async def create( settings_extruder_temp=body.settings_extruder_temp, settings_bed_temp=body.settings_bed_temp, color_hex=body.color_hex, + extra=body.extra, ) return Filament.from_db(db_item) @@ -351,15 +366,22 @@ async def create( @router.patch( "/{filament_id}", name="Update filament", - description="Update any attribute of a filament. Only fields specified in the request will be affected.", + description=( + "Update any attribute of a filament. Only fields specified in the request will be affected. " + "If extra is set, all existing extra fields will be removed and replaced with the new ones." + ), response_model_exclude_none=True, - responses={404: {"model": Message}}, + response_model=Filament, + responses={ + 400: {"model": Message}, + 404: {"model": Message}, + }, ) -async def update( +async def update( # noqa: ANN201 db: Annotated[AsyncSession, Depends(get_db_session)], filament_id: int, body: FilamentUpdateParameters, -) -> Filament: +): patch_data = body.dict(exclude_unset=True) if "density" in patch_data and body.density is None: @@ -367,6 +389,13 @@ async def update( if "diameter" in patch_data and body.diameter is None: raise RequestValidationError([ErrorWrapper(ValueError("diameter cannot be unset"), ("query", "diameter"))]) + if body.extra: + all_fields = await get_extra_fields(db, EntityType.filament) + try: + validate_extra_field_dict(all_fields, body.extra) + except ValueError as e: + return JSONResponse(status_code=400, content=Message(message=str(e)).dict()) + db_item = await filament.update( db=db, filament_id=filament_id, diff --git a/spoolman/api/v1/models.py b/spoolman/api/v1/models.py index 4288e12..3d36e9b 100644 --- a/spoolman/api/v1/models.py +++ b/spoolman/api/v1/models.py @@ -60,6 +60,12 @@ class Vendor(BaseModel): registered: datetime = Field(description="When the vendor was registered in the database. UTC Timezone.") name: str = Field(max_length=64, description="Vendor name.", example="Polymaker") comment: Optional[str] = Field(max_length=1024, description="Free text comment about this vendor.", example="") + extra: dict[str, str] = Field( + description=( + "Extra fields for this vendor. All values are JSON-encoded data. " + "Query the /fields endpoint for more details about the fields." + ), + ) @staticmethod def from_db(item: models.Vendor) -> "Vendor": @@ -69,6 +75,7 @@ class Vendor(BaseModel): registered=item.registered, name=item.name, comment=item.comment, + extra={field.key: field.value for field in item.extra}, ) @@ -128,6 +135,12 @@ class Filament(BaseModel): description="Hexadecimal color code of the filament, e.g. FF0000 for red. Supports alpha channel at the end.", example="FF0000", ) + extra: dict[str, str] = Field( + description=( + "Extra fields for this filament. All values are JSON-encoded data. " + "Query the /fields endpoint for more details about the fields." + ), + ) @staticmethod def from_db(item: models.Filament) -> "Filament": @@ -148,6 +161,7 @@ class Filament(BaseModel): settings_extruder_temp=item.settings_extruder_temp, settings_bed_temp=item.settings_bed_temp, color_hex=item.color_hex, + extra={field.key: field.value for field in item.extra}, ) @@ -198,6 +212,12 @@ class Spool(BaseModel): example="", ) archived: bool = Field(description="Whether this spool is archived and should not be used anymore.") + extra: dict[str, str] = Field( + description=( + "Extra fields for this spool. All values are JSON-encoded data. " + "Query the /fields endpoint for more details about the fields." + ), + ) @staticmethod def from_db(item: models.Spool) -> "Spool": @@ -235,6 +255,7 @@ class Spool(BaseModel): lot_nr=item.lot_nr, comment=item.comment, archived=item.archived if item.archived is not None else False, + extra={field.key: field.value for field in item.extra}, ) diff --git a/spoolman/api/v1/router.py b/spoolman/api/v1/router.py index 812db8a..fd9949e 100644 --- a/spoolman/api/v1/router.py +++ b/spoolman/api/v1/router.py @@ -15,7 +15,7 @@ from spoolman.database.database import backup_global_db from spoolman.exceptions import ItemNotFoundError from spoolman.ws import websocket_manager -from . import filament, models, other, setting, spool, vendor +from . import field, filament, models, other, setting, spool, vendor logger = logging.getLogger(__name__) @@ -108,4 +108,5 @@ app.include_router(filament.router) app.include_router(spool.router) app.include_router(vendor.router) app.include_router(setting.router) +app.include_router(field.router) app.include_router(other.router) diff --git a/spoolman/api/v1/spool.py b/spoolman/api/v1/spool.py index 35ec5ea..1391dfd 100644 --- a/spoolman/api/v1/spool.py +++ b/spoolman/api/v1/spool.py @@ -18,6 +18,7 @@ from spoolman.database import spool from spoolman.database.database import get_db_session from spoolman.database.utils import SortOrder from spoolman.exceptions import ItemCreateError +from spoolman.extra_fields import EntityType, get_extra_fields, validate_extra_field_dict from spoolman.ws import websocket_manager logger = logging.getLogger(__name__) @@ -59,6 +60,10 @@ class SpoolParameters(BaseModel): example="", ) archived: bool = Field(default=False, description="Whether this spool is archived and should not be used anymore.") + extra: Optional[dict[str, str]] = Field( + None, + description="Extra fields for this spool.", + ) class SpoolUpdateParameters(SpoolParameters): @@ -337,6 +342,13 @@ async def create( # noqa: ANN201 content={"message": "Only specify either remaining_weight or used_weight."}, ) + if body.extra: + all_fields = await get_extra_fields(db, EntityType.spool) + try: + validate_extra_field_dict(all_fields, body.extra) + except ValueError as e: + return JSONResponse(status_code=400, content=Message(message=str(e)).dict()) + try: db_item = await spool.create( db=db, @@ -350,6 +362,7 @@ async def create( # noqa: ANN201 lot_nr=body.lot_nr, comment=body.comment, archived=body.archived, + extra=body.extra, ) return Spool.from_db(db_item) except ItemCreateError: @@ -366,7 +379,8 @@ async def create( # noqa: ANN201 description=( "Update any attribute of a spool. " "Only fields specified in the request will be affected. " - "remaining_weight and used_weight can't be set at the same time." + "remaining_weight and used_weight can't be set at the same time. " + "If extra is set, all existing extra fields will be removed and replaced with the new ones." ), response_model_exclude_none=True, response_model=Spool, @@ -388,6 +402,13 @@ async def update( # noqa: ANN201 content={"message": "Only specify either remaining_weight or used_weight."}, ) + if body.extra: + all_fields = await get_extra_fields(db, EntityType.spool) + try: + validate_extra_field_dict(all_fields, body.extra) + except ValueError as e: + return JSONResponse(status_code=400, content=Message(message=str(e)).dict()) + if "filament_id" in patch_data and body.filament_id is None: raise RequestValidationError( [ErrorWrapper(ValueError("filament_id cannot be unset"), ("query", "filament_id"))], diff --git a/spoolman/api/v1/vendor.py b/spoolman/api/v1/vendor.py index e3b72fe..260e609 100644 --- a/spoolman/api/v1/vendor.py +++ b/spoolman/api/v1/vendor.py @@ -15,6 +15,7 @@ from spoolman.api.v1.models import Message, Vendor, VendorEvent from spoolman.database import vendor from spoolman.database.database import get_db_session from spoolman.database.utils import SortOrder +from spoolman.extra_fields import EntityType, get_extra_fields, validate_extra_field_dict from spoolman.ws import websocket_manager router = APIRouter( @@ -32,6 +33,10 @@ class VendorParameters(BaseModel): description="Free text comment about this vendor.", example="", ) + extra: Optional[dict[str, str]] = Field( + None, + description="Extra fields for this vendor.", + ) class VendorUpdateParameters(VendorParameters): @@ -167,15 +172,25 @@ async def notify( name="Add vendor", description="Add a new vendor to the database.", response_model_exclude_none=True, + response_model=Vendor, + responses={400: {"model": Message}}, ) -async def create( +async def create( # noqa: ANN201 db: Annotated[AsyncSession, Depends(get_db_session)], body: VendorParameters, -) -> Vendor: +): + if body.extra: + all_fields = await get_extra_fields(db, EntityType.vendor) + try: + validate_extra_field_dict(all_fields, body.extra) + except ValueError as e: + return JSONResponse(status_code=400, content=Message(message=str(e)).dict()) + db_item = await vendor.create( db=db, name=body.name, comment=body.comment, + extra=body.extra, ) return Vendor.from_db(db_item) @@ -184,20 +199,34 @@ async def create( @router.patch( "/{vendor_id}", name="Update vendor", - description="Update any attribute of a vendor. Only fields specified in the request will be affected.", + description=( + "Update any attribute of a vendor. Only fields specified in the request will be affected. " + "If extra is set, all existing extra fields will be removed and replaced with the new ones." + ), response_model_exclude_none=True, - responses={404: {"model": Message}}, + response_model=Vendor, + responses={ + 400: {"model": Message}, + 404: {"model": Message}, + }, ) -async def update( +async def update( # noqa: ANN201 db: Annotated[AsyncSession, Depends(get_db_session)], vendor_id: int, body: VendorUpdateParameters, -) -> Vendor: +): patch_data = body.dict(exclude_unset=True) if "name" in patch_data and body.name is None: raise RequestValidationError([ErrorWrapper(ValueError("name cannot be unset"), ("query", "name"))]) + if body.extra: + all_fields = await get_extra_fields(db, EntityType.vendor) + try: + validate_extra_field_dict(all_fields, body.extra) + except ValueError as e: + return JSONResponse(status_code=400, content=Message(message=str(e)).dict()) + db_item = await vendor.update( db=db, vendor_id=vendor_id, diff --git a/spoolman/database/filament.py b/spoolman/database/filament.py index 05ace5f..2c69e2d 100644 --- a/spoolman/database/filament.py +++ b/spoolman/database/filament.py @@ -41,6 +41,7 @@ async def create( settings_extruder_temp: Optional[int] = None, settings_bed_temp: Optional[int] = None, color_hex: Optional[str] = None, + extra: Optional[dict[str, str]] = None, ) -> models.Filament: """Add a new filament to the database.""" vendor_item: Optional[models.Vendor] = None @@ -62,6 +63,7 @@ async def create( settings_extruder_temp=settings_extruder_temp, settings_bed_temp=settings_bed_temp, color_hex=color_hex, + extra=[models.FilamentField(key=k, value=v) for k, v in (extra or {}).items()], ) db.add(filament) await db.commit() @@ -131,7 +133,7 @@ async def find( stmt = stmt.order_by(field.desc()) rows = await db.execute(stmt) - result = list(rows.scalars().all()) + result = list(rows.unique().scalars().all()) if total_count is None: total_count = len(result) @@ -152,6 +154,8 @@ async def update( filament.vendor = None else: filament.vendor = await vendor.get_by_id(db, v) + elif k == "extra": + filament.extra = [models.FilamentField(key=k, value=v) for k, v in v.items()] else: setattr(filament, k, v) await db.commit() diff --git a/spoolman/database/models.py b/spoolman/database/models.py index 685deb0..f06ac6f 100644 --- a/spoolman/database/models.py +++ b/spoolman/database/models.py @@ -4,10 +4,11 @@ from datetime import datetime from typing import Optional from sqlalchemy import ForeignKey, Integer, String, Text +from sqlalchemy.ext.asyncio import AsyncAttrs from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column, relationship -class Base(DeclarativeBase): +class Base(AsyncAttrs, DeclarativeBase): pass @@ -19,6 +20,11 @@ class Vendor(Base): name: Mapped[str] = mapped_column(String(64)) comment: Mapped[Optional[str]] = mapped_column(String(1024)) filaments: Mapped[list["Filament"]] = relationship(back_populates="vendor") + extra: Mapped[list["VendorField"]] = relationship( + back_populates="vendor", + cascade="save-update, merge, delete, delete-orphan", + lazy="joined", + ) class Filament(Base): @@ -41,6 +47,11 @@ class Filament(Base): settings_extruder_temp: Mapped[Optional[int]] = mapped_column(comment="Overridden extruder temperature.") settings_bed_temp: Mapped[Optional[int]] = mapped_column(comment="Overridden bed temperature.") color_hex: Mapped[Optional[str]] = mapped_column(String(8)) + extra: Mapped[list["FilamentField"]] = relationship( + back_populates="filament", + cascade="save-update, merge, delete, delete-orphan", + lazy="joined", + ) class Spool(Base): @@ -58,6 +69,11 @@ class Spool(Base): lot_nr: Mapped[Optional[str]] = mapped_column(String(64)) comment: Mapped[Optional[str]] = mapped_column(String(1024)) archived: Mapped[Optional[bool]] = mapped_column() + extra: Mapped[list["SpoolField"]] = relationship( + back_populates="spool", + cascade="save-update, merge, delete, delete-orphan", + lazy="joined", + ) class Setting(Base): @@ -66,3 +82,30 @@ class Setting(Base): key: Mapped[str] = mapped_column(String(64), primary_key=True, index=True) value: Mapped[str] = mapped_column(Text()) last_updated: Mapped[datetime] = mapped_column() + + +class VendorField(Base): + __tablename__ = "vendor_field" + + vendor_id: Mapped[int] = mapped_column(ForeignKey("vendor.id"), primary_key=True, index=True) + vendor: Mapped["Vendor"] = relationship(back_populates="extra") + key: Mapped[str] = mapped_column(String(64), primary_key=True, index=True) + value: Mapped[str] = mapped_column(Text()) + + +class FilamentField(Base): + __tablename__ = "filament_field" + + filament_id: Mapped[int] = mapped_column(ForeignKey("filament.id"), primary_key=True, index=True) + filament: Mapped["Filament"] = relationship(back_populates="extra") + key: Mapped[str] = mapped_column(String(64), primary_key=True, index=True) + value: Mapped[str] = mapped_column(Text()) + + +class SpoolField(Base): + __tablename__ = "spool_field" + + spool_id: Mapped[int] = mapped_column(ForeignKey("spool.id"), primary_key=True, index=True) + spool: Mapped["Spool"] = relationship(back_populates="extra") + key: Mapped[str] = mapped_column(String(64), primary_key=True, index=True) + value: Mapped[str] = mapped_column(Text()) diff --git a/spoolman/database/spool.py b/spoolman/database/spool.py index dc29d32..3f49670 100644 --- a/spoolman/database/spool.py +++ b/spoolman/database/spool.py @@ -43,6 +43,7 @@ async def create( lot_nr: Optional[str] = None, comment: Optional[str] = None, archived: bool = False, + extra: Optional[dict[str, str]] = None, ) -> models.Spool: """Add a new spool to the database. Leave weight empty to assume full spool.""" filament_item = await filament.get_by_id(db, filament_id) @@ -71,6 +72,7 @@ async def create( lot_nr=lot_nr, comment=comment, archived=archived, + extra=[models.SpoolField(key=k, value=v) for k, v in (extra or {}).items()], ) db.add(spool) await db.commit() @@ -158,7 +160,7 @@ async def find( stmt = stmt.order_by(field.desc()) rows = await db.execute(stmt) - result = list(rows.scalars().all()) + result = list(rows.unique().scalars().all()) if total_count is None: total_count = len(result) @@ -182,6 +184,8 @@ async def update( spool.used_weight = max(spool.filament.weight - v, 0) elif isinstance(v, datetime): setattr(spool, k, utc_timezone_naive(v)) + elif k == "extra": + spool.extra = [models.SpoolField(key=k, value=v) for k, v in v.items()] else: setattr(spool, k, v) await db.commit() diff --git a/spoolman/database/vendor.py b/spoolman/database/vendor.py index db7be09..628a8ed 100644 --- a/spoolman/database/vendor.py +++ b/spoolman/database/vendor.py @@ -18,12 +18,14 @@ async def create( db: AsyncSession, name: Optional[str] = None, comment: Optional[str] = None, + extra: Optional[dict[str, str]] = None, ) -> models.Vendor: """Add a new vendor to the database.""" vendor = models.Vendor( name=name, registered=datetime.utcnow().replace(microsecond=0), comment=comment, + extra=[models.VendorField(key=k, value=v) for k, v in (extra or {}).items()], ) db.add(vendor) await db.commit() @@ -72,7 +74,7 @@ async def find( stmt = stmt.order_by(field.desc()) rows = await db.execute(stmt) - result = list(rows.scalars().all()) + result = list(rows.unique().scalars().all()) if total_count is None: total_count = len(result) @@ -88,7 +90,10 @@ async def update( """Update the fields of a vendor object.""" vendor = await get_by_id(db, vendor_id) for k, v in data.items(): - setattr(vendor, k, v) + if k == "extra": + vendor.extra = [models.VendorField(key=k, value=v) for k, v in v.items()] + else: + setattr(vendor, k, v) await db.commit() await vendor_changed(vendor, EventType.UPDATED) return vendor diff --git a/spoolman/extra_fields.py b/spoolman/extra_fields.py new file mode 100644 index 0000000..ca003a2 --- /dev/null +++ b/spoolman/extra_fields.py @@ -0,0 +1,199 @@ +"""Custom/extra fields for spoolman entities.""" + +import json +import logging +from enum import Enum +from typing import Optional + +from fastapi.encoders import jsonable_encoder +from pydantic import BaseModel, Field +from sqlalchemy.ext.asyncio import AsyncSession + +from spoolman.database import setting as db_setting +from spoolman.exceptions import ItemNotFoundError +from spoolman.settings import parse_setting + +logger = logging.getLogger(__name__) + + +class EntityType(Enum): + vendor = "vendor" + filament = "filament" + spool = "spool" + + +class ExtraFieldType(Enum): + text = "text" + integer = "integer" + integer_range = "integer_range" + float = "float" + float_range = "float_range" + datetime = "datetime" + boolean = "boolean" + choice = "choice" + + +class ExtraFieldParameters(BaseModel): + name: str = Field(description="Nice name", min_length=1, max_length=128) + unit: Optional[str] = Field(None, description="Unit of the value", min_length=1, max_length=16) + entity_type: EntityType = Field(description="Entity type this field is for") + field_type: ExtraFieldType = Field(description="Type of the field") + default_value: Optional[str] = Field(None, description="Default value of the field") + choices: Optional[list[str]] = Field(None, description="Choices for the field, only for field type choice") + multi_choice: Optional[bool] = Field(None, description="Whether multiple choices can be selected") + + +class ExtraField(ExtraFieldParameters): + key: str = Field(description="Unique key", regex="^[a-z0-9_]+$", min_length=1, max_length=64) + + +def validate_extra_field_value(field: ExtraFieldParameters, value: str) -> None: # noqa: C901, PLR0912 + """Validate that the value has the correct type.""" + try: + data = json.loads(value) + except json.JSONDecodeError: + raise ValueError("Value is not valid JSON.") from None + + if field.field_type == ExtraFieldType.text: + if not isinstance(data, str): + raise ValueError("Value is not a string.") + elif field.field_type == ExtraFieldType.integer: + if not isinstance(data, int): + raise ValueError("Value is not an integer.") + elif field.field_type == ExtraFieldType.integer_range: + if not isinstance(data, list): + raise ValueError("Value is not a list.") + if len(data) != 2: # noqa: PLR2004 + raise ValueError("Value list must have exactly two values.") + if not all(isinstance(value, int) for value in data): + raise ValueError("Value list must contain only integers.") + elif field.field_type == ExtraFieldType.float: + if not isinstance(data, (float, int)) or isinstance(data, bool): + raise ValueError("Value is not a float.") + elif field.field_type == ExtraFieldType.float_range: + if not isinstance(data, list): + raise ValueError("Value is not a list.") + if len(data) != 2: # noqa: PLR2004 + raise ValueError("Value list must have exactly two values.") + if not all(isinstance(value, (float, int)) and not isinstance(value, bool) for value in data): + raise ValueError("Value list must contain only floats.") + elif field.field_type == ExtraFieldType.datetime: + if not isinstance(data, str): + raise ValueError("Value is not a string.") + elif field.field_type == ExtraFieldType.boolean: + if not isinstance(data, bool): + raise ValueError("Value is not a boolean.") + elif field.field_type == ExtraFieldType.choice: + if field.multi_choice: + if not isinstance(data, list): + raise ValueError("Value is not a list.") + if not all(isinstance(value, str) for value in data): + raise ValueError("Value list must contain only strings.") + if field.choices is not None and not all(value in field.choices for value in data): + raise ValueError("Value list contains invalid choices.") + else: + if not isinstance(data, str): + raise ValueError("Value is not a string.") + if field.choices is not None and data not in field.choices: + raise ValueError("Value is not a valid choice.") + else: + raise ValueError(f"Unknown field type {field.field_type}.") + + +def validate_extra_field(field: ExtraFieldParameters) -> None: # noqa: C901 + """Validate an extra field.""" + # Validate choices exist if field type is choice + if field.field_type == ExtraFieldType.choice: + if field.choices is None: + raise ValueError("Choices must be set for field type choice.") + if not isinstance(field.choices, list): + raise ValueError("Choices must be a list.") + if not all(isinstance(choice, str) for choice in field.choices): + raise ValueError("Choices must be a list of strings.") + if len(field.choices) == 0: + raise ValueError("Choices must not be empty.") + if field.multi_choice is None: + raise ValueError("Multi choice must be set for field type choice.") + else: + if field.choices is not None: + raise ValueError("Choices must not be set for field type other than choice.") + if field.multi_choice is not None: + raise ValueError("Multi choice must not be set for field type other than choice.") + + # Validate default value data type + if field.default_value is not None: + try: + validate_extra_field_value(field, field.default_value) + except ValueError as e: + raise ValueError(f"Default value is not valid: {e}") from None + + +def validate_extra_field_dict(all_fields: list[ExtraField], fields_input: dict[str, str]) -> None: + """Validate a dict of extra fields.""" + all_field_lookup = {field.key: field for field in all_fields} + for key, value in fields_input.items(): + if key not in all_field_lookup: + raise ValueError(f"Unknown extra field {key}.") + field = all_field_lookup[key] + try: + validate_extra_field_value(field, value) + except ValueError as e: + raise ValueError(f"Invalid extra field for key {key}: {e!s}") from None + + +async def get_extra_fields(db: AsyncSession, entity_type: EntityType) -> list[ExtraField]: + """Get all extra fields for a specific entity type.""" + setting_def = parse_setting(f"extra_fields_{entity_type.name}") + try: + setting = await db_setting.get(db, setting_def) + setting_value = setting.value + except ItemNotFoundError: + setting_value = setting_def.default + + setting_array = json.loads(setting_value) + if not isinstance(setting_array, list): + logger.warning("Setting %s is not a list, using default.", setting_def.key) + setting_array = [] + + return [ExtraField.parse_obj(obj) for obj in setting_array] + + +async def add_or_update_extra_field(db: AsyncSession, entity_type: EntityType, extra_field: ExtraField) -> None: + """Add or update an extra field for a specific entity type.""" + validate_extra_field(extra_field) + + extra_fields = await get_extra_fields(db, entity_type) + extra_fields = [field for field in extra_fields if field.key != extra_field.key] + extra_fields.append(extra_field) + + setting_def = parse_setting(f"extra_fields_{entity_type.name}") + await db_setting.update(db=db, definition=setting_def, value=json.dumps(jsonable_encoder(extra_fields))) + + logger.info("Added/updated extra field %s for entity type %s.", extra_field.key, entity_type.name) + + +async def delete_extra_field(db: AsyncSession, entity_type: EntityType, key: str) -> None: + """Delete an extra field for a specific entity type.""" + extra_fields = await get_extra_fields(db, entity_type) + + # Check if the field exists + if not any(field.key == key for field in extra_fields): + raise ItemNotFoundError(f"Extra field with key {key} does not exist.") + + extra_fields = [field for field in extra_fields if field.key != key] + + setting_def = parse_setting(f"extra_fields_{entity_type.name}") + await db_setting.update(db=db, definition=setting_def, value=json.dumps(jsonable_encoder(extra_fields))) + + logger.info("Deleted extra field %s for entity type %s.", key, entity_type.name) + + +async def populate_with_defaults(db: AsyncSession, entity_type: EntityType, existing: dict[str, str]) -> None: + """Populate the given list of extra fields with defaults.""" + extra_fields = await get_extra_fields(db, entity_type) + for extra_field in extra_fields: + if extra_field.default_value is None: + continue + if extra_field.key in existing: + continue + existing[extra_field.key] = extra_field.default_value diff --git a/spoolman/settings.py b/spoolman/settings.py index 4f35191..b3600bc 100644 --- a/spoolman/settings.py +++ b/spoolman/settings.py @@ -62,3 +62,7 @@ def parse_setting(key: str) -> SettingDefinition: register_setting("currency", SettingType.STRING, json.dumps("EUR")) + +register_setting("extra_fields_vendor", SettingType.ARRAY, json.dumps([])) +register_setting("extra_fields_filament", SettingType.ARRAY, json.dumps([])) +register_setting("extra_fields_spool", SettingType.ARRAY, json.dumps([])) diff --git a/tests_integration/tests/setting/test_get.py b/tests_integration/tests/setting/test_get.py index bb8c006..36c5b05 100644 --- a/tests_integration/tests/setting/test_get.py +++ b/tests_integration/tests/setting/test_get.py @@ -35,10 +35,9 @@ def test_get_all(): # Verify settings = result.json() - assert settings == { - "currency": { - "value": '"EUR"', - "is_set": False, - "type": "string", - }, + assert "currency" in settings + assert settings["currency"] == { + "value": '"EUR"', + "is_set": False, + "type": "string", } From 06e384986244f0888c5903971eda9f301df9dc82 Mon Sep 17 00:00:00 2001 From: Donkie Date: Sat, 6 Jan 2024 17:16:15 +0100 Subject: [PATCH 02/18] No longer need to specify entity_type in body --- spoolman/api/v1/field.py | 2 +- spoolman/extra_fields.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spoolman/api/v1/field.py b/spoolman/api/v1/field.py index 0f1504c..9efcba7 100644 --- a/spoolman/api/v1/field.py +++ b/spoolman/api/v1/field.py @@ -59,7 +59,7 @@ async def update( key: Annotated[str, Path(min_length=1, max_length=64, regex="^[a-z0-9_]+$")], body: ExtraFieldParameters, ) -> Union[list[ExtraField], JSONResponse]: - body_with_key = ExtraField.parse_obj(body.copy(update={"key": key})) + body_with_key = ExtraField.parse_obj(body.copy(update={"key": key, "entity_type": entity_type})) try: await add_or_update_extra_field(db, entity_type, body_with_key) diff --git a/spoolman/extra_fields.py b/spoolman/extra_fields.py index ca003a2..83e5c18 100644 --- a/spoolman/extra_fields.py +++ b/spoolman/extra_fields.py @@ -36,7 +36,6 @@ class ExtraFieldType(Enum): class ExtraFieldParameters(BaseModel): name: str = Field(description="Nice name", min_length=1, max_length=128) unit: Optional[str] = Field(None, description="Unit of the value", min_length=1, max_length=16) - entity_type: EntityType = Field(description="Entity type this field is for") field_type: ExtraFieldType = Field(description="Type of the field") default_value: Optional[str] = Field(None, description="Default value of the field") choices: Optional[list[str]] = Field(None, description="Choices for the field, only for field type choice") @@ -45,6 +44,7 @@ class ExtraFieldParameters(BaseModel): class ExtraField(ExtraFieldParameters): key: str = Field(description="Unique key", regex="^[a-z0-9_]+$", min_length=1, max_length=64) + entity_type: EntityType = Field(description="Entity type this field is for") def validate_extra_field_value(field: ExtraFieldParameters, value: str) -> None: # noqa: C901, PLR0912 From 86e126f775db9876a250970f288c63c79dd2510a Mon Sep 17 00:00:00 2001 From: Donkie Date: Sat, 6 Jan 2024 17:16:30 +0100 Subject: [PATCH 03/18] Added some basic field CRUD tests --- spoolman/extra_fields.py | 14 +- tests_integration/tests/conftest.py | 51 ++++ tests_integration/tests/fields/__init__.py | 1 + tests_integration/tests/fields/test_create.py | 262 ++++++++++++++++++ tests_integration/tests/fields/test_delete.py | 31 +++ tests_integration/tests/fields/test_get.py | 60 ++++ 6 files changed, 411 insertions(+), 8 deletions(-) create mode 100644 tests_integration/tests/fields/__init__.py create mode 100644 tests_integration/tests/fields/test_create.py create mode 100644 tests_integration/tests/fields/test_delete.py create mode 100644 tests_integration/tests/fields/test_get.py diff --git a/spoolman/extra_fields.py b/spoolman/extra_fields.py index 83e5c18..ad32284 100644 --- a/spoolman/extra_fields.py +++ b/spoolman/extra_fields.py @@ -38,7 +38,11 @@ class ExtraFieldParameters(BaseModel): unit: Optional[str] = Field(None, description="Unit of the value", min_length=1, max_length=16) field_type: ExtraFieldType = Field(description="Type of the field") default_value: Optional[str] = Field(None, description="Default value of the field") - choices: Optional[list[str]] = Field(None, description="Choices for the field, only for field type choice") + choices: Optional[list[str]] = Field( + None, + description="Choices for the field, only for field type choice", + min_items=1, + ) multi_choice: Optional[bool] = Field(None, description="Whether multiple choices can be selected") @@ -100,18 +104,12 @@ def validate_extra_field_value(field: ExtraFieldParameters, value: str) -> None: raise ValueError(f"Unknown field type {field.field_type}.") -def validate_extra_field(field: ExtraFieldParameters) -> None: # noqa: C901 +def validate_extra_field(field: ExtraFieldParameters) -> None: """Validate an extra field.""" # Validate choices exist if field type is choice if field.field_type == ExtraFieldType.choice: if field.choices is None: raise ValueError("Choices must be set for field type choice.") - if not isinstance(field.choices, list): - raise ValueError("Choices must be a list.") - if not all(isinstance(choice, str) for choice in field.choices): - raise ValueError("Choices must be a list of strings.") - if len(field.choices) == 0: - raise ValueError("Choices must not be empty.") if field.multi_choice is None: raise ValueError("Multi choice must be set for field type choice.") else: diff --git a/tests_integration/tests/conftest.py b/tests_integration/tests/conftest.py index a87de9f..f8d537e 100644 --- a/tests_integration/tests/conftest.py +++ b/tests_integration/tests/conftest.py @@ -240,3 +240,54 @@ def length_from_weight(*, weight: float, diameter: float, density: float) -> flo volume_cm3 = weight / density volume_mm3 = volume_cm3 * 1000 return volume_mm3 / (math.pi * (diameter / 2) ** 2) + + +def assert_dicts_compatible(actual: Any, expected: Any, path: str = "") -> None: # noqa: ANN401 + """Assert that two dictionaries are compatible for unit testing a REST API. + + Args: + actual (dict): The actual dictionary. + expected (dict): The expected dictionary. + path (str): The path to the current level in the dictionary (used for error messages). + + Raises: + AssertionError: If dictionaries are not compatible. + """ + # Check if both inputs are dictionaries + if not (isinstance(actual, dict) and isinstance(expected, dict)): + raise TypeError(f"At {path}: Actual and expected values must be dictionaries.") + + # Check if actual dictionary contains all keys of the expected dictionary + missing_keys = [key for key in expected if key not in actual] + if missing_keys: + raise AssertionError(f"At {path}: Missing keys in actual dictionary: {missing_keys}") + + # Recursively check values if the corresponding keys exist + for key, expected_value in expected.items(): + actual_value = actual[key] + subpath = f"{path}.{key}" if path else key # Update the path for the current level + + # If the value is another dictionary, recurse into it + if isinstance(expected_value, dict): + assert_dicts_compatible(actual_value, expected_value, path=subpath) + elif actual_value != expected_value: # Check if values are equal + raise AssertionError( + f"At {subpath}: Values do not match. Expected: {expected_value}, Actual: {actual_value}", + ) + + +def assert_lists_compatible(a: Iterable[dict[str, Any]], b: Iterable[dict[str, Any]], sort_key: str = "id") -> None: + """Compare two lists of items where the order of the items is not guaranteed.""" + a_sorted = sorted(a, key=lambda x: x[sort_key]) + b_sorted = sorted(b, key=lambda x: x[sort_key]) + if len(a_sorted) != len(b_sorted): + pytest.fail(f"Lists have different lengths: {len(a_sorted)} != {len(b_sorted)}") + + for a_filament, b_filament in zip(a_sorted, b_sorted): + assert_dicts_compatible(a_filament, b_filament) + + +def assert_httpx_success(response: httpx.Response) -> None: + """Assert that a response is successful.""" + if not response.is_success: + pytest.fail(f"Request failed: {response.status_code} {response.text}") diff --git a/tests_integration/tests/fields/__init__.py b/tests_integration/tests/fields/__init__.py new file mode 100644 index 0000000..a59459b --- /dev/null +++ b/tests_integration/tests/fields/__init__.py @@ -0,0 +1 @@ +"""Integration tests for the custom extra fields system.""" diff --git a/tests_integration/tests/fields/test_create.py b/tests_integration/tests/fields/test_create.py new file mode 100644 index 0000000..83b1e13 --- /dev/null +++ b/tests_integration/tests/fields/test_create.py @@ -0,0 +1,262 @@ +"""Integration tests for the custom extra fields system.""" + +import json +from datetime import datetime, timezone + +import httpx +import pytest + +from ..conftest import assert_httpx_success + +URL = "http://spoolman:8000" + + +def test_add_text_field(): + """Test adding a text field for spools.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mytextfield", + json={ + "name": "My text field", + "field_type": "text", + "default_value": json.dumps("Hello World"), + }, + ) + assert_httpx_success(result) + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/mytextfield") + assert_httpx_success(result) + + +def test_add_text_field_filament(): + """Test adding a text field for filaments.""" + result = httpx.post( + f"{URL}/api/v1/field/filament/mytextfield", + json={ + "name": "My text field", + "field_type": "text", + "default_value": json.dumps("Hello World"), + }, + ) + assert_httpx_success(result) + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/filament/mytextfield") + assert_httpx_success(result) + + +def test_add_text_field_vendor(): + """Test adding a text field for vendors.""" + result = httpx.post( + f"{URL}/api/v1/field/vendor/mytextfield", + json={ + "name": "My text field", + "field_type": "text", + "default_value": json.dumps("Hello World"), + }, + ) + assert_httpx_success(result) + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/vendor/mytextfield") + assert_httpx_success(result) + + +def test_add_integer_field(): + """Test adding an integer field for spools.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/myintegerfield", + json={ + "name": "My integer field", + "field_type": "integer", + "default_value": json.dumps(42), + }, + ) + assert_httpx_success(result) + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/myintegerfield") + assert_httpx_success(result) + + +def test_add_integer_range_field(): + """Test adding an integer range field for spools.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/myintegerrangefield", + json={ + "name": "My integer range field", + "field_type": "integer_range", + "default_value": json.dumps([0, 100]), + }, + ) + assert_httpx_success(result) + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/myintegerrangefield") + assert_httpx_success(result) + + +def test_add_float_field(): + """Test adding a float field for spools.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/myfloatfield", + json={ + "name": "My float field", + "field_type": "float", + "default_value": json.dumps(3.14), + }, + ) + assert_httpx_success(result) + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/myfloatfield") + assert_httpx_success(result) + + +def test_add_float_range_field(): + """Test adding a float range field for spools.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/myfloatrangefield", + json={ + "name": "My float range field", + "field_type": "float_range", + "default_value": json.dumps([0.0, 1.0]), + }, + ) + assert_httpx_success(result) + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/myfloatrangefield") + assert_httpx_success(result) + + +def test_add_datetime_field(): + """Test adding a datetime field for spools.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mydatetimefield", + json={ + "name": "My datetime field", + "field_type": "datetime", + "default_value": json.dumps(datetime.now(timezone.utc).isoformat()), + }, + ) + assert_httpx_success(result) + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/mydatetimefield") + assert_httpx_success(result) + + +def test_add_boolean_field(): + """Test adding a boolean field for spools.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mybooleanfield", + json={ + "name": "My boolean field", + "field_type": "boolean", + "default_value": json.dumps(True), # noqa: FBT003 + }, + ) + assert_httpx_success(result) + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/mybooleanfield") + assert_httpx_success(result) + + +@pytest.mark.parametrize( + "multi_choice", + [True, False], +) +def test_add_choice_field(multi_choice: bool): # noqa: FBT001 + """Test adding a choice field for spools.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mychoicefield", + json={ + "name": "My choice field", + "field_type": "choice", + "choices": ["foo", "bar", "baz"], + "default_value": json.dumps(["foo"]) if multi_choice else json.dumps("foo"), + "multi_choice": multi_choice, + }, + ) + assert_httpx_success(result) + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/mychoicefield") + assert_httpx_success(result) + + +def test_add_text_field_invalid_data(): + """Test adding a text field with invalid default value.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mytextfield", + json={ + "name": "My text field", + "field_type": "text", + "default_value": json.dumps(42), + }, + ) + assert result.status_code == 400 + assert result.json()["message"] == "Default value is not valid: Value is not a string." + + +def test_add_choice_field_without_multi_choice(): + """Test adding a choice field without multi_choice set.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mychoicefield", + json={ + "name": "My choice field", + "field_type": "choice", + "choices": ["foo", "bar", "baz"], + "default_value": json.dumps("foo"), + }, + ) + assert result.status_code == 400 + assert result.json()["message"] == "Multi choice must be set for field type choice." + + +def test_add_choice_field_invalid_choices(): + """Test adding a choice field with invalid choices.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mychoicefield", + json={ + "name": "My choice field", + "field_type": "choice", + "choices": ["foo", "bar", "baz", {"foo": "bar"}], + "default_value": json.dumps(["foo"]), + "multi_choice": True, + }, + ) + assert result.status_code == 422 + + +def test_add_choice_field_invalid_default_value(): + """Test adding a choice field with invalid default value.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mychoicefield", + json={ + "name": "My choice field", + "field_type": "choice", + "choices": ["foo", "bar", "baz"], + "default_value": json.dumps(42), + "multi_choice": False, + }, + ) + assert result.status_code == 400 + assert result.json()["message"] == "Default value is not valid: Value is not a string." + + +def test_add_choice_field_no_choices(): + """Test adding a choice field without choices set.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mychoicefield", + json={ + "name": "My choice field", + "field_type": "choice", + "default_value": json.dumps("foo"), + "multi_choice": True, + }, + ) + assert result.status_code == 400 + assert result.json()["message"] == "Choices must be set for field type choice." diff --git a/tests_integration/tests/fields/test_delete.py b/tests_integration/tests/fields/test_delete.py new file mode 100644 index 0000000..3c540a5 --- /dev/null +++ b/tests_integration/tests/fields/test_delete.py @@ -0,0 +1,31 @@ +"""Integration tests for the custom extra fields system.""" + +import json + +import httpx + +from ..conftest import assert_httpx_success + +URL = "http://spoolman:8000" + + +def test_delete_field(): + """Test adding a field, deleting it, and making sure it's gone.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mytextfield", + json={ + "name": "My text field", + "field_type": "text", + "default_value": json.dumps("Hello World"), + }, + ) + assert_httpx_success(result) + + # Delete + result = httpx.delete(f"{URL}/api/v1/field/spool/mytextfield") + assert_httpx_success(result) + + # Verify + result = httpx.get(f"{URL}/api/v1/field/spool") + assert_httpx_success(result) + assert result.json() == [] diff --git a/tests_integration/tests/fields/test_get.py b/tests_integration/tests/fields/test_get.py new file mode 100644 index 0000000..26da655 --- /dev/null +++ b/tests_integration/tests/fields/test_get.py @@ -0,0 +1,60 @@ +"""Integration tests for the custom extra fields system.""" + +import json + +import httpx + +from ..conftest import assert_httpx_success, assert_lists_compatible + +URL = "http://spoolman:8000" + + +def test_get_field(): + """Test adding a couple of fields to the spool and then getting them.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mytextfield", + json={ + "name": "My text field", + "field_type": "text", + "default_value": json.dumps("Hello World"), + }, + ) + assert_httpx_success(result) + + result = httpx.post( + f"{URL}/api/v1/field/spool/myintfield", + json={ + "name": "My int field", + "field_type": "integer", + "default_value": json.dumps(42), + }, + ) + assert_httpx_success(result) + + result = httpx.get(f"{URL}/api/v1/field/spool") + assert_httpx_success(result) + assert_lists_compatible( + result.json(), + [ + { + "name": "My text field", + "key": "mytextfield", + "field_type": "text", + "default_value": json.dumps("Hello World"), + }, + { + "name": "My int field", + "key": "myintfield", + "field_type": "integer", + "default_value": json.dumps(42), + }, + ], + sort_key="key", + ) + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/mytextfield") + assert_httpx_success(result) + + result = httpx.delete(f"{URL}/api/v1/field/spool/myintfield") + assert_httpx_success(result) From 09b8705133d5340596bd1e1d82bc0b6fae9ff52c Mon Sep 17 00:00:00 2001 From: Donkie Date: Sat, 6 Jan 2024 20:08:39 +0100 Subject: [PATCH 04/18] Existing extra fields for entities are now cleared if a field is deleted --- spoolman/database/filament.py | 8 ++++++++ spoolman/database/spool.py | 7 +++++++ spoolman/database/vendor.py | 8 ++++++++ spoolman/extra_fields.py | 13 +++++++++++++ 4 files changed, 36 insertions(+) diff --git a/spoolman/database/filament.py b/spoolman/database/filament.py index 2c69e2d..a87f27a 100644 --- a/spoolman/database/filament.py +++ b/spoolman/database/filament.py @@ -5,6 +5,7 @@ from collections.abc import Sequence from datetime import datetime from typing import Optional, Union +import sqlalchemy from sqlalchemy import func, select from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.asyncio import AsyncSession @@ -175,6 +176,13 @@ async def delete(db: AsyncSession, filament_id: int) -> None: raise ItemDeleteError("Failed to delete filament.") from exc +async def clear_extra_field(db: AsyncSession, key: str) -> None: + """Delete all extra fields with a specific key.""" + await db.execute( + sqlalchemy.delete(models.FilamentField).where(models.FilamentField.key == key), + ) + + logger = logging.getLogger(__name__) diff --git a/spoolman/database/spool.py b/spoolman/database/spool.py index 3f49670..7710231 100644 --- a/spoolman/database/spool.py +++ b/spoolman/database/spool.py @@ -200,6 +200,13 @@ async def delete(db: AsyncSession, spool_id: int) -> None: await spool_changed(spool, EventType.DELETED) +async def clear_extra_field(db: AsyncSession, key: str) -> None: + """Delete all extra fields with a specific key.""" + await db.execute( + sqlalchemy.delete(models.SpoolField).where(models.SpoolField.key == key), + ) + + async def use_weight_safe(db: AsyncSession, spool_id: int, weight: float) -> None: """Consume filament from a spool by weight in a way that is safe against race conditions. diff --git a/spoolman/database/vendor.py b/spoolman/database/vendor.py index 628a8ed..67ba082 100644 --- a/spoolman/database/vendor.py +++ b/spoolman/database/vendor.py @@ -3,6 +3,7 @@ from datetime import datetime from typing import Optional +import sqlalchemy from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession @@ -106,6 +107,13 @@ async def delete(db: AsyncSession, vendor_id: int) -> None: await vendor_changed(vendor, EventType.DELETED) +async def clear_extra_field(db: AsyncSession, key: str) -> None: + """Delete all extra fields with a specific key.""" + await db.execute( + sqlalchemy.delete(models.VendorField).where(models.VendorField.key == key), + ) + + async def vendor_changed(vendor: models.Vendor, typ: EventType) -> None: """Notify websocket clients that a vendor has changed.""" await websocket_manager.send( diff --git a/spoolman/extra_fields.py b/spoolman/extra_fields.py index ad32284..8eb94eb 100644 --- a/spoolman/extra_fields.py +++ b/spoolman/extra_fields.py @@ -9,7 +9,10 @@ from fastapi.encoders import jsonable_encoder from pydantic import BaseModel, Field from sqlalchemy.ext.asyncio import AsyncSession +from spoolman.database import filament as db_filament from spoolman.database import setting as db_setting +from spoolman.database import spool as db_spool +from spoolman.database import vendor as db_vendor from spoolman.exceptions import ItemNotFoundError from spoolman.settings import parse_setting @@ -183,6 +186,16 @@ async def delete_extra_field(db: AsyncSession, entity_type: EntityType, key: str setting_def = parse_setting(f"extra_fields_{entity_type.name}") await db_setting.update(db=db, definition=setting_def, value=json.dumps(jsonable_encoder(extra_fields))) + # Delete the extra field for all entities + if entity_type == EntityType.vendor: + await db_vendor.clear_extra_field(db, key) + elif entity_type == EntityType.filament: + await db_filament.clear_extra_field(db, key) + elif entity_type == EntityType.spool: + await db_spool.clear_extra_field(db, key) + else: + raise ValueError(f"Unknown entity type {entity_type.name}.") + logger.info("Deleted extra field %s for entity type %s.", key, entity_type.name) From 36a346bf645e099ecca0ae6d1b66ab425a7f3356 Mon Sep 17 00:00:00 2001 From: Donkie Date: Sat, 6 Jan 2024 20:09:09 +0100 Subject: [PATCH 05/18] Added restrictions on changing existing fields --- spoolman/extra_fields.py | 19 ++ tests_integration/tests/fields/test_create.py | 169 +++++++++++++++++- 2 files changed, 187 insertions(+), 1 deletion(-) diff --git a/spoolman/extra_fields.py b/spoolman/extra_fields.py index 8eb94eb..43e451f 100644 --- a/spoolman/extra_fields.py +++ b/spoolman/extra_fields.py @@ -164,6 +164,25 @@ async def add_or_update_extra_field(db: AsyncSession, entity_type: EntityType, e validate_extra_field(extra_field) extra_fields = await get_extra_fields(db, entity_type) + + # If the field already exists, verify that we don't do anything that would break existing data + existing_field = next((field for field in extra_fields if field.key == extra_field.key), None) + if existing_field is not None: + if existing_field.field_type != extra_field.field_type: + raise ValueError("Field type cannot be changed.") + if extra_field.field_type == ExtraFieldType.choice: + # Can't change multi choice since that would break existing data + if existing_field.multi_choice != extra_field.multi_choice: + raise ValueError("Multi choice cannot be changed.") + + # Verify that we have only added new choices, not removed any + if ( + existing_field.choices is not None + and extra_field.choices is not None + and not all(choice in extra_field.choices for choice in existing_field.choices) + ): + raise ValueError("Cannot remove existing choices.") + extra_fields = [field for field in extra_fields if field.key != extra_field.key] extra_fields.append(extra_field) diff --git a/tests_integration/tests/fields/test_create.py b/tests_integration/tests/fields/test_create.py index 83b1e13..23718be 100644 --- a/tests_integration/tests/fields/test_create.py +++ b/tests_integration/tests/fields/test_create.py @@ -6,7 +6,7 @@ from datetime import datetime, timezone import httpx import pytest -from ..conftest import assert_httpx_success +from ..conftest import assert_httpx_success, assert_lists_compatible URL = "http://spoolman:8000" @@ -68,6 +68,7 @@ def test_add_integer_field(): f"{URL}/api/v1/field/spool/myintegerfield", json={ "name": "My integer field", + "unit": "mm", "field_type": "integer", "default_value": json.dumps(42), }, @@ -260,3 +261,169 @@ def test_add_choice_field_no_choices(): ) assert result.status_code == 400 assert result.json()["message"] == "Choices must be set for field type choice." + + +def test_update_existing_field(): + """Test updating an existing field.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mytextfield", + json={ + "name": "My text field", + "field_type": "text", + "default_value": json.dumps("Hello World"), + }, + ) + assert_httpx_success(result) + + result = httpx.post( + f"{URL}/api/v1/field/spool/mytextfield", + json={ + "name": "My text field", + "field_type": "text", + "default_value": json.dumps("Goodbye World"), + }, + ) + assert_httpx_success(result) + + # Verify that the default value was updated, and that it is still the only field + result = httpx.get(f"{URL}/api/v1/field/spool") + assert_httpx_success(result) + assert_lists_compatible( + result.json(), + [ + { + "name": "My text field", + "key": "mytextfield", + "field_type": "text", + "default_value": json.dumps("Goodbye World"), + }, + ], + sort_key="key", + ) + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/mytextfield") + assert_httpx_success(result) + + +def test_update_field_change_field_type(): + """Test updating an existing field and changing the field type, should not be allowed.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mytextfield", + json={ + "name": "My text field", + "field_type": "text", + "default_value": json.dumps("Hello World"), + }, + ) + assert_httpx_success(result) + + result = httpx.post( + f"{URL}/api/v1/field/spool/mytextfield", + json={ + "name": "My text field", + "field_type": "integer", + "default_value": json.dumps(42), + }, + ) + assert result.status_code == 400 + assert result.json()["message"] == "Field type cannot be changed." + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/mytextfield") + assert_httpx_success(result) + + +def test_update_field_add_choice(): + """Test updating an existing field and adding a choice, should be allowed.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mychoicefield", + json={ + "name": "My choice field", + "field_type": "choice", + "choices": ["foo", "bar", "baz"], + "default_value": json.dumps(["foo"]), + "multi_choice": True, + }, + ) + assert_httpx_success(result) + + result = httpx.post( + f"{URL}/api/v1/field/spool/mychoicefield", + json={ + "name": "My choice field", + "field_type": "choice", + "choices": ["foo", "bar", "baz", "qux"], + "default_value": json.dumps(["foo"]), + "multi_choice": True, + }, + ) + assert_httpx_success(result) + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/mychoicefield") + assert_httpx_success(result) + + +def test_update_field_remove_choice(): + """Test updating an existing field and removing a choice, should not be allowed.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mychoicefield", + json={ + "name": "My choice field", + "field_type": "choice", + "choices": ["foo", "bar", "baz"], + "default_value": json.dumps(["foo"]), + "multi_choice": True, + }, + ) + assert_httpx_success(result) + + result = httpx.post( + f"{URL}/api/v1/field/spool/mychoicefield", + json={ + "name": "My choice field", + "field_type": "choice", + "choices": ["foo", "baz"], + "default_value": json.dumps(["foo"]), + "multi_choice": True, + }, + ) + assert result.status_code == 400 + assert result.json()["message"] == "Cannot remove existing choices." + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/mychoicefield") + assert_httpx_success(result) + + +def test_update_field_change_multi_choice(): + """Test updating an existing field and changing the multi_choice setting, should not be allowed.""" + result = httpx.post( + f"{URL}/api/v1/field/spool/mychoicefield", + json={ + "name": "My choice field", + "field_type": "choice", + "choices": ["foo", "bar", "baz"], + "default_value": json.dumps(["foo"]), + "multi_choice": True, + }, + ) + assert_httpx_success(result) + + result = httpx.post( + f"{URL}/api/v1/field/spool/mychoicefield", + json={ + "name": "My choice field", + "field_type": "choice", + "choices": ["foo", "bar", "baz"], + "default_value": json.dumps("foo"), + "multi_choice": False, + }, + ) + assert result.status_code == 400 + assert result.json()["message"] == "Multi choice cannot be changed." + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/spool/mychoicefield") + assert_httpx_success(result) From a868e1889f6f8ccb8644b2fb6fac64a542080da3 Mon Sep 17 00:00:00 2001 From: Donkie Date: Sat, 6 Jan 2024 20:09:25 +0100 Subject: [PATCH 06/18] Added integration tests for using extra fields --- .../tests/fields/test_utilize.py | 156 ++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 tests_integration/tests/fields/test_utilize.py diff --git a/tests_integration/tests/fields/test_utilize.py b/tests_integration/tests/fields/test_utilize.py new file mode 100644 index 0000000..f7a370c --- /dev/null +++ b/tests_integration/tests/fields/test_utilize.py @@ -0,0 +1,156 @@ +"""Integration tests for the custom extra fields system.""" + +import json + +import httpx + +from ..conftest import assert_httpx_success + +URL = "http://spoolman:8000" + + +def test_add_vendor_with_extra_field(): + """Test adding a vendor with a custom field.""" + result = httpx.post( + f"{URL}/api/v1/field/vendor/mytextfield", + json={ + "name": "My text field", + "field_type": "text", + "default_value": json.dumps("Hello World"), + }, + ) + assert_httpx_success(result) + + result = httpx.post( + f"{URL}/api/v1/vendor", + json={ + "name": "My Vendor", + "extra": { + "mytextfield": '"My Value"', + }, + }, + ) + assert_httpx_success(result) + + # Verify + result = httpx.get(f"{URL}/api/v1/vendor/{result.json()['id']}") + assert_httpx_success(result) + vendor = result.json() + assert vendor["name"] == "My Vendor" + assert vendor["extra"] == {"mytextfield": '"My Value"'} + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/vendor/mytextfield") + assert_httpx_success(result) + + result = httpx.delete(f"{URL}/api/v1/vendor/{vendor['id']}") + assert_httpx_success(result) + + +def test_add_vendor_with_invalid_extra_field(): + """Test adding a vendor with an invalid custom field.""" + result = httpx.post( + f"{URL}/api/v1/vendor", + json={ + "name": "My Vendor", + "extra": { + "somefield": 42, + }, + }, + ) + assert result.status_code == 400 + assert "somefield" in result.json()["message"].lower() + + +def test_add_vendor_with_extra_field_then_delete(): + """Test adding a vendor with an extra field, then delete the field. + + Vendor GET response should then not contain the extra field. + """ + result = httpx.post( + f"{URL}/api/v1/field/vendor/mytextfield", + json={ + "name": "My text field", + "field_type": "text", + "default_value": json.dumps("Hello World"), + }, + ) + assert_httpx_success(result) + + result = httpx.post( + f"{URL}/api/v1/vendor", + json={ + "name": "My Vendor", + "extra": { + "mytextfield": '"My Value"', + }, + }, + ) + assert_httpx_success(result) + + # Verify + result = httpx.get(f"{URL}/api/v1/vendor/{result.json()['id']}") + assert_httpx_success(result) + vendor = result.json() + assert vendor["name"] == "My Vendor" + assert vendor["extra"] == {"mytextfield": '"My Value"'} + + # Remove field + result = httpx.delete(f"{URL}/api/v1/field/vendor/mytextfield") + assert_httpx_success(result) + + # Verify + result = httpx.get(f"{URL}/api/v1/vendor/{vendor['id']}") + assert_httpx_success(result) + vendor = result.json() + assert vendor["name"] == "My Vendor" + assert "extra" not in vendor or vendor["extra"] == {} + + result = httpx.delete(f"{URL}/api/v1/vendor/{vendor['id']}") + assert_httpx_success(result) + + +def test_update_existing_vendor_with_new_extra_field(): + """Test updating an existing vendor with a new extra field.""" + result = httpx.post( + f"{URL}/api/v1/vendor", + json={ + "name": "My Vendor", + }, + ) + assert_httpx_success(result) + vendor_id = result.json()["id"] + + result = httpx.post( + f"{URL}/api/v1/field/vendor/mytextfield", + json={ + "name": "My text field", + "field_type": "text", + "default_value": json.dumps("Hello World"), + }, + ) + assert_httpx_success(result) + + result = httpx.patch( + f"{URL}/api/v1/vendor/{vendor_id}", + json={ + "extra": { + "mytextfield": '"My Value"', + }, + }, + ) + assert_httpx_success(result) + + # Verify + result = httpx.get(f"{URL}/api/v1/vendor/{vendor_id}") + assert_httpx_success(result) + vendor = result.json() + assert vendor["name"] == "My Vendor" + assert vendor["extra"] == {"mytextfield": '"My Value"'} + + # Clean up + result = httpx.delete(f"{URL}/api/v1/field/vendor/mytextfield") + assert_httpx_success(result) + + result = httpx.delete(f"{URL}/api/v1/vendor/{vendor_id}") + assert_httpx_success(result) From 4adee1327b430a811d1a1213df01af9c5382fb64 Mon Sep 17 00:00:00 2001 From: Donkie Date: Sat, 6 Jan 2024 20:11:45 +0100 Subject: [PATCH 07/18] Ruff fix --- tests_integration/tests/spool/test_update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests_integration/tests/spool/test_update.py b/tests_integration/tests/spool/test_update.py index feaad44..ffcaac1 100644 --- a/tests_integration/tests/spool/test_update.py +++ b/tests_integration/tests/spool/test_update.py @@ -5,7 +5,7 @@ from typing import Any import httpx import pytest -from ..conftest import length_from_weight # noqa: TID252 +from ..conftest import length_from_weight URL = "http://spoolman:8000" From 9362d04dad6f4c0cf5eb96ed173b5c2fca8afc99 Mon Sep 17 00:00:00 2001 From: Donkie Date: Sat, 6 Jan 2024 20:20:26 +0100 Subject: [PATCH 08/18] Add caching for extra fields retrieval --- spoolman/extra_fields.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/spoolman/extra_fields.py b/spoolman/extra_fields.py index 43e451f..83f02e2 100644 --- a/spoolman/extra_fields.py +++ b/spoolman/extra_fields.py @@ -142,8 +142,14 @@ def validate_extra_field_dict(all_fields: list[ExtraField], fields_input: dict[s raise ValueError(f"Invalid extra field for key {key}: {e!s}") from None +extra_field_cache = {} + + async def get_extra_fields(db: AsyncSession, entity_type: EntityType) -> list[ExtraField]: """Get all extra fields for a specific entity type.""" + if entity_type in extra_field_cache: + return extra_field_cache[entity_type] + setting_def = parse_setting(f"extra_fields_{entity_type.name}") try: setting = await db_setting.get(db, setting_def) @@ -156,7 +162,9 @@ async def get_extra_fields(db: AsyncSession, entity_type: EntityType) -> list[Ex logger.warning("Setting %s is not a list, using default.", setting_def.key) setting_array = [] - return [ExtraField.parse_obj(obj) for obj in setting_array] + fields = [ExtraField.parse_obj(obj) for obj in setting_array] + extra_field_cache[entity_type] = fields + return fields async def add_or_update_extra_field(db: AsyncSession, entity_type: EntityType, extra_field: ExtraField) -> None: @@ -189,6 +197,9 @@ async def add_or_update_extra_field(db: AsyncSession, entity_type: EntityType, e setting_def = parse_setting(f"extra_fields_{entity_type.name}") await db_setting.update(db=db, definition=setting_def, value=json.dumps(jsonable_encoder(extra_fields))) + # Update cache + extra_field_cache[entity_type] = extra_fields + logger.info("Added/updated extra field %s for entity type %s.", extra_field.key, entity_type.name) @@ -205,6 +216,9 @@ async def delete_extra_field(db: AsyncSession, entity_type: EntityType, key: str setting_def = parse_setting(f"extra_fields_{entity_type.name}") await db_setting.update(db=db, definition=setting_def, value=json.dumps(jsonable_encoder(extra_fields))) + # Update cache + extra_field_cache[entity_type] = extra_fields + # Delete the extra field for all entities if entity_type == EntityType.vendor: await db_vendor.clear_extra_field(db, key) From 77e781f1ea9a4e2841c8acdce6130602101d9dc4 Mon Sep 17 00:00:00 2001 From: Donkie Date: Sun, 7 Jan 2024 11:11:57 +0100 Subject: [PATCH 09/18] Added settings menu with working currency editing --- client/public/locales/en/common.json | 15 +++- client/public/locales/sv/common.json | 15 +++- client/src/App.tsx | 18 ++++- .../pages/settings/ExtraFieldsSettings.tsx | 5 ++ client/src/pages/settings/GenericSettings.tsx | 79 +++++++++++++++++++ client/src/pages/settings/index.tsx | 64 +++++++++++++++ client/src/pages/settings/query.ts | 55 +++++++++++++ 7 files changed, 248 insertions(+), 3 deletions(-) create mode 100644 client/src/pages/settings/ExtraFieldsSettings.tsx create mode 100644 client/src/pages/settings/GenericSettings.tsx create mode 100644 client/src/pages/settings/index.tsx create mode 100644 client/src/pages/settings/query.ts diff --git a/client/public/locales/en/common.json b/client/public/locales/en/common.json index 23e3f97..23d06e0 100644 --- a/client/public/locales/en/common.json +++ b/client/public/locales/en/common.json @@ -41,7 +41,8 @@ "deleteError": "Error when deleting {{resource}} (status code: {{statusCode}})", "editSuccess": "Successfully edited {{resource}}", "editError": "Error when editing {{resource}} (status code: {{statusCode}})", - "importProgress": "Importing: {{processed}}/{{total}}" + "importProgress": "Importing: {{processed}}/{{total}}", + "saveSuccessful": "Save successful!" }, "kofi": "Tip me on Ko-fi", "loading": "Loading", @@ -252,6 +253,18 @@ "table": { "actions": "Actions" }, + "settings": { + "header": "Settings", + "generic": { + "tab": "Generic", + "currency": { + "label": "Currency" + } + }, + "extra_fields": { + "tab": "Extra Fields" + } + }, "documentTitle": { "default": "Spoolman", "suffix": " | Spoolman", diff --git a/client/public/locales/sv/common.json b/client/public/locales/sv/common.json index ba75663..a21903b 100644 --- a/client/public/locales/sv/common.json +++ b/client/public/locales/sv/common.json @@ -41,7 +41,8 @@ "deleteError": "Det gick inte att ta bort {{resource}} (statuskod: {{statusCode}})", "editSuccess": "Lyckades ändra {{resource}}", "editError": "Det gick inte att ändra {{resource}} (statuskod: {{statusCode}})", - "importProgress": "Importerar: {{processed}}/{{total}}" + "importProgress": "Importerar: {{processed}}/{{total}}", + "saveSuccessful": "Sparning lyckades!" }, "loading": "Laddar", "version": "Version", @@ -240,6 +241,18 @@ "table": { "actions": "Åtgärder" }, + "settings": { + "header": "Inställningar", + "generic": { + "tab": "Allmänt", + "currency": { + "label": "Valuta" + } + }, + "extra_fields": { + "tab": "Extra Fält" + } + }, "documentTitle": { "default": "Spoolman", "suffix": " | Spoolman", diff --git a/client/src/App.tsx b/client/src/App.tsx index bc2c03e..550fbcb 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -10,7 +10,14 @@ import dataProvider from "./components/dataProvider"; import { useTranslation } from "react-i18next"; import { BrowserRouter, Outlet, Route, Routes } from "react-router-dom"; import { ColorModeContextProvider } from "./contexts/color-mode"; -import { FileOutlined, HighlightOutlined, HomeOutlined, QuestionOutlined, UserOutlined } from "@ant-design/icons"; +import { + FileOutlined, + HighlightOutlined, + HomeOutlined, + QuestionOutlined, + ToolOutlined, + UserOutlined, +} from "@ant-design/icons"; import { ConfigProvider } from "antd"; import React from "react"; import { Locale } from "antd/es/locale"; @@ -139,6 +146,14 @@ function App() { icon: , }, }, + { + name: "settings", + list: "/settings", + meta: { + canDelete: false, + icon: , + }, + }, { name: "help", list: "/help", @@ -202,6 +217,7 @@ function App() { } /> } /> + } /> } /> } /> diff --git a/client/src/pages/settings/ExtraFieldsSettings.tsx b/client/src/pages/settings/ExtraFieldsSettings.tsx new file mode 100644 index 0000000..95bbec5 --- /dev/null +++ b/client/src/pages/settings/ExtraFieldsSettings.tsx @@ -0,0 +1,5 @@ +import React from "react"; + +export function ExtraFieldsSettings() { + return <>Extra Fields Settings Content; +} diff --git a/client/src/pages/settings/GenericSettings.tsx b/client/src/pages/settings/GenericSettings.tsx new file mode 100644 index 0000000..f0e1adc --- /dev/null +++ b/client/src/pages/settings/GenericSettings.tsx @@ -0,0 +1,79 @@ +import React from "react"; +import { useTranslate } from "@refinedev/core"; +import { Button, Form, Input, message } from "antd"; +import { useGetSettings, useSetSetting } from "./query"; + +export function GenericSettings() { + const settings = useGetSettings(); + const setSetting = useSetSetting(); + const [form] = Form.useForm(); + const [messageApi, contextHolder] = message.useMessage(); + const t = useTranslate(); + + // Set initial form values + React.useEffect(() => { + if (settings.data) { + form.setFieldsValue({ + currency: JSON.parse(settings.data.currency.value), + }); + } + }, [settings.data, form]); + + // Popup message if setSetting is successful + React.useEffect(() => { + if (setSetting.isSuccess) { + messageApi.success(t("notifications.saveSuccessful")); + } + }, [setSetting.isSuccess, messageApi, t]); + + // Handle form submit + const onFinish = (values: { currency: string }) => { + // Check if the currency has changed + if (settings.data?.currency.value !== JSON.stringify(values.currency)) { + setSetting.mutate({ + key: "currency", + value: JSON.stringify(values.currency), + }); + } + }; + + return ( + <> +
+ + + + + + + +
+ {contextHolder} + + ); +} diff --git a/client/src/pages/settings/index.tsx b/client/src/pages/settings/index.tsx new file mode 100644 index 0000000..575f067 --- /dev/null +++ b/client/src/pages/settings/index.tsx @@ -0,0 +1,64 @@ +import React from "react"; +import { IResourceComponentsProps, useTranslate } from "@refinedev/core"; +import dayjs from "dayjs"; +import utc from "dayjs/plugin/utc"; +import { Content } from "antd/es/layout/layout"; +import { Menu, theme } from "antd"; +import { SolutionOutlined, ToolOutlined } from "@ant-design/icons"; +import { useSavedState } from "../../utils/saveload"; +import { GenericSettings } from "./GenericSettings"; +import { ExtraFieldsSettings } from "./ExtraFieldsSettings"; + +dayjs.extend(utc); + +const { useToken } = theme; + +export const Settings: React.FC = () => { + const { token } = useToken(); + const t = useTranslate(); + const [current, setCurrent] = useSavedState("settings-tab", "generic"); + + return ( + <> +

+ {t("settings.header")} +

+ + setCurrent(key)} + items={[ + { key: "generic", label: t("settings.generic.tab"), icon: }, + { key: "extra", label: t("settings.extra_fields.tab"), icon: }, + ]} + style={{ + marginBottom: "3em", + }} + /> +
+ {current === "generic" && } + {current === "extra" && } +
+ + + ); +}; + +export default Settings; diff --git a/client/src/pages/settings/query.ts b/client/src/pages/settings/query.ts new file mode 100644 index 0000000..63ce94a --- /dev/null +++ b/client/src/pages/settings/query.ts @@ -0,0 +1,55 @@ +import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; + +interface SettingResponseValue { + value: string; + is_set: boolean; + type: string; +} + +interface SettingsResponse { + [key: string]: SettingResponseValue; +} + +export function useGetSettings() { + const apiEndpoint = import.meta.env.VITE_APIURL; + return useQuery({ + queryKey: ["settings"], + queryFn: async () => { + const response = await fetch(`${apiEndpoint}/setting`); + return response.json(); + }, + }); +} + +export function useGetSetting(key: string) { + const apiEndpoint = import.meta.env.VITE_APIURL; + return useQuery({ + queryKey: ["settings", key], + queryFn: async () => { + const response = await fetch(`${apiEndpoint}/setting/${key}`); + return response.json(); + }, + }); +} + +export function useSetSetting() { + const queryClient = useQueryClient(); + + const apiEndpoint = import.meta.env.VITE_APIURL; + return useMutation({ + mutationFn: async ({ key, value }) => { + const response = await fetch(`${apiEndpoint}/setting/${key}`, { + method: "POST", + headers: { + "Content-Type": "application/json", + }, + body: JSON.stringify(value), + }); + return response.json(); + }, + onSuccess: (_data, { key }) => { + // Invalidate and refetch + queryClient.invalidateQueries(["settings", key]); + }, + }); +} From 483c48151bcd736b8c90bb91b1fb579f3dcecc1a Mon Sep 17 00:00:00 2001 From: Donkie Date: Sun, 14 Jan 2024 22:18:01 +0100 Subject: [PATCH 10/18] Work on extra fields settings UI Almost finished --- client/public/locales/en/common.json | 34 +- client/src/App.tsx | 2 +- .../pages/settings/ExtraFieldsSettings.tsx | 553 +++++++++++++++++- ...enericSettings.tsx => GeneralSettings.tsx} | 6 +- client/src/pages/settings/index.tsx | 57 +- client/src/pages/settings/queryFields.ts | 130 ++++ .../settings/{query.ts => querySettings.ts} | 6 + 7 files changed, 767 insertions(+), 21 deletions(-) rename client/src/pages/settings/{GenericSettings.tsx => GeneralSettings.tsx} (92%) create mode 100644 client/src/pages/settings/queryFields.ts rename client/src/pages/settings/{query.ts => querySettings.ts} (91%) diff --git a/client/public/locales/en/common.json b/client/public/locales/en/common.json index 23d06e0..166a062 100644 --- a/client/public/locales/en/common.json +++ b/client/public/locales/en/common.json @@ -42,7 +42,8 @@ "editSuccess": "Successfully edited {{resource}}", "editError": "Error when editing {{resource}} (status code: {{statusCode}})", "importProgress": "Importing: {{processed}}/{{total}}", - "saveSuccessful": "Save successful!" + "saveSuccessful": "Save successful!", + "validationError": "Validation error: {{error}}" }, "kofi": "Tip me on Ko-fi", "loading": "Loading", @@ -255,14 +256,39 @@ }, "settings": { "header": "Settings", - "generic": { - "tab": "Generic", + "general": { + "tab": "General", "currency": { "label": "Currency" } }, "extra_fields": { - "tab": "Extra Fields" + "tab": "Extra Fields", + "description": "

Here you can add extra custom fields to your entities.

Once a field is added, you can not change its key or type, and for choice type fields you can not remove choices or change the multi select state. If you remove a field, the associated data for all entities will be deleted.

Extra fields can not be sorted or filtered in the table views.

", + "params": { + "key": "Key", + "name": "Name", + "field_type": "Type", + "unit": "Unit", + "default_value": "Default Value", + "choices": "Choices", + "multi_choice": "Multi Choice" + }, + "field_type": { + "text": "Text", + "integer": "Integer", + "integer_range": "Integer Range", + "float": "Float", + "float_range": "Float Range", + "datetime": "Datetime", + "boolean": "Boolean", + "choice": "Choice" + }, + "boolean_true": "Yes", + "boolean_false": "No", + "choices_missing_error": "You cannot remove choices. The following choices are missing: {{choices}}", + "non_unique_key_error": "The key must be unique.", + "key_not_changed": "Please change the key to something else." } }, "documentTitle": { diff --git a/client/src/App.tsx b/client/src/App.tsx index 550fbcb..fbcddf3 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -217,7 +217,7 @@ function App() { } /> } /> - } /> + } /> } /> } /> diff --git a/client/src/pages/settings/ExtraFieldsSettings.tsx b/client/src/pages/settings/ExtraFieldsSettings.tsx index 95bbec5..10653b4 100644 --- a/client/src/pages/settings/ExtraFieldsSettings.tsx +++ b/client/src/pages/settings/ExtraFieldsSettings.tsx @@ -1,5 +1,554 @@ -import React from "react"; +import { + Button, + Checkbox, + DatePicker, + Flex, + Form, + FormInstance, + Input, + InputNumber, + Select, + Table, + Typography, + message, +} from "antd"; +import { EntityType, Field, FieldType, useGetFields, useSetField } from "./queryFields"; +import { useTranslate } from "@refinedev/core"; +import { useState } from "react"; +import { ColumnType } from "antd/es/table"; +import { Trans } from "react-i18next"; +import { useParams } from "react-router-dom"; +import { FormItemProps, Rule } from "antd/es/form"; +import { PlusOutlined } from "@ant-design/icons"; + +interface EditableCellProps extends React.HTMLAttributes { + record: FieldHolder; + editing: boolean; + dataIndex: string; + form: FormInstance; + children: React.ReactNode; +} + +interface ColumnProps extends ColumnType { + editable: boolean; + required: boolean; +} + +interface FieldHolder { + key: string; + field: Field; + is_new: boolean; +} + +const EditableCell: React.FC = ({ record, editing, dataIndex, children, form, ...restProps }) => { + const t = useTranslate(); + + if (!editing) { + return {children}; + } + + const fieldType = form.getFieldValue("field_type") as FieldType; + const choices = form.getFieldValue("choices") as string[]; + + const title = t(`settings.extra_fields.params.${dataIndex}`); + + let inputNode; + const rules: Rule[] = []; + const formItemProps: FormItemProps = {}; + if (dataIndex === "key") { + inputNode = ; + rules.push({ + required: true, + min: 1, + pattern: /^[a-z0-9_]+$/, + }); + rules.push({ + validator: async (_, value) => { + // Ensure key is not new_field + if (value === "new_field") { + throw new Error(t("settings.extra_fields.key_not_changed")); + } + }, + }); + } else if (dataIndex === "field_type") { + inputNode = ( + ; + rules.push({ + required: true, + min: 1, + }); + } else if (dataIndex === "unit") { + if ( + fieldType === FieldType.integer || + fieldType === FieldType.integer_range || + fieldType === FieldType.float || + fieldType === FieldType.float_range + ) { + inputNode = ; + } else { + inputNode = null; + } + rules.push({ + required: false, + }); + } else if (dataIndex === "default_value") { + if (fieldType === FieldType.boolean) { + inputNode = ; + formItemProps.valuePropName = "checked"; + rules.push({ + type: "boolean", + }); + } else if (fieldType === FieldType.text) { + inputNode = ; + rules.push({ + type: "string", + }); + } else if (fieldType === FieldType.integer) { + inputNode = ; + rules.push({ + type: "integer", + }); + } else if (fieldType === FieldType.float) { + inputNode = ; + rules.push({ + type: "float", + }); + } else if (fieldType === FieldType.integer_range) { + inputNode = ; + rules.push({ + type: "string", + pattern: /^-?\d+\s*-\s*-?\d+$/, + }); + } else if (fieldType === FieldType.float_range) { + inputNode = ; + rules.push({ + type: "string", + pattern: /^-?\d+([.,]\d+)?\s*-\s*-?\d+([.,]\d+)?$/, + }); + } else if (fieldType === FieldType.datetime) { + inputNode = ; + } else if (fieldType === FieldType.choice) { + inputNode = ( + ; + rules.push({ + required: true, + min: 1, + type: "array", + validator: async (_, value) => { + // Verify that all values in record.choices are in value + const recordChoices = record.field.choices || []; + const valueChoices = value || []; + const missingChoices = recordChoices.filter((choice) => !valueChoices.includes(choice)); + if (missingChoices.length > 0) { + throw new Error( + t("settings.extra_fields.choices_missing_error", { + choices: missingChoices.join(", "), + }) + ); + } + }, + }); + } else { + inputNode = null; + } + } else if (dataIndex === "multi_choice") { + if (fieldType === FieldType.choice) { + inputNode = ( + { + // Reset default_value when changing multi_choice + form.setFieldsValue({ + default_value: undefined, + }); + }} + > + {title} + + ); + formItemProps.valuePropName = "checked"; + rules.push({ + type: "boolean", + }); + } else { + inputNode = null; + } + } else { + inputNode = null; + } + + const formItem = inputNode ? ( + + {inputNode} + + ) : null; + + return {formItem}; +}; export function ExtraFieldsSettings() { - return <>Extra Fields Settings Content; + const { entityType } = useParams<{ entityType: EntityType }>(); + const t = useTranslate(); + const [form] = Form.useForm(); + const fields = useGetFields(entityType as EntityType); + const setField = useSetField(entityType as EntityType); + const [isSubmitting, setIsSubmitting] = useState(false); + const [newField, setNewField] = useState(null); + + const [messageApi, contextHolder] = message.useMessage(); + + const [editingKey, setEditingKey] = useState(""); + + const isEditing = (record: FieldHolder) => record.field.key === editingKey; + + const edit = (record: Partial & { key: React.Key }) => { + const values = { ...record }; + console.log(values); + if (values.default_value) { + values.default_value = JSON.parse(values.default_value); + } else { + values.default_value = ""; + } + form.setFieldsValue(values); + setEditingKey(record.key); + }; + + const cancel = () => { + setEditingKey(""); + }; + + const save = async (_key: React.Key) => { + let row; + try { + row = (await form.validateFields()) as Field; + } catch (errInfo) { + // Ignore these errors because they are already handled by the form + return; + } + + // Do some value conversions + try { + // Convert float and integer range to array using the validation regex + if (row.field_type === FieldType.float_range && typeof row.default_value === "string") { + const pattern = /^(-?\d+(?:[.,]\d+)?)\s*-\s*(-?\d+(?:[.,]\d+)?)$/; + const matches = row.default_value.match(pattern); + if (matches) { + const val1 = parseFloat(matches[1].replace(",", ".")); + const val2 = parseFloat(matches[2].replace(",", ".")); + row.default_value = JSON.stringify([Math.min(val1, val2), Math.max(val1, val2)]); + } + } else if (row.field_type === FieldType.integer_range && typeof row.default_value === "string") { + const pattern = /^(-?\d+)\s*-\s*(-?\d+)$/; + const matches = row.default_value.match(pattern); + if (matches) { + const val1 = parseInt(matches[1]); + const val2 = parseInt(matches[2]); + row.default_value = JSON.stringify([Math.min(val1, val2), Math.max(val1, val2)]); + } + } else { + // Just stringify all other values + row.default_value = JSON.stringify(row.default_value); + } + + // Set multi_choice if it's not set and field_type is choice + if (row.field_type === FieldType.choice && row.multi_choice === undefined) { + row.multi_choice = false; + } + + // If unit is an empty string, set it to null instead + if (row.unit === "") { + row.unit = undefined; + } + } catch (errInfo) { + if (errInfo instanceof Error) { + messageApi.error(t("notifications.validationError", { error: errInfo.message })); + // Check if errInfo has the errorFields property, then we should skip the error message + } + return; + } + + // Validate that row.key is unique and not among the other keys + const keys = new Set(fields.data?.map((field) => field.key) || []); + if (keys.has(row.key)) { + messageApi.error(t("settings.extra_fields.non_unique_key_error")); + return; + } + + // Submit it! + try { + console.log(row); + + setIsSubmitting(true); + + setField.reset(); + + await setField.mutateAsync({ + key: row.key, + params: row, + }); + + setEditingKey(""); + setNewField(null); + } catch (errInfo) { + if (errInfo instanceof Error) { + messageApi.error(errInfo.message); + } + } + setIsSubmitting(false); + }; + + const niceName = t(`${entityType}.${entityType}`); + + const columns: ColumnProps[] = [ + { + title: t("settings.extra_fields.params.key"), + dataIndex: ["field", "key"], + key: "key", + editable: false, + required: true, + }, + { + title: t("settings.extra_fields.params.name"), + dataIndex: ["field", "name"], + editable: true, + required: true, + }, + { + title: t("settings.extra_fields.params.field_type"), + dataIndex: ["field", "field_type"], + editable: false, + required: true, + render(value) { + return t(`settings.extra_fields.field_type.${value}`); + }, + }, + { + title: t("settings.extra_fields.params.unit"), + dataIndex: ["field", "unit"], + editable: true, + required: false, + }, + { + title: t("settings.extra_fields.params.default_value"), + dataIndex: ["field", "default_value"], + editable: true, + required: false, + render(value, record) { + const val = JSON.parse(value || "null"); + if (typeof val === "boolean") { + return val ? t("settings.extra_fields.boolean_true") : t("settings.extra_fields.boolean_false"); + } else if (typeof val === "number" || typeof val === "string") { + return val; + } else if (Array.isArray(val) && record.field.field_type === FieldType.choice) { + return val.join(", "); + } else if ( + (Array.isArray(val) && record.field.field_type === FieldType.integer_range) || + record.field.field_type === FieldType.float_range + ) { + return `${val[0]} - ${val[1]}`; + } else { + return null; + } + }, + }, + { + title: t("settings.extra_fields.params.choices"), + dataIndex: ["field", "choices"], + editable: true, + required: false, + render(value, record) { + if (record.field.field_type === FieldType.choice && Array.isArray(value)) { + return value.join(", "); + } else { + return null; + } + }, + }, + { + title: t("settings.extra_fields.params.multi_choice"), + dataIndex: ["field", "multi_choice"], + editable: false, + required: false, + render(value, record) { + if (record.field.field_type === FieldType.choice) { + return value ? t("settings.extra_fields.boolean_true") : t("settings.extra_fields.boolean_false"); + } else { + return null; + } + }, + }, + { + title: "", + dataIndex: "operation", + editable: false, + required: false, + render: (_: unknown, record: FieldHolder) => { + const editable = isEditing(record); + return editable ? ( + + save(record.field.key)} style={{ marginRight: 8 }}> + Save + + cancel()}>Cancel + + ) : ( + edit(record.field)}> + Edit + + ); + }, + }, + ]; + + const mergedColumns = columns.map((col) => { + if (col.dataIndex === "operation") { + return col; + } + return { + ...col, + onCell: function (record: FieldHolder): EditableCellProps { + return { + record, + editing: isEditing(record), + dataIndex: (col.dataIndex?.toString() || "").split(",").pop() || "", + form: form, + children: [], + }; + }, + }; + }); + + const tableFields: FieldHolder[] = [ + ...(fields.data || []).map((field) => ({ + key: field.key, + field, + is_new: false, + })), + ...(newField ? [newField] : []), + ]; + + return ( + <> +

+ {t("settings.extra_fields.tab")} - {niceName} +

+ , + }} + /> +
+ + + {newField == null && ( + + ; + if (!editing || !canEditField(dataIndex, record.is_new)) { + return ( + + ); } const fieldType = form.getFieldValue("field_type") as FieldType; @@ -60,6 +83,7 @@ const EditableCell: React.FC = ({ record, editing, dataIndex, rules.push({ required: true, min: 1, + max: 64, pattern: /^[a-z0-9_]+$/, }); rules.push({ @@ -123,6 +147,14 @@ const EditableCell: React.FC = ({ record, editing, dataIndex, rules.push({ required: true, min: 1, + max: 128, + }); + } else if (dataIndex === "order") { + inputNode = ; + rules.push({ + required: true, + min: 0, + type: "integer", }); } else if (dataIndex === "unit") { if ( @@ -131,12 +163,13 @@ const EditableCell: React.FC = ({ record, editing, dataIndex, fieldType === FieldType.float || fieldType === FieldType.float_range ) { - inputNode = ; + inputNode = ; } else { inputNode = null; } rules.push({ required: false, + max: 16, }); } else if (dataIndex === "default_value") { if (fieldType === FieldType.boolean) { @@ -158,7 +191,7 @@ const EditableCell: React.FC = ({ record, editing, dataIndex, } else if (fieldType === FieldType.float) { inputNode = ; rules.push({ - type: "float", + type: "number", }); } else if (fieldType === FieldType.integer_range) { inputNode = ; @@ -173,7 +206,7 @@ const EditableCell: React.FC = ({ record, editing, dataIndex, pattern: /^-?\d+([.,]\d+)?\s*-\s*-?\d+([.,]\d+)?$/, }); } else if (fieldType === FieldType.datetime) { - inputNode = ; + inputNode = ; } else if (fieldType === FieldType.choice) { inputNode = ( ; + rules.push({ + type: "string", + }); + } else if (field.field_type === FieldType.datetime) { + inputNode = ; + } else if (field.field_type === FieldType.boolean) { + inputNode = ; + formItemProps.valuePropName = "checked"; + rules.push({ + type: "boolean", + }); + } else if (field.field_type === FieldType.choice) { + inputNode = ( +
{children} + {children} +