Skip to content

Fix MultipleObjectsReturned in EntityModelManager.for_user when entity has multiple managers - #329

Open
Leggendario12 wants to merge 4 commits into
arrobalytics:masterfrom
Leggendario12:master
Open

Fix MultipleObjectsReturned in EntityModelManager.for_user when entity has multiple managers#329
Leggendario12 wants to merge 4 commits into
arrobalytics:masterfrom
Leggendario12:master

Conversation

@Leggendario12

Copy link
Copy Markdown

Summary

EntityModelManager.for_user uses an OR filter across a ManyToMany relationship (managers). When an entity has
more than one manager, the underlying SQL LEFT OUTER JOIN produces one row per manager match, which causes
.get() calls downstream to raise MultipleObjectsReturned — even though only one entity actually matches.

Root Cause

qs.filter(Q(admin=user_model) | Q(managers__in=[user_model]))

translates to SQL roughly like:

SELECT django_ledger_entitymodel.*
FROM django_ledger_entitymodel
LEFT OUTER JOIN django_ledger_entitymodel_managers ON (
    django_ledger_entitymodel.id = django_ledger_entitymodel_managers.entitymodel_id
)
WHERE (
    django_ledger_entitymodel.admin_id = <user_id>
    OR django_ledger_entitymodel_managers.customuser_id = <user_id>
)

If an entity has N managers, the join produces N rows for that entity. Any view or queryset method that calls
.get() on the result of for_user() will raise MultipleObjectsReturned as soon as a single entity has 2+ managers
assigned.

Scenario That Triggers the Bug

  1. Create an entity.
  2. Assign two or more managers to the entity (entity.managers.add(user_a, user_b)).
  3. Navigate to the dashboard or any view that calls EntityModel.objects.for_user(user_model=request.user).get(...).
  4. MultipleObjectsReturned is raised.
  Error Traceback

  Request Method: GET
  Request URL: http://localhost:8000/ledger/entity/<entity-slug>/dashboard/

  Traceback (most recent call last):
    File ".../django/core/handlers/exception.py", line 55, in inner
      response = get_response(request)
    ...
    File ".../django_ledger/templatetags/django_ledger.py", line 290, in render
      entity_model = EntityModel.objects.for_user(
          user_model=request.user
      ).get(slug=entity_slug)
    File ".../django/db/models/query.py", in get
      raise self.model.MultipleObjectsReturned(...)

  django.core.exceptions.MultipleObjectsReturned: get() returned more than one EntityModel -- it returned 2!

The entity itself is not duplicated in the database — the duplicate rows are an artefact of the many-to-many join.
The count equals the number of managers on the entity.

The Fix

Add .distinct() to collapse the duplicate rows produced by the join:

Before

return qs.filter(Q(admin=user_model) | Q(managers__in=[user_model]))

After

return qs.filter(Q(admin=user_model) | Q(managers__in=[user_model])).distinct()

This is the standard Django pattern when filtering across ManyToMany relationships — see the Django docs on
spanning multi-valued relationships
(https://docs.djangoproject.com/en/stable/topics/db/queries/#spanning-multi-valued-relationships).

The fix is a one-line change. It has no effect on which entities are returned — distinct() only removes duplicate
rows. A regression test:

  entity.managers.add(user_a, user_b)
  qs = EntityModel.objects.for_user(user_model=user_a)
  assert qs.filter(slug=entity.slug).count() == 1   # was 2 before this fix
  assert qs.get(slug=entity.slug).pk == entity.pk   # was MultipleObjectsReturned

EntityModelManager.for_user builds a query using OR across a ManyToMany
(managers) relationship. Without distinct(), entities with 2+ managers
produce duplicate rows from the JOIN, causing .get() to raise
MultipleObjectsReturned. Add .distinct() to collapse the duplicates.
When no period has been closed, last_closing_date is None. Comparing
None < receipt_date raises TypeError. Treat None as before all dates
(i.e. deletion is always allowed when no period has been closed).
Files now stored as {slug}_{pk}/logo/logo{ext} instead of the root
of the media bucket, making per-entity folders in R2/S3 cleaner.
Logo is now managed exclusively via the EntityInvoiceSettings extension
model in the consuming application, keeping the core entity model lean.
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

Successfully merging this pull request may close these issues.

1 participant