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

How to improve str() representation of relationships? Eg: when deleting from admin #96

Closed
nemesifier opened this issue Feb 8, 2017 · 12 comments

Comments

@nemesifier
Copy link
Contributor

I have two models: Config and Template (django-netjsonconfig), Config has a sortedm2m relation to Template.

When deleting an object which has m2m relationships from admin interface, django says:

Are you sure?

    Are you sure you want to delete the selected template? All of the following objects and their related items will be deleted:

Summary

    - Config-template relationships: 6
    - Templates: 1

Objects

    - Template: [Generic-OpenWRT] Wireless settings
        - Config-template relationship: Config_templates object
        - Config-template relationship: Config_templates object
        - Config-template relationship: Config_templates object
        - Config-template relationship: Config_templates object
        - Config-template relationship: Config_templates object
        - Config-template relationship: Config_templates object

The repetition of Config-template relationship: Config_templates object conveys no information. I would like to display something more useful.

For example, each config has a name, let's say I'm deleting a template which was installed on two config objects named Auditorium and Gym

I've been able to accomplish this only through monkey patching:

class Config(models.Model):
    # ...
    pass

def sortedm2m__str__(self):
    """
    Improves string representation of m2m relationship objects
    """
    return self.config.name

Config.templates.through.__str__ = sortedm2m__str__

But I'm wondering if there's a better way. Any suggestion?

nemesifier added a commit to openwisp/django-netjsonconfig that referenced this issue Feb 8, 2017
@gregmuellegger
Copy link
Collaborator

No I have no good solution to this with the current implementation of sortedm2m. But we could ofcourse extend the SortedManyToManyField to support this usecase. I see the need you have and saw those not-so-nice string representations also a couple of times already.

  1. Idea: Add a to_string argument to the field so you can provide a function that gets a, b and sort_value as arguments, where as a is an instance of the from-model, Config in your case, and b is an instance of the to-model, so Template in your case.

  2. Idea: Provide a base_class argument to the field, so you can modify the base class that the through-model inherits from. This would allow you to customize the __str__ method by defining it in an abstract base class.

What do you think? Would you be interested in working on a solution to your problem. I'm happy for any pull request.

@nemesifier
Copy link
Contributor Author

Both ideas would work. I'm inclined to prefer 2 because I think that would allow to customize more logic of the automatically generated "through" model. Is my supposition right? Yes I can work on this, I don't like to leave that monkey-patching in django-netjsonconfig.

@nemesifier
Copy link
Contributor Author

@gregmuellegger have you had any time to think about it? Would you prefer idea1 or idea2?

@nemesifier
Copy link
Contributor Author

@gregmuellegger I'm still keen to work on this.

@gregmuellegger
Copy link
Collaborator

gregmuellegger commented Feb 28, 2017

Yes, I would go with your recommendation. The base_class seems super flexible and good for a lot of other customizations. So option 2 has my vote as well.

Thanks for your on going interest :)

@rohithasrk
Copy link
Contributor

Working on this.

@rohithasrk
Copy link
Contributor

By reading the above thread and also the code, I've understood the following.

  • We need to customise the __str__ method of the through model to print something that would be passed as an argument.

I've got some queries regarding this,

  • There needs to be a base_class defined in SortedManyToManyField class, isn't it?
  • What exactly needs to be inside the __str__ method of through model? How do we achieve that level of abstraction?
  • "This would allow you to customize the str method by defining it in an abstract base class." - Can you please elaborate?

@nemesisdesign @gregmuellegger Please help.

@nemesifier
Copy link
Contributor Author

@rohithasrk:

yes we need to add a base_class parameter to SortedManyToManyField ;

by default the __str__ method does not have to do anything in particular, it will be the responsibility of the users of django-sortedm2m to supply a specialized __str__ method; it may as well be absent;

we need to add a way to supply a base class for the through model of SortedManyToManyField in order to customise it, at the moment it is not possible to do this without resorting to monkey patching.

@rohithasrk
Copy link
Contributor

So, I understand that I need to do two things,

  • Add a base_class parameter to SortedManyToManyField which would be by defining a function to add the field. (Different from normally adding the field to a model) -- Still confused in this though.
  • Add a function named modify_through (for ex.) which modifies the through model of SortedManyToManyField. And modify_through shall look somewhat like this,
def modify_through(self, base_class, some_function):
    self.through.__str__ = base_class.some_function

Is it right? @nemesisdesign

@nemesifier
Copy link
Contributor Author

not right, you have to find a way to pass a base class that django-sortedm2m can use as a base for its through model. Look for create_intermediate_model and similar function in the codebase. You will have to read the internal code of this project.

@rohithasrk
Copy link
Contributor

I see. I'll go through once.

@rohithasrk
Copy link
Contributor

@nemesisdesign @gregmuellegger : Please review PR #101

rohithasrk added a commit to rohithasrk/django-sortedm2m that referenced this issue Mar 24, 2017
rohithasrk added a commit to rohithasrk/django-sortedm2m that referenced this issue Mar 24, 2017
rohithasrk added a commit to rohithasrk/django-sortedm2m that referenced this issue Mar 24, 2017
rohithasrk added a commit to rohithasrk/django-sortedm2m that referenced this issue Mar 25, 2017
rohithasrk added a commit to rohithasrk/django-sortedm2m that referenced this issue Mar 28, 2017
rohithasrk added a commit to rohithasrk/django-sortedm2m that referenced this issue Mar 28, 2017
rohithasrk added a commit to rohithasrk/django-sortedm2m that referenced this issue Mar 28, 2017
rohithasrk added a commit to rohithasrk/django-sortedm2m that referenced this issue Jun 13, 2017
gregmuellegger added a commit that referenced this issue Aug 1, 2017
Fixes #96: Improve string representation of sortedm2m relationships
rohithasrk added a commit to rohithasrk/django-sortedm2m that referenced this issue Aug 2, 2017
* Support Django up to 1.11, fix DeprecationWarnings

Also set warnings to raise as errors in unit tests to prevent
regressions that re-introduce the warnings.

* Renamed duplicated TestStringReference classes jazzband#105

Fixes jazzband#105

* Add changelog for 1.4.0 release

* Bump version to 1.4.0

* Prepare new dev version

* Adding missing link to jazzband#104 in changelog

* Fixes jazzband#96: Improve string representation of sortedm2m relationships
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

No branches or pull requests

3 participants