Implement TableConfig class

[dantownsend]

Currently, create_admin accepts a list of Table subclasses. This can be modified to accept either Table subclasses, or instances of a new class called TableConfig.

TableConfig will be a dataclass which accepts a table argument, as well as a bunch of configuration options. For example:

@dataclasses.dataclass
class TableConfig:
    """
    :param list_columns:
        Used to specify which columns are shown on the list page. If None,
        all are shown.
    :param list_filters:
        Used to specify which filters are shown on the list page. If None,
        all are shown.

    """
    table: Type[Table]
    list_columns: Optional[List[Union[str, Column]] = None
    list_filters: Optional[List[Union[str, Column]] = None
    # Plus some additional options, which haven't been finalised.
    ...

app = create_admin(
    tables=[
        SomeTable,
        TableConfig(
            table=MyTable,
            list_columns=[MyTable.name],
            list_filters=[MyTable.name]
        )
    ]
)

It will allow more fine grained control of the how the admin looks.

[sinisaos]

@dantownsend I'm experimenting with this and this feature could be useful. Something like this can change columns we want to display in admin and that's fine for listing,

@dataclass
class TableConfig:
    """
    :param list_columns:
        Used to specify which columns are shown on the list page. If None,
        all are shown.
    :param list_filters:
        Used to specify which filters are shown on the list page. If None,
        all are shown.
    """
    
    table: t.Type[Table]
    list_columns: t.Optional[t.List[Column]] = None
    # list_filters:  t.Optional[t.List[Column]] = None
    
    def __post_init__(self):
        """
        Display the columns in the admin UI and make shure
        that primary key is always included for crud operations
        and FK column to avoid OperationalError
        """
        if self.list_columns is None:
            self.table._meta.columns = self.table._meta.columns
        else:
            self.table._meta.columns = [
                column
                for column in itertools.chain(
                    self.table._meta.default_columns,
                    self.list_columns,
                    self.table._meta.foreign_key_columns,
                )
                if column in self.table._meta.columns
            ]

but it's not good for adding end editing records because the changed table (from __post_init__) goes into PiccoloCRUD and PiccoloCRUD create model based on columns in TableConfig list_columns and another table column is missing. Do you have some advice how to solve this or we can show/hide columns and filters in frontend. Thanks in advance.

[dantownsend]

@sinisaos Yeah, it's a tricky one.

Rather than excluding the columns from the Piccolo table, it might be better to use an unmodified Piccolo table.

Then add an extra field to the /api/tables/some_table/schema response called visible_columns.

{
"title": "DirectorIn",
"type": "object",
"properties": {},
"help_text": "The main director for a movie.",
"visible_columns": ["id", "name"]
}

The UI will then work as before - we just need a check on the listing page if the column is in visible_columns.

For that to work we'll have to modify create_pydantic_model so it accepts an extra argument: We could then pass in something like:

create_pydantic_model(extra={'visible_columns': ['id', 'name']})

I thought about having an entirely new endpoint like /api/tables/some_table/config, but it seems unnecessary if we can manage this using the existing schema endpoint.

[sinisaos]

@dantownsend Great. Thanks. I'll try that approach.

[dantownsend]

@sinisaos The only downside with this approach is we're over fetching from the API. Imagine the table has 20 columns, and we just want to display 2 of them, then we're still fetching all 20.

I don't think it's a big deal initially.

But down the road we could modify PiccoloCRUD's get_all method, so we can ask it for a subset of columns. Something like:

/api/tables/movie?columns=id,name

[sinisaos]

@dantownsend You're right, but anyhow we need to fetch all the columns to be able to edit and add new records. I will play with this, and if I need help or advice, I will write here.

[sinisaos]

@dantownsend Trying your approach, I got stuck. So far I have added visible columns to pydantic.py

def create_pydantic_model(
    ...
    deserialize_json: bool = False,
    visible_columns: t.Tuple[str] = (),
) -> t.Type[pydantic.BaseModel]

  class CustomConfig(Config):
      schema_extra = {
          "help_text": table._meta.help_text,
          "visible_columns": visible_columns, # we also can pass tuple(i._meta.name for i in table._meta.columns) to get all columns visible by default
      }

I also added TableConfig to Piccolo Admin endpoints.py to change list of visible columns

@dataclass
class TableConfig:
    """
    :param list_columns:
        Used to specify which columns are shown on the list page. If None,
        all are shown.
    """
    
    table: t.Type[Table]
    list_columns: t.List[Column] = field(default_factory=list)

    def __post_init__(self):
        """
        Display the columns in the admin UI and make shure
        that primary key and FK column is always included
        """
        self.list_columns = [
            column
            for column in itertools.chain(
                self.table._meta.default_columns,
                self.list_columns,
                self.table._meta.foreign_key_columns,
            )
        ]
        return self.list_columns

I can change the PiccoloCRUD pydantic_model in the tables loop and print the correct schema, but how to change only the TableConfig table and leave the rest tables unchanged

# endpoints py
piccolo_crud: PiccoloCRUD = PiccoloCRUD

for table in tables:
    if isinstance(table, TableConfig):
        piccolo_crud.pydantic_model = create_pydantic_model(
            table.table,
            model_name=f"{table.table.__name__}In",
            visible_columns=tuple(
                i._meta.name for i in table.list_columns
            ),
        )
        print(piccolo_crud.pydantic_model.schema_json(indent=2))
    else:
        piccolo_crud.pydantic_model = create_pydantic_model(
            table,
            model_name=f"{table.__name__}In",
            visible_columns=tuple(
                i._meta.name for i in table._meta.columns
            ),
        )
        print(piccolo_crud.pydantic_model.schema_json(indent=2))

I also changed the cellNames function to RowListing.vue to list only visible columns

cellNames() {
    const keys = []
    for (const key in this.rows[0]) {
        if (!key.endsWith("_readable")) {
            keys.push(key)
        }
    }
    const columnsDifference = keys.filter((column) =>
        this.schema.visible_columns.includes(column)
    )
    return columnsDifference
  },

Sorry for long comment.

[sinisaos]

@dantownsend Ideally change for TableConfig will happen here, but I don’t know how. Something like this

@property
def pydantic_model(self) -> t.Type[pydantic.BaseModel]:
    """
    Useful for serialising inbound data from POST and PUT requests.
    """
    if something:
        visible_columns = visible_columns=tuple(
                i._meta.name for i in table.list_columns
            ),
    else:
        visible_columns=tuple(
                i._meta.name for i in table._meta.columns
            ),
    return create_pydantic_model(
        self.table, model_name=f"{self.table.__name__}In",
        visible_columns=visible_columns,
    )

Another approach could be to add visible_columns in TableMetaclass and then user could add visible columns in table definition. Something like this.

class Director(Table, help_text="The main director for a movie."):
    name = Varchar(length=300, null=False)
    gender = Varchar(length=1, choices=Gender)

    @classmethod
    def get_readable(cls):
        return Readable(template="%s", columns=[cls.name])

    @classmethod
    def set_visible(cls):
        return [cls.name] # some like this and then we can pass it to pydantic.py visible columns

Then we dont need TableConfig

[dantownsend]

@sinisaos I like the idea of the TableConfig because in the future we could allow the user to customise all sorts of things (e.g. overriding the name which appears in the UI).

When tables are passed to create_admin, I think they should all be converted to TableConfig. It will make the code cleaner.

# in AdminRouter.__init__

self.tables = [table if isinstance(TableConfig) else TableConfig(table=table) for table in tables)

[dantownsend]

You should be able to simplify TableConfig so it's just this:

@dataclass
class TableConfig:
    """
    :param list_columns:
        Used to specify which columns are shown on the list page. If None,
        all are shown.
    """
    
    table: t.Type[Table]
    list_columns: t.List[Column] = field(default_factory=list)

    def __post_init__(self):
        """
        Display the columns in the admin UI and make shure
        that primary key and FK column is always included
        """
       if not self.list_columns:
           self.list_columns = self.table._meta.columns

That should simplify things.

[sinisaos]

@dantownsend Thanks. I'll try that during the day.

[sinisaos]

@dantownsend In order for your example to work I had to change

def create_pydantic_model(
    ...
    deserialize_json: bool = False,
    visible_columns: t.Tuple[str] = (),
) -> t.Type[pydantic.BaseModel]

  class CustomConfig(Config):
      schema_extra = {
          "help_text": table._meta.help_text,
          "visible_columns": tuple(
                i._meta.name for i in table._meta.columns
            ),
      }

to pass all columns to visible_columns but the unfortunately problem is stay the same. I don't able to change visible_columns for table with columns in list_columns. The only way I was able to change the schema is to change the pydantic_model property object in the tables loop,

# endpoints py
piccolo_crud: PiccoloCRUD = PiccoloCRUD

for table in tables:
    if isinstance(table, TableConfig):
        piccolo_crud.pydantic_model = create_pydantic_model(
            table.table,
            model_name=f"{table.table.__name__}In",
            visible_columns=tuple(
                i._meta.name for i in table.list_columns
            ),
        )
        # print(piccolo_crud.pydantic_model.schema_json(indent=2))
    else:
        piccolo_crud.pydantic_model = create_pydantic_model(
            table,
            model_name=f"{table.__name__}In",
            visible_columns=tuple(
                i._meta.name for i in table._meta.columns
            ),
        )
        # print(piccolo_crud.pydantic_model.schema_json(indent=2))

but it's a loop and only the last result from the loop is returned, and all the tables change to the last result schema, which is not good.

[dantownsend]

@sinisaos I think this might be the issue:

def create_pydantic_model(
    ...
    deserialize_json: bool = False,
    visible_columns: t.Tuple[str] = (),
) -> t.Type[pydantic.BaseModel]

  class CustomConfig(Config):
      schema_extra = {
          "help_text": table._meta.help_text,
          # You should have to do this - just use the visible_columns value passed into the function.
          "visible_columns": tuple(
                i._meta.name for i in table._meta.columns
            ),
      }

It can just be:

def create_pydantic_model(
    ...
    deserialize_json: bool = False,
    visible_columns: t.Tuple[str] = (),
) -> t.Type[pydantic.BaseModel]

  class CustomConfig(Config):
      schema_extra = {
          "help_text": table._meta.help_text,
          "visible_columns":  visible_columns
      }

Does that help?

[sinisaos]

@dantownsend Unfortunately no. Without this visible_columns

class CustomConfig(Config):
      schema_extra = {
          "help_text": table._meta.help_text,
          # You should have to do this - just use the visible_columns value passed into the function.
          "visible_columns": tuple(
                i._meta.name for i in table._meta.columns
            ),
      }

is empty. We need to do that or add visible columns in here.

# in PiccoloCRUD class in piccolo_api.endpoints.crud
@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",
        visible_columns=tuple(
            i._meta.name for i in self.table._meta.columns
        ),
    )

I think if we manage somehow (I don't know how) to pass TableConfig list_columns to pydantic_model will solve the problem because we can pass the table to PiccoloCRUD, and PiccoloCRUD will create a schema with the correct visible_columns. Something like this:

@property
def pydantic_model(self) -> t.Type[pydantic.BaseModel]:
    """
    Useful for serialising inbound data from POST and PUT requests.
    """
    if (some condition to get TableConfig visible_columns) :
        visible_columns = visible_columns=tuple(
                i._meta.name for i in table.list_columns
            ),
    else:
        visible_columns=tuple(
                i._meta.name for i in table._meta.columns
            ),
    return create_pydantic_model(
        self.table, model_name=f"{self.table.__name__}In",
        visible_columns=visible_columns,
    )

[dantownsend]

@sinisaos I'll have to look at it in detail - it sounds trickier than I originally thought.

[sinisaos]

@dantownsend I know you didn't imagine it that way, but if there's no easy workaround for TableConfig, we can always put visible_colums in TableMeta (just like help_text or tags) to display in the UI

@dataclass
class TableMeta:
    ...
    tags: t.List[str] = field(default_factory=list)
    help_text: t.Optional[str] = None
    visible_columns: t.Optional[t.List[str]] = None
    ...

class Table(metaclass=TableMetaclass):
    _meta = TableMeta()

    def __init_subclass__(
        ...
        tags: t.List[str] = [],
        help_text: t.Optional[str] = None,
        visible_columns: t.Optional[t.List[str]] = None,
    ):
    ...
    cls._meta = TableMeta(
        ...
        tags=tags,
        help_text=help_text,
        visible_columns=visible_columns,
        _db=db,
    )

then in pydantic.py we add

...
visible_columns = tuple(
    [
        [i._meta.name for i in table._meta.columns]
        if table._meta.visible_columns is None
        else table._meta.visible_columns
    ][0]
)

class CustomConfig(Config):
   schema_extra = {
       "help_text": table._meta.help_text,
       "visible_columns": visible_columns,
   }
...

and then in Piccolo Admin RowListing.vue

cellNames() {
  const keys = []
  for (const key in this.rows[0]) {
      if (!key.endsWith("_readable")) {
          keys.push(key)
      }
  }
  const columnsDifference = keys.filter((column) =>
      this.schema.visible_columns.includes(column)
  )
  return columnsDifference
},

To use that we just add visible columns in Table construction and everything works great.

class Director(
    Table,
    help_text="The main director for a movie.",
    visible_columns=["id", "name"],
):

[dantownsend]

@sinisaos That's a nice idea.

My main reason I didn't want to add extra attributes to the Table class, is it increases the likelihood of a clash with a column name.

Adding them to the Table Metaclass avoids this. However, it could be problematic because lets say you do this:

class MyTable(Table, visible_columns=[MyTable.id, MyTable.name]):
    ...

Would that cause an error? Because MyTable isn't fully defined yet.

[sinisaos]

@dantownsend Thanks. This is just an idea if we fail to solve the problem with TableConfig.

My main reason I didn't want to add extra attributes to the Table class, is it increases the likelihood of a clash with a column name.

If I correctly understand, I think we are safe here because it is just a string and if we enter the column name to visible_columns incorrectly it will not be visible in the UI and that's it.

class MyTable(Table, visible_columns=[MyTable.id, MyTable.name]):
    ...

I have to try it and see if there is a error.

[sinisaos]

class MyTable(Table, visible_columns=[MyTable.id, MyTable.name]):
    ...

This doesn't work because NameError is raised. We need to stick to a list of strings. Please can you leave a comment here if you can't solve the TableConfig problem. Then I can do PR to Piccolo and Piccolo Admin to make this changes work. Cheers.

[sinisaos]

@dantownsend Just for reminder. Have you had success in implementing TableConfig? If not, with your approval I can make PR for Piccolo to add visible columns to the table.py as this could help someone who has a lot of columns and doesn’t want to display them all in Piccolo Admin.

[dantownsend]

@sinisaos I had a look. I have a potential solution - it effects the Piccolo, Piccolo API, and Piccolo Admin repos ...

I'll push them up so you can take a look.

[dantownsend]

@sinisaos These are the relevant PRs:

piccolo-orm/piccolo#313
piccolo-orm/piccolo_api#96
#101

What do you think of them as a solution?

[sinisaos]

@dantownsend It looks great, but first I want to give it a try and then report how it works.

[dantownsend]

@sinisaos Thanks, if you could give them a go that would be great.

[dantownsend]

This is mostly implemented by #102

However, TableConfig can be further improved, so will keep this issue open for now. For example - with list_filters as specified in the task description.

[sinisaos]

@dantownsend I will add possibility to change list of columns in filter. It's basically the same as visible columns with different modification on frontend. If I'm not able to do PR today, I will do it no later than tomorrow.

[dantownsend]

@sinisaos You're right - the implementation should be very similar. Thanks a lot 👍

[dantownsend]

Implemented by:

#102
#103