save() creating instead of updating

I have a view that I’m using to create and (hopefully) update some rows. The problem is that when I use it to edit it adds a new record instead. I’d assumed since the form instance is obviously known (data is populated when editing) that save() would do the right thing. But I must not be doing the right thing. Here is the function:
views.py

def strike_detail(request, pk):
  strike = get_object_or_404(Strike, pk=pk)
  form = StrikeModelForm(instance=strike)
  context = {'strike': strike, 'form': form}
  if request.method == 'GET':
    return render(request, 'Striker/strike.html', context)
  if request.method == 'PUT':
    data = QueryDict(request.body).dict()
    form = StrikeModelForm(data, instance=strike)
    context = {'strike':strike, 'form':form}
    if form.is_valid():
      form.save()
      return render(request, 'Striker/partials/strike-details.html', context)        
    context = {'form':form}
    return render(request, 'Striker/partials/edit-strike-form.html', context)

and form

class StrikeModelForm(forms.ModelForm):
  class Meta:
    model = Strike
    fields = (
      'player',
      'strike_date',
      'activity',
      'ishard',
      'comments'
    )
    widgets = {
      'strike_date': widgets.DateInput(attrs={'type':'date'}),
      'comments': Textarea(attrs={'cols': 40, 'rows': 5}),
    }

Happy to post anything else that could be useful.

Are you using the implicitly-created id field for the primary key, or are you using a different field for the pk?

I am only using the implicitly-created id. Since I’m still a noob I should say that I believe I am, in any case I’m not explicitly using any other key. I assumed since I see all the fields in the edit view that the key would be already determined. If it helps, here are the relevant models:

class Player(models.Model):
  name = models.CharField(max_length=30)
  playerId = models.CharField(max_length=15)
  gp = models.IntegerField()
...
  updated = models.CharField(max_length=30,blank=True)
  guild = models.ForeignKey(Guild, default=1, on_delete=models.CASCADE)
  
  def __str__(self):
    return f'{self.name}, GP: {self.gp}'
  
class Strike(models.Model):
  STRIKE_ACTIVITY = (
    ('TW', 'TW'),
    ('TB', 'TB'),
    ('Tickets', 'Tickets'),
    ('Other', 'Other'),
  )
  player = models.ForeignKey(Player, on_delete=models.CASCADE)
  strike_date = models.DateField()
  ishard = models.BooleanField(default=True)
  comments = models.TextField(max_length=200, blank=True)
  activity = models.CharField(max_length=10, choices=STRIKE_ACTIVITY)
  
  
  
  class Meta:
    ordering = ["-strike_date"]
  
  def __str__(self):
    return f'{self.pk}, {self.player}, {self.strike_date}, {self.ishard}, {self.activity}, {self.comments}'

Thanks for the clarification. I didn’t think you had defined an alternate primary key, but I did think it was important to make sure.

You might want to verify exactly what data contains after this line.

You may also want to verify what the form looks like after it has been bound to the data.

(Using a print statement for those two items would be sufficient for this.)

What you’re hoping not to find in either case is an id field. (What I’m guessing you’re going to find is an id field with a value None.)

Well this is odd. Nothing is printed out if I add print statements (immediately after the variable assignments) yet the form is evidently being saved because I see the new record displayed. What devilry is this? :wink: No, really, how is this possible?

Please show your modified view.

Also, how are you running your application? Are you running it using runserver from a command line?

I really appreciate your helping me with this. I am doing this with runserver from cli (is there another way?) I am seeing all the get and post requests

def strike_detail(request, pk):
  strike = get_object_or_404(Strike, pk=pk)
  form = StrikeModelForm(instance=strike)
  context = {'strike': strike, 'form': form}
  if request.method == 'GET':
    return render(request, 'Striker/strike.html', context)
  if request.method == 'PUT':
    data = QueryDict(request.body).dict()
    form = StrikeModelForm(data, instance=strike)
    print("hello")
    print(data)
    print(form)
    context = {'strike':strike, 'form':form}
    if form.is_valid():
      form.save()
      return render(request, 'Striker/partials/strike-details.html', context)        
    context = {'form':form}
    return render(request, 'Striker/partials/edit-strike-form.html', context)

Yes there are multiple ways, but at this stage for this issue, this is the best way of doing it.

But you’re not seeing the “hello” and the two subsequent lines?

Also, is this the complete view?

Finally, can you copy the section of the console log around where the PUT is being submitted?

Nope. I added it just to be sure I wasn’t seeing the output. Checked in the browser with devtools and nothing is logged there either. ???

[12/Nov/2022 10:29:25] "GET /striker/strikes/60/ HTTP/1.1" 200 9123
[12/Nov/2022 10:29:30] "POST /striker/strikes/ HTTP/1.1" 200 130
[12/Nov/2022 10:29:34] "GET /striker/strikes/ HTTP/1.1" 200 33838

Yes it is the complete view

This is showing a POST and not a PUT. However, your view is looking for a PUT.

What does your urls.py file look like?

I changed it to a PUT with no difference (except that edit does nothing instead of creating a new entry, but I think this is the difference between using PUT and POST no?)
changed entries:

[12/Nov/2022 10:36:51] "GET /striker/strikes/63/ HTTP/1.1" 200 9117
[12/Nov/2022 10:37:00] "PUT /striker/strikes/ HTTP/1.1" 200 35129
[12/Nov/2022 10:37:23] "GET /striker/strikes/ HTTP/1.1" 200 35129

still no output from print.
I should say that this is an htmx request:

  <form 
                hx-put="{% url 'strike.list' %}" 
                hx-trigger="submit" 
                hx-target="#information"
                class="space-y-6" action="#">

The only related url is

  path('striker/strikes/<int:pk>/', views.strike_detail, name='strike.detail' ),

for completeness here is the html:

{% extends './base.html' %}
{% load static %}
{% load widget_tweaks %}
{% block content %}
<div class="mb-4">
<a class="text-white bg-gradient-to-br from-purple-600 to-blue-500 hover:bg-gradient-to-bl focus:ring-4 focus:outline-none focus:ring-blue-300 dark:focus:ring-blue-800 font-medium rounded-lg text-sm px-2 py-1.5 text-center mr-2 mb-2" href="{% url 'strike.list' %}">Back to strikes</a>
</div>
<div class="body-font overflow-hidden">
  <div class="px-5 py-24 mx-auto">


                <div id="information" class="mt-8 text-2xl"></div>

        <div class="relative p-4 w-full max-w-md h-full md:h-auto">
          <!-- Modal content -->
          <div class="relative bg-white rounded-lg shadow dark:bg-gray-700">
            <div class="py-6 px-6 lg:px-8">
              <h3 class="mb-4 text-xl font-medium text-gray-900 dark:text-white">Edit Strike</h3>
              <form 
                hx-put="{% url 'strike.list' %}" 
                hx-trigger="submit" 
                hx-target="#information"
                class="space-y-6" action="#">
                <div>
                  {% render_field form.player|add_label_class:"p-2" %}
                  {% render_field form.player class="bg-gray-50 border border-gray-300 text-gray-900 text-sm rounded-lg focus:ring-blue-500 focus:border-blue-500 block w-full p-2.5 dark:bg-gray-600 dark:border-gray-500 dark:placeholder-gray-400 dark:text-white" %}
                </div>
                <div>
                  {% render_field form.strike_date|add_label_class:"p-2" %}
                  {% render_field form.strike_date class="bg-gray-50 border border-gray-300 text-gray-900 text-sm rounded-lg focus:ring-blue-500 focus:border-blue-500 block w-full p-2.5 dark:bg-gray-600 dark:border-gray-500 dark:placeholder-gray-400 dark:text-white" %}
                </div>
                <div>
                  {% render_field form.activity|add_label_class:"p-2" %}
                  {% render_field form.activity class="bg-gray-50 border border-gray-300 text-gray-900 text-sm rounded-lg focus:ring-blue-500 focus:border-blue-500 block w-full p-2.5 dark:bg-gray-600 dark:border-gray-500 dark:placeholder-gray-400 dark:text-white" %}
                </div>
                <div>
                  {% render_field form.ishard|add_label_class:"p-2" %}
                  {% render_field form.ishard class="bg-gray-50 border border-gray-300 text-gray-900 text-sm rounded-lg focus:ring-blue-500 focus:border-blue-500 block w-full p-2.5 dark:bg-gray-600 dark:border-gray-500 dark:placeholder-gray-400 dark:text-white" %}
                </div>
                <div>
                  {% render_field form.comments|add_label_class:"p-2" %}
                  {% render_field form.comments class="bg-gray-50 border border-gray-300 text-gray-900 text-sm rounded-lg focus:ring-blue-500 focus:border-blue-500 block w-full p-2.5 dark:bg-gray-600 dark:border-gray-500 dark:placeholder-gray-400 dark:text-white" %}
                </div>
                <button type="submit"
                  class="w-full text-white bg-blue-700 hover:bg-blue-800 focus:ring-4 focus:outline-none focus:ring-blue-300 font-medium rounded-lg text-sm px-5 py-2.5 text-center dark:bg-blue-600 dark:hover:bg-blue-700 dark:focus:ring-blue-800">Save</button>
              </form>
            </div>
          </div>
        </div>


    <div id="strike-list">
      {% include 'Striker/partials/strike-list.html' %}
    </div>
  </div>
</div>

{% endblock content %}

OK, I think I figured it out. I’m pointing to the wrong url.

So this is the relevant view:

def strike_list(request):
  strikes = Strike.objects.all()
  if request.method == 'POST':
    form = StrikeModelForm(request.POST)
    print(form)
    if form.is_valid():
      form.save()
      strikes = Strike.objects.all()
      context = {'form': form, 'strikes': strikes}
      return render(request, 'Striker/partials/success.html')
    else:
      return render(request, 'Striker/partials/failure.html')
  form = StrikeModelForm()
  context = {'form': form, 'strikes': strikes}
  return render(request, "Striker/strikes.html", context)

And this is the form object:

<tr><th><label for="id_player">Player:</label></th><td><select name="player" required id="id_player">
  <option value="">---------</option>

  <option value="4">zrqml, GP: 2815555</option>

  <option value="5" selected>USS ALABAMA, GP: 2141053</option>

  <option value="6">Stay Classy, GP: 1771606</option>

  <option value="7">Kusch, GP: 1667678</option>

  <option value="8">Darth2Darth, GP: 1058416</option>

  <option value="9">chris, GP: 1562944</option>

  <option value="10">an0nym0usj0hn, GP: 995921</option>

  <option value="11">Orion AltyMcAlt, GP: 2461124</option>

  <option value="12">BobsYourUncle, GP: 1924740</option>

  <option value="13">Rodmanthegoat, GP: 2492075</option>

  <option value="14">The last starfighter, GP: 1733162</option>

  <option value="15">whitecream, GP: 1356395</option>

  <option value="16">Atreides, GP: 2346727</option>

  <option value="17">drewfisto1, GP: 1457420</option>

  <option value="18">Ace starship, GP: 2475471</option>

  <option value="19">Presbo, GP: 2484242</option>

  <option value="20">Gutzag, GP: 1359311</option>

  <option value="21">bebay, GP: 2288396</option>

  <option value="22">Bently, GP: 1719924</option>

  <option value="23">Chantza, GP: 2161487</option>

  <option value="24">Beyuhz, GP: 2398208</option>

  <option value="25">MightyMoe, GP: 2014423</option>

  <option value="26">RustedRooster, GP: 2274490</option>

  <option value="27">Slimalin, GP: 1220487</option>

  <option value="28">NoJedi, GP: 1304308</option>

  <option value="29">URMADKID99, GP: 1946823</option>

  <option value="30">Tacobell96, GP: 1725387</option>

  <option value="31">Chet Rippo, GP: 1294782</option>

  <option value="32">Garnon Hald, GP: 6091381</option>

  <option value="33">Ali Vaqar, GP: 5939626</option>

  <option value="34">Alphasniper828, GP: 1312831</option>

  <option value="35">RustyRooster, GP: 1200016</option>

  <option value="36">CommanderCrosshair, GP: 1084318</option>

  <option value="37">Riycer, GP: 5810429</option>

  <option value="38">Fatherllama, GP: 1403024</option>

  <option value="39">Tbagtooine, GP: 2368169</option>

  <option value="40">Aaranvor, GP: 785152</option>

  <option value="41">MINI Antares Stiasa, GP: 8625284</option>

  <option value="42">ChaosTitan, GP: 8544353</option>

  <option value="43">MINI The Senate, GP: 4924071</option>

  <option value="44">Mots Zrawchs, GP: 1040804</option>

  <option value="45">DarthNeka, GP: 7209995</option>

  <option value="46">IvanTgreat, GP: 7809965</option>

  <option value="47">Ayua, GP: 1004131</option>

  <option value="48">Havoc Shmavoc, GP: 1286565</option>

  <option value="49">ReyRey, GP: 5939626</option>

