Skip to content

Fix empty has_many associations incorrectly returning true#53

Merged
vincentvanbush merged 1 commit intomasterfrom
52-empty-has_many-associations-incorrectly-return-true-in-permission-checks
Feb 2, 2026
Merged

Fix empty has_many associations incorrectly returning true#53
vincentvanbush merged 1 commit intomasterfrom
52-empty-has_many-associations-incorrectly-return-true-in-permission-checks

Conversation

@vincentvanbush
Copy link
Collaborator

@vincentvanbush vincentvanbush commented Feb 2, 2026

Description

Fixes #52

This PR fixes the issue where empty has_many associations incorrectly return true in permission checks due to Enum.all?([]) returning true (vacuous truth).

Changes

Core Fix

  • File: lib/permit/permissions/parsed_condition.ex:117
  • Changed Enum.all? to Enum.any? in check_association/3 function
  • Implements "AT LEAST ONE" (existential) semantics for has_many associations

Test Updates

  • File: test/permit/permissions/condition_test.exs
  • Added test case demonstrating the empty association bug
  • Updated existing test to reflect new "ANY" semantics
  • Added test for when no record matches all conditions simultaneously
  • Renamed test to better describe "ANY semantics" behavior

Behavior Change

Before (Incorrect):

%Movie{actors: []} # Empty list
# Enum.all?([], fn -> ... end) -> true

After (Correct):

%Movie{actors: []} # Empty list
# Enum.any?([], fn -> ... end) -> false

Impact

This change ensures that:

  • ✓ Empty associations correctly return false
  • ✓ At least one matching record returns true
  • ✓ Permission rules behave consistently with Ecto query behavior

Test Results

  • ✓ All 56 tests pass in base permit library
  • ✓ All 27 tests pass in permit_ecto (including the originally failing test from PR Issue 16/functions query #22)

Related

behaviour

Signed-off-by: Michał Buszkiewicz <michal@curiosum.com>
@codecov
Copy link

codecov bot commented Feb 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
lib/permit/permissions/parsed_condition.ex 71.42% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vincentvanbush vincentvanbush merged commit 39e0ef2 into master Feb 2, 2026
16 checks passed
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.

Empty has_many associations incorrectly return true in permission checks

2 participants