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

Address admin class has name in search_fields #130

Closed
pyarun opened this issue Sep 9, 2020 · 5 comments
Closed

Address admin class has name in search_fields #130

pyarun opened this issue Sep 9, 2020 · 5 comments
Assignees
Labels
Bug good first issue Normal Medium Priority Ticket
Milestone

Comments

@pyarun
Copy link
Contributor

pyarun commented Sep 9, 2020

Address Model does not have a name field. But django admin model has name in search_fields.

This causes Error in Admin when we search.

Cannot resolve keyword 'name' into field. Choices are: formatted, id, latitude, locality, locality_id, longitude, raw, route, street_number
@banagale banagale added Bug good first issue Normal Medium Priority Ticket labels Sep 9, 2020
@banagale banagale added this to the Version 0.2.6 milestone Sep 9, 2020
@banagale
Copy link
Collaborator

banagale commented Sep 9, 2020

Thanks for the report, @pyarun.

This looks like a 10 year old mistake in django-address.address.admin

Here's the line.

Here are the valid fields, what were you hoping to search on here?

@pyarun
Copy link
Contributor Author

pyarun commented Sep 13, 2020

I think raw, route, and street_number are good candidates.

If you agree, I will see if I can fix and create an MR.

@banagale banagale modified the milestones: Version 0.2.6, Version 0.2.7 Sep 13, 2020
@banagale
Copy link
Collaborator

banagale commented Sep 13, 2020

@pyarun. I agree those are good candidates. If you'd like to create a PR, for this here #130, I'd be happy to review and merge. If you have time to add tests, they're welcome but not absolutely necessary.  I'll push this to next release for now.  

*edit: I left this in 0.2.6 because I pushed it out until next weekend. If you need more time no problem.

@pyarun
Copy link
Contributor Author

pyarun commented Sep 19, 2020

I didn't see any test cases for admin, and I have never written a test for admin, so not sure how to write it.
So i just fixed this issue in admin.

banagale added a commit that referenced this issue Sep 21, 2020
Fixes #130, corrects search fields for django admin page
@banagale
Copy link
Collaborator

@pyarun, it looks like there are ways to test modeladmin but it is for more sophisticated operations. I tested your fix and it is good. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug good first issue Normal Medium Priority Ticket
Projects
None yet
Development

No branches or pull requests

2 participants