-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add ManyToManyAdminField for use with ManyToManyField #43
base: main
Are you sure you want to change the base?
Conversation
2 similar comments
@@ -46,6 +46,9 @@ def __init__(self, attr, short_description=None, admin_order_field=None, allow_t | |||
|
|||
super(SimpleAdminField, self).__init__(short_description, admin_order_field, allow_tags) | |||
|
|||
def __name__(self): | |||
return 'SimpleAdminField' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you send me the warning message from django?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'ForeignKeyAdminField' object has no attribute '__name__'
When used like this
price_model_link = ForeignKeyAdminField('price_model')
readonly_fields = (
price_model_link,
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about put this way:
def __name__(self):
return self.__class__.__name__
It will work for all child classes with right name.
@@ -85,6 +88,39 @@ def render(self, obj): | |||
return self.default | |||
|
|||
|
|||
class ManyToManyAdminField(SimpleAdminField): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will improve the coverage by the way. I put it together after using it myself, but I haven't had time to improve the testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great
if hasattr(ref, 'get_queryset'): | ||
list_str = '<ul>' | ||
objects = ref.get_queryset() | ||
for obj in objects: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do think about using string template to replace this code?
class ManyToManyAdminField(SimpleAdminField): | ||
"""Renders a ManyToManyField as links.""" | ||
|
||
def __init__(self, attr, display=None, short_description=None, admin_order_field=None, default=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think will be great to add a template path. to help on customization from user.
then you check
if template_path:
# use template
else:
# use default way
If you need some help to fix the broked tests on django 2.0, please, ask me. Thanks for your PR |
I've added a field to show the objects of a many to many field in a list. And I added a
__name__
method toSimpleAdminField
because Django was complaining that__name__
could not be found.