piccolo-orm / piccolo_admin

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

Piccolo-Admin is unable to understand target_column arg of ForeignKey columns #362

Open aabmets opened 4 months ago

aabmets commented 4 months ago

I created tables with these definitions:

class ElementsTable(Table, tablename="meta_elements", schema=AppConfig.DB_SCHEMA):
    classification_code = ForeignKey(
        references=tables.ClassificationsTable,
        target_column=tables.ClassificationsTable.code,
        on_delete=OnDelete.cascade,
        on_update=OnUpdate.cascade,
        default=None,
        required=True,
        null=False
    )
class ClassificationsTable(Table, tablename="meta_classifications", schema=AppConfig.DB_SCHEMA):
    code = Varchar(
        length=40,
        unique=True,
        required=True,
        null=False
    )
    # other columns omitted due to irrelevance

In piccolo-admin web GUI, I attempted to insert a value from the referenced target column into the classification_code column of the meta_elements table. image

The admin GUI has hardcoded expectation that ForeignKeys only reference the ID column of referred tables and it does not even allow me to manually type a value for the classification_code column.

The constraint itself is created correctly in the Postgres database: image

These inconsistencies between the components make ForeignKeys unusable together with Piccolo-Admin. Please put more effort into checking the quality and coherence of your source code.

sinisaos commented 4 months ago

@aabmets You are right, non-primary FK does not work in admin. Here's an old issue for that. There was a PR to fix that problem, but somehow it got stuck and I closed it. I don't know if it's even valid code on the backend now and I'm sure it's not valid on the frontend because a lot of changes were made moving from Vue2 to Vue3.

sinisaos commented 4 months ago

@aabmets If you have the will and time, you can try the changes required to make the non-primary key work in Piccolo Admin. Piccolo API - https://github.com/sinisaos/piccolo_api/tree/non_primary_key Piccolo Admin - https://github.com/sinisaos/piccolo_admin/tree/non_primary_key In my case, all combinations work (serial PK, multiple non-primary key and their combination). An example of usage is in Piccolo Admin example.py. For your tables example it should be:

from piccolo_admin.endpoints import TableConfig, create_admin

elements_config = TableConfig(
    table_class=ElementsTable, target_column=[ClassificationsTable.code]
)

APP = create_admin([elements_config])

I hope this is useful.

dantownsend commented 4 months ago

@sinisaos I'm glad that code still exists. It's something we need to add at some point. All of the projects I have use quite simple foreign keys (just to primary keys), so I haven't had a chance to properly test it.

aabmets commented 4 months ago

@sinisaos Thank you for a possible solution. I went with another route: defined the column as the same type as the column in the referenced table and used a raw migration to create a foreign key constraint between the two.