piccolo-orm / piccolo_admin

A powerful web admin for your database.
https://piccolo-orm.com/ecosystem/
MIT License
321 stars 39 forks source link

Cannot edit non-default primary key in admin #271

Open PawelRoman opened 1 year ago

PawelRoman commented 1 year ago

Hi,

I have a model where a PK is defined on an explicit Varchar field, like this

class MyModel(Table):
    identifier = columns.Varchar(length=64, primary_key=True)
   .....  other fields .....

It's entirely on purpose. I don't want the PK to be an auto-incremented integer. I want a random string as PK. ORM allows for that, which is good.

The problem is trying to add those objects via admin. Admin UI does not show any textbox to enter the PK value and it is not possible to add those objects.

Apparently admin assumes that primary key column is always an auto-incremented integer. But ORM allows to have PK on non-int non-incrementable column.

Any advice how to work around this problem?

sinisaos commented 1 year ago

@PawelRoman Piccolo Admin does not provide a primary key column in forms. The primary key is automatically generated. To achieve what you want, you need to provide a random string as the default value like this

import random
import string

def random_string():
    return "".join(random.choices(string.ascii_lowercase, k=64))

class MyModel(Table):
    identifier = Varchar(
        default=random_string,
        length=64,
        primary_key=True,
    )

Hope this helps.

PawelRoman commented 1 year ago

Sorry, I kind of simplified it. I don't want any defaults. I want the field to be explicitly required. The very nature of this field is that it must be supplied by the caller on creating object (doesn't matter if creating it programmatically or in admin). It just cannot be happily generated as random string by the back-end.

In django, it'd do that by having null=False and no default defined.

sinisaos commented 1 year ago

@PawelRoman Ok, but as I wrote earlier Piccolo Admin does not provide a primary key column in forms. The primary key is automatically generated because forms are generated from Pydantic schema and primary key is excluded By removing the exclude_columns line (you can try patching your local Piccolo API installation), your column will be displayed in the form (Serial primary key remain hidden which is good, but the UUID primary key is displayed in forms and that's not good), but such changes can only be approved by @dantownsend. UPDATE: We can also try something like this to hide the Serial or UUID primary key in the forms and show all the other columns

@property
def pydantic_model(self) -> t.Type[pydantic.BaseModel]:
    """
    Useful for serialising inbound data from POST and PUT requests.
    """
    if isinstance(self.table._meta.primary_key, (UUID, Serial)):
        exclude_columns = (self.table._meta.primary_key,)
    else:
        exclude_columns = ()
    return create_pydantic_model(
        self.table,
        model_name=f"{self.table.__name__}In",
        exclude_columns=exclude_columns,
        **self.schema_extra,
    )
BezBartek commented 1 year ago

@sinisaos @PawelRoman Two thoughts:

  1. Excluding of the primary key should be applied only if the primary key is auto-increment type, otherwise is always a problem. I think that we should propose that as a new feature/fix (What is your opinion @dantownsend? Thank you :))
  2. I viewed a code that you are referring to. It seems that there is no distinguish in used pydantic models between put and post methods in admin. Let's consider fields that a user would like to treat as read only. They should be presented in create context, but hidden in edit context.
sinisaos commented 1 year ago
  1. Excluding of the primary key should be applied only if the primary key is auto-increment type, otherwise is always a problem. I think that we should propose that as a new feature/fix (What is your opinion @dantownsend? Thank you :))

As I wrote before, this can be achieved (hide Serial autoincrement and UUID primary key in forms, and all other column types will be shown in forms) with something like this

@property
def pydantic_model(self) -> t.Type[pydantic.BaseModel]:
    """
    Useful for serialising inbound data from POST and PUT requests.
    """
    return create_pydantic_model(
        self.table,
        model_name=f"{self.table.__name__}In",
        exclude_columns=(
            (self.table._meta.primary_key,)
            if isinstance(self.table._meta.primary_key, (UUID, Serial))
            else ()
        ),
        **self.schema_extra,
    )
  1. I viewed a code that you are referring to. It seems that there is no distinguish in used pydantic models between put and post methods in admin. Let's consider fields that a user would like to treat as read only. They should be presented in create context, but hidden in edit context.

Yes, you're right. We currently do not have a read-only column option. In Django we have an option to override get_readonly_fields() to achieve that

class SomeModelAdmin(admin.ModelAdmin):
    readonly_fields = ("some-readonly-field",)

    def get_readonly_fields(self, request, obj=None):
        if obj: # hide when editing, show when posting
            return self.readonly_fields
        return ()