django-forestadmin icon indicating copy to clipboard operation
django-forestadmin copied to clipboard

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

Open snwessel opened this issue 2 years ago • 3 comments

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

  • [x] Write an explicit title for the Pull Request, following Conventional Commits specification
  • [x] Test manually the implemented changes
  • [x] Validate the code quality (indentation, syntax, style, simplicity, readability)

Security

  • [x] Consider the security impact of the changes made

snwessel avatar Aug 10 '22 14:08 snwessel

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

vamonte avatar Aug 16 '22 07:08 vamonte

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

vamonte avatar Aug 23 '22 12:08 vamonte

@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 avatar Aug 24 '22 14:08 snwessel

@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 avatar Nov 16 '22 20:11 snwessel

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

vamonte avatar Nov 23 '22 09:11 vamonte

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

snwessel avatar Nov 29 '22 21:11 snwessel

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 🙏

matthv avatar Jan 05 '23 11:01 matthv