Skip to content

Add ManyToManyAdminField for use with ManyToManyField#43

Open
Furkanzmc wants to merge 8 commits into
ebertti:mainfrom
Furkanzmc:feature_branch
Open

Add ManyToManyAdminField for use with ManyToManyField#43
Furkanzmc wants to merge 8 commits into
ebertti:mainfrom
Furkanzmc:feature_branch

Conversation

@Furkanzmc
Copy link
Copy Markdown

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
Copy Markdown

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
Copy Markdown

Coverage Status

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

@coveralls
Copy link
Copy Markdown

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.

Comment thread easy/admin/field.py
super(SimpleAdminField, self).__init__(short_description, admin_order_field, allow_tags)

def __name__(self):
return 'SimpleAdminField'
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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.

Comment thread easy/admin/field.py
return self.default


class ManyToManyAdminField(SimpleAdminField):
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍

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

Choose a reason for hiding this comment

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

Great

Comment thread easy/admin/field.py
if hasattr(ref, 'get_queryset'):
list_str = '<ul>'
objects = ref.get_queryset()
for obj in objects:
Copy link
Copy Markdown
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?

Comment thread easy/admin/field.py
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
Copy Markdown
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
Copy Markdown
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