ForestAdmin / django-forestadmin

🐍 Django agent for Forest Admin to integrate directly to your existing Django backend application.
https://www.forestadmin.com
GNU General Public License v3.0
122 stars 18 forks source link

fix(csv): prevent error with model without id attr #114

Closed snwessel closed 1 year ago

snwessel commented 2 years ago

Definition of Done

The model's pk should be able to have a name other than 'id'.

There were some changes made related to this 8 days ago (https://github.com/ForestAdmin/django-forestadmin/pull/112) but it did not fix the issue for us (community forum post here).

I would like to propose a couple more small changes which get it working for our use case.

General

Security

vamonte commented 2 years ago

@snwessel Could you send me a collection to raise the issue. I don't reproduce the issue.

vamonte commented 2 years ago

@snwessel Could you fix the issues issues raise by flake8.

snwessel commented 2 years ago

@vamonte Thanks again! I just merged all of my commits into a single commit with a name that hopefully matches your requirements. Let me know if there's anything else you'd like me to update!

snwessel commented 1 year ago

@vamonte would you mind taking another look at this PR when you have the chance? It looks like I have you're approval but can't rerun the automated tests or merge the branch on my own. Thank you!

vamonte commented 1 year ago

@vamonte would you mind taking another look at this PR when you have the chance? It looks like I have you're approval but can't rerun the automated tests or merge the branch on my own. Thank you!

@snwessel Hello, thank's for your work, Could you fix the linting issue ?

snwessel commented 1 year ago

Thank you @vamonte! I think it should be good to go now.

matthv commented 1 year ago

Hello @snwessel. Thanks a lot for your contribution 💪 , I opened another PR that fixes the commit lint. I add you as a co-author on it.

In the future if you offer another PR, could you use the conventional commit message 🙏