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

Add ManyToManyAdminField for use with ManyToManyField #43

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Furkanzmc
Copy link

I've added a field to show the objects of a many to many field in a list. And I added a __name__ method to SimpleAdminField because Django was complaining that __name__ could not be found.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 92.433% when pulling 3185241 on Furkanzmc:feature_branch into 0548ffd on ebertti:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 92.433% when pulling 3185241 on Furkanzmc:feature_branch into 0548ffd on ebertti:master.

@coveralls
Copy link

coveralls commented Feb 22, 2018

Coverage Status

Coverage decreased (-0.3%) to 92.433% when pulling 3185241 on Furkanzmc:feature_branch into 0548ffd on ebertti:master.

@@ -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'
Copy link
Owner

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?

Copy link
Author

@Furkanzmc Furkanzmc Feb 22, 2018

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,
)

Copy link
Owner

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):
Copy link
Owner

Choose a reason for hiding this comment

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

👍

Copy link
Author

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.

Copy link
Owner

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:
Copy link
Owner

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):
Copy link
Owner

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

@ebertti
Copy link
Owner

ebertti commented Feb 24, 2018

If you need some help to fix the broked tests on django 2.0, please, ask me.

Thanks for your PR

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

Successfully merging this pull request may close these issues.

3 participants