Django submitting two forms cannot assign instance error-CodePudding

I have the following models/forms/view in which I have managed to submit to two different models as follows:

Models

class Account(models.Model):
    username = models.ForeignKey(User, on_delete=models.SET_NULL, null=True)
    name = models.CharField(max_length=150)
    actflag = models.CharField(max_length=1, blank=True)
    acttime = models.DateTimeField(blank=True, null=True)
    comments = models.TextField(_('comments'), max_length=500, blank=True)

    def __str__(self):
        return self.name

class ISIN(models.Model):
    code = models.CharField(max_length=12)
    account_name = models.ForeignKey(Account, on_delete=models.SET_NULL, null=True)
    actflag = models.CharField(max_length=1, blank=True)
    acttime = models.DateTimeField(blank=True, null=True)

    def __str__(self):
        return self.code

Forms

from apps.portfolio.models import Account, ISIN

class PortfolioForm(forms.ModelForm):

    class Meta:
        model = Account
        fields = ['name', 'comments']

class IdentifierForm(forms.ModelForm):

    class Meta:
        model = ISIN
        fields = ['code']

View

def portfolios(request):

    if request.user.is_authenticated:

        if request.POST:
            fm = PortfolioForm(request.POST)
            fm2 = IdentifierForm(request.POST)

            if fm.is_valid():
                messages.success(request, 'Portfolio has been created.')

                account = fm.save(commit=False)
                account.username = request.user
                account.acttime = timezone.now()
                account.actflag = 'I'
                account.save()

                isin = fm2.save(commit=False)
                #isin.account_name = account.name
                isin.acttime = timezone.now()
                isin.actflag = 'I'
                isin.save()

                return redirect('portfolios')
        else:
            fm = PortfolioForm()
            fm2 = IdentifierForm()

        context = {"name": request.user, "form": fm, "form2": fm2}

        return render(request, 'portfolios.html', context)

    else:
        return redirect('login')

However, you will notice the commented line in my view: isin.account_name = account.name, when I uncomment this line and try to submit the forms again I get the following error: Cannot assign "'test'": "ISIN.account_name" must be a "Account" instance.

I believe it's to do with ForeignKey but still unsure how to store the newly created account name the user submitted within the isin model.

Help is much appreciated.

CodePudding user response：

Although my answer solves the problem you originally had, there are a couple additional points that I wanted to make.

Improve naming and fix the original error

Your field is called account_name, and it implies that a string will be stored there. If it was actually a string, you would be able to do what you tried:

isin.account_name = account.name

In reality, you have a ForeignKey to the Account model, so you have to actually save a reference to the account object:

isin.account_name = account

It's a really good idea to have a foreign key instead of just a string because it avoids denormalization.

The problem here is the name of the field, account_name. If you later want to access the account name, you would have to write something like isis.account_name.name. Sounds wrong, doesn't it?

You could solve this by renaming your field like so:

class ISIN(models.Model):
    code = models.CharField(max_length=12)
    account = models.ForeignKey(Account, on_delete=models.SET_NULL, null=True)
    actflag = models.CharField(max_length=1, blank=True)
    acttime = models.DateTimeField(blank=True, null=True)

    def __str__(self):
        return self.code

Then, in your view, you would just isin.account = account, and later, if you wanted to access the name, you would use isin.account.name.

Another minor thing is that in some places an account is called Account and in other places it's Portfolio. This creates an illusion that they're unrelated entities and makes your code harder to read and maintain.

You probably should decide which one is the better term, and make it consistent everywhere.

Use builtin timestamp mechanism

Looks like you're using the acttime field to manually store creation time of accounts and ISINs.

You could use Django's auto_now_add property to do that automatically, like so:

class Account(models.Model):
    acttime = models.DateTimeField(auto_now_add=True)

If you also wanted to store the last time an Account was updated, you could use auto_now (also renamed fields here for clarity):

class Account(models.Model):
    created_at = models.DateTimeField(auto_now_add=True)
    updated_at = models.DateTimeField(auto_now=True)

And to stay DRY, you could make a mixin for that and use it in both Account and ISIN:

class TimeStampMixin(models.Model):
    created_at = models.DateTimeField(auto_now_add=True)
    updated_at = models.DateTimeField(auto_now=True)

    class Meta:
        abstract = True

class Account(TimeStampMixin, models.Model):
    username = models.ForeignKey(User, on_delete=models.SET_NULL, null=True)
    name = models.CharField(max_length=150)
    actflag = models.CharField(max_length=1, blank=True)
    comments = models.TextField(_('comments'), max_length=500, blank=True)

    def __str__(self):
        return self.name

class ISIN(TimeStampMixin, models.Model):
    code = models.CharField(max_length=12)
    account = models.ForeignKey(Account, on_delete=models.SET_NULL, null=True)
    actflag = models.CharField(max_length=1, blank=True)

    def __str__(self):
        return self.code

This way, the creation time and the latest update time are automatically stored in your models (the ones that inherit from TimeStampMixin).

Validate both forms

Looks like you're only checking one of the forms for validity, and not the other: if fm.is_valid():

You should probably check both, in case ISIN.code is invalid:

if fm.is_valid() and fm2.is_valid():

CodePudding user response：

What it means is that you have to make an instance of the account model by getting the name in order to save the form like so:

def portfolios(request):
    if request.user.is_authenticated:
        if request.POST:
            fm = PortfolioForm(request.POST)
            fm2 = IdentifierForm(request.POST)

            if fm.is_valid():
                messages.success(request, 'Portfolio has been created.')

                account = fm.save(commit=False)
                account.username = request.user
                account.acttime = timezone.now()
                account.actflag = 'I'
                account.save()
                # Here is where we get the instance of account
                account = Account.objects.get(name=account.name)
                isin = fm2.save(commit=False)
                isin.account_name = account
                isin.acttime = timezone.now()
                isin.actflag = 'I'
                isin.save()

                return redirect('portfolios')
        else:
            fm = PortfolioForm()
            fm2 = IdentifierForm()

        context = {"name": request.user, "form": fm, "form2": fm2}

        return render(request, 'portfolios.html', context)

    else:
        return redirect('login')

CodePudding user response：

The field account_name is a ForeignKey to Account, but you are assigning an string. You should to assign an Account.

Change:

isin.account_name = account.name

To:

isin.account_name = account