Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sort corporate donors by donation amount within each bracket #1867

Open
jacobian opened this issue Jan 9, 2025 · 6 comments
Open

Sort corporate donors by donation amount within each bracket #1867

jacobian opened this issue Jan 9, 2025 · 6 comments

Comments

@jacobian
Copy link
Member

jacobian commented Jan 9, 2025

I'm looking at the corporate donors on https://www.djangoproject.com/fundraising/. The intent there is that, within each donation bracket, donors are meant to be sorted by donation amount, then alphabetically (e.g. so if a Silver donor donated $7,000, the'd be sorted above one who donated $5,000, even though both are in the Silver bracket).

However, responding to a question from a donor, I think I've discovered two problems:

  1. The code actually sorts by total amount donated all time, not the current donation level:
    class CorporateMemberManager(models.Manager):
    def for_public_display(self):
    objs = (
    self.get_queryset()
    .filter(
    invoice__expiration_date__gte=timezone_today(),
    )
    .annotate(donated_amount=models.Sum("invoice__amount"))
    )
    return objs.order_by("-donated_amount", "display_name")
    def by_membership_level(self):
    members_by_type = defaultdict(list)
    members = self.for_public_display()
    for member in members:
    key = MEMBERSHIP_TO_KEY[member.membership_level]
    members_by_type[key].append(member)
    return members_by_type
  2. And it doesn't appear to be working. I don't want to publish donation amounts, but anyone with access to the live site data should check out Bronze sponsors. The sort there appears purely alphabetical, but there are large differences in donation amounts there. (If whoever ends up debugging this doesn't have access to live donation data, feel free to contact me privately and I'll get you that access to debug.)

(1) is arguably by design and not a bug, so I'm gonna discuss this with the board and the fundraising team to double check that my understanding of the intent (sort by current donation amount, not total amount) is correct.

But (2) is definitely a bug -- the code clear intends to sort by donation amount, but the sort seems to be lost somewhere somehow.

@jefftriplett
Copy link
Member

I always mess annotate up because it doesn't fit my brain. I don't see anything obvious, but may there is a default that's being weird that a filter kwargs might help work around. <-- totally grasping.

@adamzap
Copy link
Member

adamzap commented Jan 10, 2025

I'm having trouble reproducing the issue locally, and I don't have access to production data. @jacobian, could it be that some of the Invoices in question are past their expiration_date and aren't being summed? That's the only thing I can think of. CorporateMemberManager.for_public_display is called by the code you referenced, and it filters out expired Invoices here.

The order_by logic seems fine to me because the call at the end of for_public_display should overwrite the model's default ordering and persist through the dictionary construction until the template is rendered.

@sarahboyce
Copy link
Contributor

Invoices in question are past their expiration_date and aren't being summed

Or that they don't have an expiration_date? I also don't have access to the data and struggling to replicate

@adamzap
Copy link
Member

adamzap commented Jan 10, 2025

Or that they don't have an expiration_date? I also don't have access to the data and struggling to replicate

Oh yes, good thinking!

Also, it's interesting that the end of the bronze tier on production doesn't seem to be sorted by display_name, so not "purely alphabetical" as mentioned above?

image

@jefftriplett
Copy link
Member

I dug a little deeper into our Platinum and Gold members.

The invoice__expiration_date__gte=timezone_today() limits us to the last payment that was made and hasn't expired.

So what you are seeing for these categories is correct because a few sponsors have paid more. None of the Platinum have paid the same amount. I would argue a few should be moved to Gold, but I can bring that up with the board.

We also have at least one that's listed as $0 because they pay monthly through GitHub Sponsors and that doesn't generate an Invoice record for our CorporateMember model.

Side note: I think we'd have to switch the filter after the annotate to get the historical sum.

@StephanieAG
Copy link

StephanieAG commented Jan 26, 2025

To clarify @sarahboyce's thought: @jefftriplett, do all invoices have an expiration_date? (So not those you mentioned who pay through GitHub Sponsors because they don't have an invoice record at all, but just the ones that do.)

Related clarification: is the expiration_date the literal date of expiration for the invoice? (Practically speaking, I imagine this would equate to something like Net 30 terms, so the expiration date would consistently be 30 days after the date an invoice is sent.) Or is expiration_date instead used to flag the date when a sponsor should be followed up with to see if they'd be interested in donating again? (Practically speaking, I imagine this would equate to something like the same date of the payment is made but in the following year but could be inconsistent between sponsors if some prefer to pay in different time intervals, e.g. monthly instead of annually.)

Regarding the intent to sort by donation amount, then alphabetically, and how "the invoice__expiration_date__gte=timezone_today() limits us to the last payment that was made and hasn't expired.":

I don't know the original intent, but my .02 is that it'd be helpful to use the invoices' paid_date so that any paid invoices within the last year of today's date are included, then sort by amount sum, then alphabetically for donations matching in summed amount. This way, it's still based on recent donations (as opposed to all historical donations), but a sponsor that gives two donations of $5k (so $10k total) within the last year would show up before a sponsor that made one donation of $6k within the last year.

That said, I don't know the original reason for using expiration_date (so maybe there's a good reason to do it this way that I don't know?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants