CODEWITHSUNDEEP
pythondjangopython-2.7
miss_tais
miss_tais

Reputation: 67

Save() in ManyToMany relation

I have two models with ManyToMany relation. Here is the code:

class Player(models.Model):

    first_name = models.CharField(max_length = 30, verbose_name = u"First name")
    last_name = models.CharField(max_length = 50, verbose_name = u"Last name")

    def __unicode__(self):
        return "%s %s" % (self.last_name, self.first_name)

class Tournament(models.Model):
    title = models.CharField(max_length = 100, verbose_name = u"Tournament's title")
    players = models.ManyToManyField(Player,verbose_name = u"Tournament's players")

    def __unicode__(self):
        return self.title

    def save(self, **kwargs):
        Tournament(title = self.title)
        all_players = Player.objects.all()
        for member in all_players:
            member_of_tournament = member.tournament_set.filter(title = self.title)
            for j in member_of_tournament:
                print member.tournament_set.filter(title = self.title)
                self.players.add(member)

        super(Tournament, self).save(**kwargs)

When I save Tournament for the first time it saves only the title. But when I save next time it saves players too and relates them to the Tournament. How can I save them at the same time with the tournament?

Upvotes: 1

Views: 289

Answers (1)

James R
James R

Reputation: 4656

I think you have a number issues here:

class Tournament(models.Model):
    title = models.CharField(max_length = 100, verbose_name = u"Tournament's title")
    players = models.ManyToManyField(Player,verbose_name = u"Tournament's players")

    def __unicode__(self):
        return self.title

    def save(self, **kwargs):
        Tournament(title = self.title)
        all_players = Player.objects.all()
        for member in all_players:
            member_of_tournament = member.tournament_set.filter(title = self.title)
            for j in member_of_tournament:
                print member.tournament_set.filter(title = self.title)
                self.players.add(member)

        super(Tournament, self).save(**kwargs)

In general, you don't want to have your m2m relationships in the model save method (and in this case, the logic for it isn't very good in any case)

You have a few issues in the save method itself, so let me address those:

def save(self, **kwargs):
    Tournament(title = self.title)

The last line above is doing nothing. You instantiate an instance of Tournament, but don't save it to a variable. You actually already have an instance of tournament anyway (called self, if this case).

    all_players = Player.objects.all()
    for member in all_players:
        member_of_tournament = member.tournament_set.filter(title = self.title)

Here, you loop through all the players in the DB, whether they match your query or not.

This is really inefficient.

In the next line, you have member_of_tournament = member.tournament_set.filter(title = self.title). This is plural, so you should call this members_of_tournament because this is an array / list / queryset.

I'm honestly not sure what the strategy is of the rest, but suffice to say, you shouldn't do it like that, probably.

You should just get ride of the entire custom save method and in your view that is driving this, you should do something like this:

tournament = Tournament(title=title)
tournament.save()
players_i_care_about = [players, go, here]

tournament.players = players_i_care_about #(removes all players and saves new players)

or

for player in players_i_care_about:
    tournament.players.add(player) #(only adds these players, doesn't remove any players)

The reason for this, your view knows about what players belong to what tournament, but your model should be agnostic to that logic.

Upvotes: 4

Related Questions