Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
aabmets opened this issue Feb 20, 2024 · 4 comments

Comments

@aabmets
Copy link

aabmets commented Feb 20, 2024

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
Copy link
Member

@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
Copy link
Member

@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
Copy link
Member

@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
Copy link
Author

aabmets commented Feb 22, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants