[FIXED] How to Structure Models and Forms

Issue

I'm working on a Django app to track scoring for Golf. I'm currently spinning my wheels on how to structure my models and forms for a scorecard form. I've found forms to be a bit more difficult than I thought they would be.

I'm trying to figure out if i'm going about this the right way? i've thought about combining the GolfRound and GolfHole into one model, but it feels kind of wrong to have a field for each hole.

I'm leaning towards a nested inlineformset, but i'm not sure if that's the right approach.

Here are my models:

***MODELS***
class TeeTime(models.Model):
    updated = models.DateTimeField(auto_now=True)
    timestamp = models.DateTimeField(auto_now_add=True)
    activity = models.ForeignKey(GolfActivity, on_delete=models.CASCADE, 
    related_name='event_teetime')
    tee_time = models.TimeField(auto_now=False, auto_now_add=False, blank=True)
    players = models.ManyToManyField(GolfTripMember, blank=True, through='TeeTimePlayers')


class GolfRound(models.Model):
    tee_time = models.ForeignKey(TeeTime, on_delete=models.CASCADE, related_name = 'round')
    golfer =  models.ForeignKey(GolfTripMember, on_delete=models.CASCADE, related_name = 'round_player')
    gross_score = models.IntegerField(null=True)
    net_score = models.IntegerField(null=True)
    par_3 = models.BooleanField(null=True, default=False)
    complete = models.BooleanField(null=True, default=False)
    active = models.BooleanField(null=True, default=False)


class HoleScore(models.Model):
    name = models.IntegerField(choices=INDEX, default = INDEX[0][0])
    round = models.ForeignKey(GolfRound, on_delete=models.CASCADE, related_name='hole_score')
    par = models.IntegerField(null=True)
    HCP_index = models.IntegerField(null=True)
    netscore = models.IntegerField(null=True, blank=True)
    grossscore = models.IntegerField(null=True, blank=True)

Any thoughts on my model structure, and approaches I should take to implement the form?

Here is a typical scorecard for the non-golfers out there. Each Row represents a different golfer and their score.

Solution

After a little more attempts to understand how golf works, I think I found the right structure. I will be damned if it doesn't make sense.

• MODELS

I made a few changes to the model schema.

1. Course

I added GolfCourse model in order to associate each GolfRound with a particular course.

2. Hole

I made Hole its own model and moved par to it. I learned that each hole has its own par value.

3. GolfRound

I took out all score fields. I realised these are calculated fields and as such, they should be calculated only when needed not as model attributes. The view will handle the calculations.

4. Score

This model should have FK to the GolfRound, Hole, and Golfer. Additionally, it should have strokes attributes (the number of times the golfer hit the ball). This is what will be used to calculate their score (Gross or Net).

5. Golfer

One additional attribute here is the handicap. I learned that each golfer has their own handicap value. And since it will be important in calculating their Net score, the Golfer model seems a logical place to put it.

class Hole(models.Model):
    golf_course = models.ForeignKey("GolfCourse", on_delete=models.CASCADE)
    number = models.PositiveIntegerField()
    name = models.CharField(max_length=50)
    par = models.PositiveIntegerField()
    women_par = models.PositiveIntegerField()
    played = models.BooleanField(default=False)

    def __str__(self):
        return self.name
class Golfer(models.Model):
    name = models.CharField(max_length=50)
    handicap = models.PositiveIntegerField(default=3)

    def __str__(self):
        return self.name

class GolfCourse(models.Model):
    name = models.CharField(max_length=50)

    def __str__(self):
        return self.name

class GolfRound(models.Model):
    date = models.DateTimeField(auto_now_add=True)
    course = models.ForeignKey(GolfCourse, on_delete=models.CASCADE, verbose_name=_("Golf course"))

    def __str__(self):
        return f"Round {self.pk}"

    def save(self, *args, **kwargs):
        super().save(*args, **kwargs)
        # When starting a new round, mark all holes as not played. 
        # it will be important in the views
        Hole.objects.update(played=False)

class Score(models.Model):
    golfer = models.ForeignKey(Golfer, on_delete=models.CASCADE)
    golf_round = models.ForeignKey(GolfRound, on_delete=models.CASCADE)
    hole = models.ForeignKey(Hole, on_delete=models.CASCADE, limit_choices_to=Q(played=False))
    strokes = models.PositiveIntegerField()

    def __str__(self):
        return f"{self.golfer.name}: {self.hole.name} -- {self.strokes}"

    def save(self, *args, **kwargs):
        super().save(*args, **kwargs)
        # When the hole has been played, i.e. after saving the inlineformset, mark the hole as played.
        # It's useful in the views and convenient rendering of the formse
        self.hole.played = True
        self.hole.save(update_fields=["played"])

• FORMS There are only two forms: GolfRoundForm for creating new round and score_inlineformset for saving the golfers' scores for that particular round.

score_inline = forms.inlineformset_factory(
    parent_model=GolfRound, 
    model=Score, 
    fields="__all__", 
    can_delete=False, 
    extra=Golfer.objects.count() 
    # The extra will ensure that the inlineformset provides exactly the same nmber of forms as there are players
    # By default, but it can be improved, see related comment below
)

class GolfRoundForm(forms.ModelForm):
    class Meta:
        model = GolfRound
        fields = "__all__"

• VIEWS For simplicity (debatable), I only have one view class that dynamically renders the inline_formset or the GolfRound form. I am using a single view but the logic can be separated to two views: one for Scores, another for GolfRound. Anywho, the view is commented. Of importance is the calculation of gross and net scores. Gross score is straightforward, but net seems to depend on several factors (that determine a golfer's handicap. I don't know how well this can be calculated.

To calculate the scores (which are part of the summary), the view checks if the golfer has completed all 18 holes. I the number of holes on a course may vary (it can easily be a dynamic constant or whatever.

This approach, however, is prone to fail. For example, if the score of the user is deleted, their scores will be shifted to the left, creating discrepancies in the presented summary scores.

It also assumes that the holes were played in successive order. That is, if golfers played third hole then came back to the second, the summary will still assume they played them in order. This is why I had to preload the holes with the next available hole. If golfers are allowed to play any random ball, then this is surely a bad implementation.

class GolfView(generic.View):
    template_name = "core/sport/golf.html"

    @property
    def next_hole(self):
        # This will be used to automatically fill the next hole once all playes have played the hole
        return Hole.objects.filter(played=False) 
    
    @property
    def next_hole_exists(self):
        # This will be used to automatically fill the next hole once all playes have played the hole
        return self.next_hole.exists() 

    def get(self, request, *args, **kwargs):
        golf_round = GolfRound.objects.get(pk=kwargs.get("golf_round_pk"))

        holes_available = True
        
        all_course_holes = Hole.objects.all()

        # dictionaries of golfer and their scores for this round will appended here
        golfer_scores = [] 
        # Get all scores for this round
        scores = Score.objects.filter(golf_round=golf_round) 

        # This  gets all golfers registered, but it could be improved,
        # see the next block
        for golfer in Golfer.objects.filter(): 
            scores = list(golfer.score_set.filter(golf_round=golf_round).values_list("strokes", flat=True))
            if len(scores) == 18:
                gross = sum(scores)
                net = sum(scores) - golfer.handicap
                scores.extend([gross, net])
            dict_ = {"name": golfer, "scores": scores}
            if scores:
                golfer_scores.append(dict_)

        if self.next_hole_exists: # We still have holes to play
            # Initial list will prepopulate the scores inlineformset with available players
            # by default it is loading all registered golfers, perhaps the better approach 
            # would be to filter only those registered for this particular round 
            initial = [{"golfer": golfer.pk, "hole": self.next_hole.first().pk} for golfer in Golfer.objects.all()] 

            formset = score_inline(
                instance=golf_round, 
                queryset=Score.objects.none(), # queyset argument has to be none because we will be creating new instances
                initial=initial,
                )
            btn_value = "Save scores"
            table_caption = "Current scores"
            context = locals()
        
        # All holes have been played, generate round summary i.e. final scorecard, and offer to start new Round            
        else:
            holes_available = False
            round_form = GolfRoundForm()
            btn_value = "Start new round"
            table_caption = "Final scores"
            context = locals()
        return render(request, self.template_name, context)

    def post(self, request, *args, **kwargs):
        if self.next_hole_exists:
            golf_round = GolfRound.objects.get(pk=kwargs.get("golf_round_pk"))
            formset = score_inline(request.POST, instance=golf_round, initial=[{"golfer": golfer.pk} for golfer in Golfer.objects.all()])
            if formset.is_valid():
                formset.save()
                return redirect("core:golf-home", golf_round.pk)
        else:
            round_form = GolfRoundForm(request.POST)
            if round_form.is_valid():
                golf_round = round_form.save()
                return redirect("core:golf-home", golf_round.pk)
        return render(request, self.template_name, locals())

• HTML

<div class="w-10/12 mx-auto mt-10 bg-red-400 rounded-lg">
        <h1 class="text-xl text-center py-2">Golfers scores: {{golf_round}}</h1>
  <form action="" method="post" class="w-full bg-green-400 text-center px-4 py-3">
  {% csrf_token %}
  {% if holes_available %}
    {{formset.management_form}}
    {% for form in formset %}
      <div class="grid grid-cols-3 gap-3 ">
        {% for field in form.visible_fields %}
          {{field.as_field_group}}
        {% endfor %}
          </div>
    {% endfor %}
  {% else %}
    <div class="grid w-1/2 mx-auto">
     {{round_form.course.as_field_group}}
    </div>
  {% endif %}
    <input type="submit" value="{{btn_value}}" class="w-5/12 rounded-md mt-5 p-2 bg-blue-900 text-white cursor-pointer">
  </form>

</div>

<hr class="border-t-2 border-black mt-6">

<div class="w-10-12 mx-auto mt-5">
  <table class="table-fixed w-11/12 mx-auto text-center border rounded-lg">
    <caption class="border-b-4 border-black text-2xl pb-2 mb-1">{{table_caption}}</caption>
    <thead>
      <tr class="border-b-2 border-black">
        <th class="w-32">Hole no.</th>
        {% for hole in all_course_holes %}
          <th>{{hole.number}}</th>
        {% endfor %}
        <th>Gross</th>
        <th>Net</th>
      </tr>
      <tr class="border-b-2 border-black">
        <th>Par</th>
        {% for hole in all_course_holes %}
          <th>{{hole.par}}</th>
        {% endfor %}
      </tr>
      <tr class="border-b-4 border-black">
        <th>Women par</th>
        {% for hole in all_course_holes %}
          <th>{{hole.women_par}}</th>
        {% endfor %}
      </tr>
    </thead>
    <tbody>
      {% for player in golfer_scores %}
      <tr class="border-b-2 border-black">
        <td>{{player.name}}</td>
        {% for score in player.scores %}
          <td>{{score}}</td>
        {% endfor %}
      </tr>
      {% endfor %}
    </tbody>
  </table>
</div>

This is the final look I got:

Answered By - McPherson