getting-started-with-django icon indicating copy to clipboard operation
getting-started-with-django copied to clipboard

AgentUpdateView not working

Open Bowiemb opened this issue 2 years ago • 8 comments

Everything in the application from the tutorial so far (about to get into deployment) works perfectly except for UpdateAgentView. I've copy/pasted straight from this GitHub, and it still won't update the information for the agents. The form just goes back to agent-list with no changes on the front end, and no changes in the Agent or User table.

I've seen a few others post about this same issue on the YouTube video.

With that being said, what an amazing tutorial, and I've very appreciative of your work!

My best, Michael

Bowiemb avatar Dec 11 '21 03:12 Bowiemb

I was able to fix this by adding the following to the AgentUpdateView:

def form_valid(self, form): if form.is_valid(): form.save() return super(AgentUpdateView, self).form_valid(form)

However, in the video, he says you don't need this... and it's not needed for the Lead Update. I'm not sure why it wasn't updating the agents without this.

The update form is still not pre-populated like the Lead Update Form is. I still have not fixed this part.

Edit: Actually the above didn't work.. I had to insert the below code for the Update to actually update. However, the form is still not pre-populating.

def form_valid(self, form):
        first_name = form.cleaned_data['first_name']
        last_name = form.cleaned_data['last_name']
        email = form.cleaned_data['email']
        username = form.cleaned_data['username']
        agent = Agent.objects.filter(
            pk=self.kwargs['pk'])[0]
        # print(agent.user.id)
        User.objects.filter(pk=agent.user.id).update(
            email=email,
            username=username,
            first_name=first_name,
            last_name=last_name
        )
        # print("here")
        return super(AgentUpdateView, self).form_valid(form)

Bowiemb avatar Dec 11 '21 04:12 Bowiemb

I think I figured out what the problem is, but I'm not sure how to fix it.

I think the problem is that the model form is changing User, but it's getting passed the wrong pk. It's getting passed the pk of the agent instead of the pk of the user. However, I'm not sure how to fix this.

I tried the following with no luck:

 def get_queryset(self):
        curr_user = self.request.user
        agent = Agent.objects.filter(organization=curr_user.userprofile)[0]
        print(agent.user.id)
        print(User.objects.filter(id=agent.user.id)[0].id)
        return User.objects.filter(id=agent.user.id)

I got a 404 not found.

I also tried passing in the agent.user.pk in the url instead of agent.pk. That didn't work either.

Bowiemb avatar Dec 11 '21 05:12 Bowiemb

Ok. I finally fixed it by using a function based view instead of a class based view:

This pre-populated the fields and updated the user information correctly. Maybe I'll figure out how to get the classed based view to work, later.

def AgentUpdateViewFunc(request, pk):
    agent = Agent.objects.get(pk=pk)
    user = User.objects.get(pk=agent.user.id)
    form = AgentModelForm(request.POST or None, instance=user)
    if form.is_valid():
        form.save()
        return redirect("agents:agent-list")
    context = {
        "form": form,
        "agent": agent
    }
    return render(request, "agents/agent_update.html", context)

Bowiemb avatar Dec 13 '21 15:12 Bowiemb

AgentUpdateView is actually not working neither here https://crm.justdjango.com/agents/, nor if you git clone the latest version from the official repo.

These are the files that cause this issue:

  • https://github.com/justdjango/getting-started-with-django/blob/main/agents/views.py
  • https://github.com/justdjango/getting-started-with-django/blob/main/agents/forms.py

paulrogov avatar Jan 26 '22 18:01 paulrogov

@Bowiemb I also tried to fix this and came up to a similar function-based view first.
But your code has a dangerous bug (I also had it the first time I implemented the solution).

If this line agent = Agent.objects.get(pk=pk) you just check agent's pk and don't check whether it belongs to the same organisation as request.user.
Therefore any user signed up as ogranizer will be able to change any agent by visiting url http://localhost:8000/agents/5/update/, where 5 can be id of any agent. You can check it yourself.

paulrogov avatar Jan 26 '22 18:01 paulrogov

I ended up with two equal solutions (honestly, I prefer function-based view, because it's easier to debug and more explicit):

1. function-based view

agents/views.py

def agent_update_view(request, pk):
	organisation= request.user.userprofile
	agent = Agent.objects.get(id=pk, organisation=organisation)
	form = AgentModelForm(instance=agent.user)

	if request.method == "POST":
		form = AgentModelForm(request.POST, instance=agent.user)
		if form.is_valid():
			form.save()
			return redirect("/agents")

	context = {
		"form": form,
		"agent": agent
	}
	return render(request, "agents/agent_update.html", context)

agents/urls.py

from .views import agent_update_view

urlpatterns += [
	path('<int:pk>/update/', agent_update_view, name="agent-update"),
]

and don't forget to comment out path('<int:pk>/update/', AgentUpdateView.as_view(), name="agent-update"),

paulrogov avatar Jan 26 '22 18:01 paulrogov

2. class-based view agents/forms.py

from django import forms
from django.contrib.auth import get_user_model

from leads.models import Agent


User = get_user_model()

class AgentModelForm(forms.ModelForm):
    class Meta:
        model = User
        fields = (
            'email',
            'username',
            'first_name',
            'last_name',
        )

    def __init__(self, *args, **kwargs):
        kwargs['instance'] = kwargs['instance'].user
        super().__init__(*args, **kwargs)


class AgentCreationModelForm(forms.ModelForm):
    class Meta:
        model = User
         fields = (
            'email',
            'username',
            'first_name',
            'last_name',
        )

agents/views.py

  • AgentUpdateView stays the same as it is in the offical repo.
  • Change form_class of AgentCreateView:
class AgentCreateView(OrganizerLoginRequiredMixin, generic.CreateView):
    template_name = "agents/agent_create.html"
    form_class = AgentCreationModelForm

.......

P.S. Maybe my code also contains some bug, but I tested it manually and it seems to work fine. @mattfreire thank you for such an amazing tutorial.
Hope you will be able to find a minute to fix this issue in the official repo.

paulrogov avatar Jan 26 '22 18:01 paulrogov

Was reading Django docs on Forms and came up with a shorter and more elegant solution.

It seems that you just need to override get_form_kwargs method inside AgentUpdateView to make it work as a class-based view.

agents/views.py

class AgentUpdateView(OrganizerLoginRequiredMixin, generic.UpdateView):
	template_name = "agents/agent_update.html"
	form_class = AgentModelForm

	def get_queryset(self):
		organisation = self.request.user.userprofile
		return Agent.objects.filter(organisation=organisation)

	def get_form_kwargs(self, **kwargs):
		kwargs = super().get_form_kwargs(**kwargs)
		kwargs['instance'] = kwargs['instance'].user
		return kwargs

	def get_success_url(self) -> str:
		return reverse("agents:agent-list")

paulrogov avatar Jan 26 '22 23:01 paulrogov