</select></td></tr>
<tr><th><label for="id_strike_date">Strike date:</label></th><td><input type="date" name="strike_date" value="2022-11-12" required id="id_strike_date"></td></tr>
<tr><th><label for="id_activity">Activity:</label></th><td><select name="activity" required id="id_activity">
  <option value="">---------</option>

  <option value="TW" selected>TW</option>

  <option value="TB">TB</option>

  <option value="Tickets">Tickets</option>

  <option value="Other">Other</option>

</select></td></tr>
<tr><th><label for="id_ishard">Ishard:</label></th><td><input type="checkbox" name="ishard" id="id_ishard" checked></td></tr>
<tr><th><label for="id_comments">Comments:</label></th><td><textarea name="comments" cols="40" rows="5" maxlength="200" id="id_comments">
Today asdfasdfasdfsadfasdf asdf asdf </textarea></td></tr>

I think I might need to make an instance (?) but if so I’m not sure what to use to set it.

How does this strike_list view know which instance of Strike to update?

Of course that was the problem. This is what worked:

def strike_list(request, **pk):
  strikes = Strike.objects.all()
  if request.method == 'POST':
    form = StrikeModelForm(request.POST)
    pk = pk['pk']
    strike, created = Strike.objects.get_or_create(id=pk)
    body = request.body.decode("UTF-8")
    result = dict((a.strip(), b.strip())
                  for a, b in (element.split('=') 
                               for element in body.split('&')))
    player = Player.objects.get(id=result['player'])
    strike.player = player
    strike.strike_date = result['strike_date']
    strike.activity = result['activity']
    ishard = True if 'ishard' in result else False
    strike.ishard = ishard
    strike.comments = result['comments']
    strike.save()
  form = StrikeModelForm()
  context = {'form': form, 'strikes': strikes}
  return render(request, "Striker/strikes.html", context)

I like it because it handles CRU in one view. Delete isn’t here but that’s handled with an htmx request.

Well I guess I’m not quite there because new strikes aren’t being created though existing ones are updated. I’m sure that’s a pretty quick fix.

When the Strike is new there is no pk passed to the function. I thought that get_or_create would automatically create the new record and assign the appropriate next id but it complains if it’s not there. For new strikes pk is an empty dictionary so I get

Field 'id' expected a number but got {}

It seems I must have some value to pass, so how do I tell it to use the next id if the strike is being created?
I guess I could put back in the old processing for new strikes and use it if pk={} to save the record. Just thought that wouldn’t be necessary.

So this does both create and update functions.

def strike_list(request, **pk):
  strikes = Strike.objects.all()
  if request.method == 'POST':
    if len(pk) == 0:
      form = StrikeModelForm(request.POST)
      print(form)
      if form.is_valid():
        form.save()
        strikes = Strike.objects.all()
        context = {'form': form, 'strikes': strikes}
        return render(request, 'Striker/partials/success.html')
      else:
        return render(request, 'Striker/partials/failure.html')
    else:
      pk = pk['pk']
      strike, created = Strike.objects.get_or_create(id=pk)
      body = request.body.decode("UTF-8")
      result = dict((a.strip(), b.strip())
                    for a, b in (element.split('=') 
                                 for element in body.split('&')))
      player = Player.objects.get(id=result['player'])
      strike.player = player
      strike.strike_date = result['strike_date']
      strike.activity = result['activity']
      ishard = True if 'ishard' in result else False
      strike.ishard = ishard
      strike.comments = result['comments']
      strike.save()
  form = StrikeModelForm()
  context = {'form': form, 'strikes': strikes}
  return render(request, "Striker/strikes.html", context)