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

Fixing #26524 - Change foreign key id list_display reference to display only the id #6497

Closed
wants to merge 1 commit into from

Conversation

krisys
Copy link
Contributor

@krisys krisys commented Apr 23, 2016

No description provided.

@timgraham
Copy link
Member

I wonder if the code could be abstracted better, for example, by adding a method to Field. In particular, more hardcoding of '_id' doesn't seem so nice.

@phildini
Copy link

phildini commented Jun 2, 2016

@timgraham would you want the abstraction to be completed before this PR is ready to land?

@timgraham
Copy link
Member

I'm not too enthusiastic about the PR in its current form. I'm not sure if there's a more elegant way, but it looks a bit hacky to me.

@krisys
Copy link
Contributor Author

krisys commented Jun 3, 2016

@timgraham , Apologies for the delay. I will spend some time this weekend and see if there is a better way to do this.

@krisys
Copy link
Contributor Author

krisys commented Jun 3, 2016

@timgraham , Can you please take a look at this and let me know if this more acceptable? Any suggestions on making this better is welcome.

@@ -9,3 +9,8 @@ class DisallowedModelAdminLookup(SuspiciousOperation):
class DisallowedModelAdminToField(SuspiciousOperation):
"""Invalid to_field was passed to admin view via URL query string"""
pass


class FieldIsAForeignKeyColumnName(Exception):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is okay. Since the exception is just used as a sort of helper in one file, I'd put the exception there.

@timgraham
Copy link
Member

merged in f668139, thanks!

@timgraham timgraham closed this Jun 8, 2016
@timgraham
Copy link
Member

I missed it, but for future reference use "Fixed" rather than "Fixing" in the commit message.

@krisys
Copy link
Contributor Author

krisys commented Jun 9, 2016

Will keep that in mind. Thanks for merging.

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