Skip to content

Commit a0cadbe

Browse files
committed
fix(roles): apply role_overrides when profile has no synced membership data
get_external_memberships short-circuited to {} whenever is_member_of was None or empty. The guard predates role_overrides: it was added (Jun 2025) purely to keep json.loads from raising on never-synced profiles, and the override overlay was later (Nov 2025) inserted behind it without revisiting the early return. As a result a manual override on a never-synced profile silently counted for nothing — until any sync wrote is_member_of JSON (even all-false), at which point the override abruptly took effect. Treat missing sync data as an empty base dict instead, so the manual layer always applies. api_only=True continues to exclude overrides, and malformed JSON still returns {} via the existing exception path. (cherry picked from commit a6b1339)
1 parent 41ad81f commit a0cadbe

2 files changed

Lines changed: 82 additions & 5 deletions

File tree

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
"""
2+
Tests for get_external_memberships, the canonical final-roles merge.
3+
4+
Covers the read-time overlay of the manual role_overrides layer onto the
5+
synced is_member_of JSON — in particular that overrides still apply when
6+
a profile has never been synced (is_member_of None or empty), and that
7+
api_only=True never includes the manual layer.
8+
"""
9+
10+
import json
11+
12+
from django.test import TestCase
13+
14+
from knowledge_commons_profiles.newprofile.models import Profile
15+
from knowledge_commons_profiles.rest_api.utils import get_external_memberships
16+
17+
18+
class GetExternalMembershipsTests(TestCase):
19+
def _profile(self, is_member_of=None, role_overrides=None):
20+
return Profile.objects.create(
21+
username="bonnie",
22+
is_member_of=is_member_of,
23+
role_overrides=role_overrides or [],
24+
)
25+
26+
def test_merges_overrides_onto_synced_memberships(self):
27+
profile = self._profile(
28+
is_member_of=json.dumps({"HASTAC": True, "STEMED+": False}),
29+
role_overrides=["STEMED+"],
30+
)
31+
self.assertEqual(
32+
get_external_memberships(profile),
33+
{"HASTAC": True, "STEMED+": True},
34+
)
35+
36+
def test_overrides_apply_when_is_member_of_is_none(self):
37+
profile = self._profile(
38+
is_member_of=None, role_overrides=["STEMED+"]
39+
)
40+
self.assertEqual(
41+
get_external_memberships(profile), {"STEMED+": True}
42+
)
43+
44+
def test_overrides_apply_when_is_member_of_is_empty_string(self):
45+
profile = self._profile(
46+
is_member_of="", role_overrides=["STEMED+"]
47+
)
48+
self.assertEqual(
49+
get_external_memberships(profile), {"STEMED+": True}
50+
)
51+
52+
def test_no_data_at_all_returns_empty(self):
53+
profile = self._profile(is_member_of=None, role_overrides=[])
54+
self.assertEqual(get_external_memberships(profile), {})
55+
56+
def test_api_only_never_includes_overrides(self):
57+
profile = self._profile(
58+
is_member_of=json.dumps({"HASTAC": True}),
59+
role_overrides=["STEMED+"],
60+
)
61+
self.assertEqual(
62+
get_external_memberships(profile, api_only=True),
63+
{"HASTAC": True},
64+
)
65+
66+
def test_api_only_with_no_sync_data_returns_empty(self):
67+
profile = self._profile(
68+
is_member_of=None, role_overrides=["STEMED+"]
69+
)
70+
self.assertEqual(
71+
get_external_memberships(profile, api_only=True), {}
72+
)
73+
74+
def test_malformed_json_returns_empty(self):
75+
profile = self._profile(
76+
is_member_of="{not json", role_overrides=["STEMED+"]
77+
)
78+
self.assertEqual(get_external_memberships(profile), {})

knowledge_commons_profiles/rest_api/utils.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -190,12 +190,11 @@ def get_external_memberships(obj: Profile, api_only=False):
190190
return {}
191191

192192
try:
193-
# Handle case where is_member_of might be None or empty
193+
# is_member_of may be None or empty for profiles that have never
194+
# been synced; treat that as an empty base so the manual
195+
# role_overrides layer still applies below
194196
member_data = obj.is_member_of
195-
if not member_data:
196-
return {}
197-
198-
member_json = json.loads(member_data)
197+
member_json = json.loads(member_data) if member_data else {}
199198

200199
if not api_only:
201200
for role in obj.role_overrides:

0 commit comments

Comments
 (0)