Skip to content

Commit f5daa5d

Browse files
athul-rsclaude
andauthored
UN-4481 [FEAT] Show each organization's home region in the organization list (#2370)
* UN-4481 [FEAT] Show the home region for organizations copied from another region When the identity provider metadata names the region an organization was created in, the organization list shows that region. For a local copy of an organization from another region it leaves out member_count, which on this deployment covers only the people who opened the copy. Organizations without the stamp keep the current behaviour until they are back-filled. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * UN-4481 [FEAT] Read the region stamp only where the deployment has regions Review fixes: OSS and on-prem ignore a region key in the provider metadata, so the contract that they never send region holds; a stamp that is not a non-empty string counts as no stamp. Tests for both, and a corrected test comment. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * UN-4481 [FIX] Compare region stamps without case or padding Trim and uppercase the metadata stamp and settings.REGION before comparing, so "us" or " US " on the home deployment no longer hides the member count. Log a stamp that is present but not a non-empty string. Pin a mixed list, a this-region organization with no data here, and non-dict metadata. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * UN-4481 [DOCS] Say what the list does for organizations without a stamp The schema, ReadMe and module docstring said member_count is absent for an organization that belongs to another region. The code can tell only from the metadata stamp, and ignores it where REGION is unset. Say that, and that an unstamped copy of another region's organization sends the copy's count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 3c59e85 commit f5daa5d

4 files changed

Lines changed: 158 additions & 15 deletions

File tree

‎backend/account_v2/ReadMe.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@
5353
| Field | Present when | Value |
5454
|---|---|---|
5555
| `role` | The caller has a membership in the organization on this deployment. | The stored role: `admin` or `user` in OSS; `unstract_admin`, `unstract_user`, `unstract_supervisor` or `unstract_reviewer` in cloud. |
56-
| `member_count` | The organization exists on this deployment. | The people the members page lists: the organization's member records on this deployment. Platform API key accounts and pending invitations are not counted. |
57-
| `region` | The organization exists on this deployment and the deployment sets `REGION`. | The region code (`US`, `EU`; `STAGING`, `DEV` or `INTEGRATION` outside production). The client maps it to a label. OSS and on-prem never send it: their settings don't read `REGION`, whatever the environment sets. It is the region of the deployment that answered: an organization from another region that the user once opened here has a local copy, so it shows this region and this deployment's member count. |
56+
| `member_count` | The organization exists on this deployment and, where the deployment sets `REGION`, its metadata doesn't name a different region. An organization created before the stamp and not back-filled carries no region, so a local copy of one from another region still sends the copy's count. | The people the members page lists: the organization's member records on this deployment. Platform API key accounts and pending invitations are not counted. |
57+
| `region` | The deployment sets `REGION`, and either the organization's metadata names a region or the organization exists on this deployment. | The region code (`US`, `EU`; `STAGING`, `DEV` or `INTEGRATION` outside production), from the metadata trimmed and uppercased, else this deployment's `REGION`. The client maps it to a label. OSS and on-prem never send it: their settings don't read `REGION`, whatever the environment sets. Cloud stamps `region` on an organization's metadata where it is created. An organization from another region that the user once opened here has a local copy; its row shows the home region and no `member_count`, since this deployment counts only the people who opened the copy. Organizations created before the stamp and not back-filled show the region of the deployment that answered. |
5858

5959
If working out these three fields fails, the list is returned without them and the failure is logged.
6060

‎backend/account_v2/organization_details.py‎

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,23 @@
66
out of the row rather than sent as null:
77
88
- ``role``: the caller has a membership row in that organization here.
9-
- ``member_count``: the organization exists here. Counts the people the
10-
members page lists: the organization's member records here, without
11-
platform API key accounts. Pending invitations have no record yet.
12-
- ``region``: the organization exists here and ``settings.REGION`` is set.
9+
- ``member_count``: the organization exists here and, where
10+
``settings.REGION`` is set, its metadata names no other region. Counts the
11+
people the members page lists: the organization's member records here,
12+
without platform API key accounts. Pending invitations have no record yet.
13+
- ``region``: only where ``settings.REGION`` is set: the region named in the
14+
organization's metadata, else ``settings.REGION`` when the organization
15+
exists here.
16+
17+
Cloud stamps ``region`` on an organization's metadata where it is created.
18+
Opening it from another region makes a local copy there, so where the stamp
19+
names another region the row shows that region and leaves out this
20+
deployment's member count, which covers only the people who opened the copy.
21+
An organization without the stamp (created before it and not back-filled) is
22+
treated as this region's: a copy of one shows this region and the copy's count.
1323
"""
1424

25+
import logging
1526
from collections.abc import Sequence
1627
from typing import Any, Protocol
1728

@@ -21,6 +32,8 @@
2132

2233
from account_v2.models import Organization, User
2334

35+
logger = logging.getLogger(__name__)
36+
2437

2538
class ListedOrganization(Protocol):
2639
"""What the auth services return for each organization.
@@ -34,6 +47,21 @@ class ListedOrganization(Protocol):
3447
name: str
3548

3649

50+
def _home_region(metadata: object) -> str | None:
51+
"""The ``region`` in an organization's metadata (from the identity provider).
52+
53+
Returned trimmed and uppercased; None when there is no usable stamp.
54+
"""
55+
stamp = metadata.get("region") if isinstance(metadata, dict) else None
56+
if stamp is None:
57+
return None
58+
code = stamp.strip().upper() if isinstance(stamp, str) else ""
59+
if not code:
60+
logger.warning("Ignoring malformed region stamp %r", stamp)
61+
return None
62+
return code
63+
64+
3765
def with_member_details(
3866
organizations: Sequence[ListedOrganization], user: User
3967
) -> list[dict[str, Any]]:
@@ -67,21 +95,26 @@ def with_member_details(
6795
user=user, organization__organization_id__in=ids
6896
).values_list("organization__organization_id", "role")
6997
)
70-
region = getattr(settings, "REGION", None)
98+
region = (getattr(settings, "REGION", None) or "").strip().upper() or None
7199

72100
rows = []
73101
for org in organizations:
102+
metadata = getattr(org, "metadata", None)
103+
home = _home_region(metadata) if region else None
74104
row: dict[str, Any] = {
75105
"id": org.id,
76106
"display_name": org.display_name,
77107
"name": org.name,
78-
"metadata": getattr(org, "metadata", None),
108+
"metadata": metadata,
79109
}
80110
if org.id in roles:
81111
row["role"] = roles[org.id]
82-
if org.id in member_counts:
112+
exists_here = org.id in member_counts
113+
is_copy = home is not None and home != region
114+
if exists_here and not is_copy:
83115
row["member_count"] = member_counts[org.id]
84-
if region:
85-
row["region"] = region
116+
org_region = home or (region if exists_here else None)
117+
if org_region:
118+
row["region"] = org_region
86119
rows.append(row)
87120
return rows

‎backend/account_v2/serializer.py‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,18 @@ class GetOrganizationsResponseSerializer(serializers.Serializer):
4949
required=False,
5050
help_text="The people the members page lists, without platform API key "
5151
"accounts. Absent if the organization has no data on this deployment, "
52-
"or if the details could not be loaded.",
52+
"if the deployment has regions and the organization's metadata names "
53+
"another one, or if the details could not be loaded. Organizations "
54+
"without a region in their metadata count as this deployment's: see "
55+
"account_v2/ReadMe.md.",
5356
)
5457
region = serializers.CharField(
5558
required=False,
56-
help_text="The deployment's region code, e.g. US or EU. Absent where "
57-
"the deployment has no regions, the organization has no data here, or "
58-
"the details could not be loaded.",
59+
help_text="The region code, e.g. US or EU, named in the organization's "
60+
"metadata, else this deployment's region if the organization exists "
61+
"here. Absent where the deployment has no regions, the region is not "
62+
"known here, or the details could not be loaded. See account_v2/ReadMe.md "
63+
"for organizations without a region in their metadata.",
5964
)
6065

6166
def to_representation(self, instance): # type: ignore

‎backend/account_v2/test_organization_details.py‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,3 +164,108 @@ def test_any_object_with_the_fields_works(self) -> None:
164164

165165
def test_no_organizations(self) -> None:
166166
self.assertEqual(with_member_details([], self.caller), [])
167+
168+
169+
# Cloud stamps the region an organization was created in on its identity
170+
# provider metadata, which the auth service returns with each organization.
171+
@override_settings(REGION="US")
172+
class HomeRegionTests(TestCase):
173+
def setUp(self) -> None:
174+
self.caller = _user("caller@example.com")
175+
self.org = _org("org-a", "Org A")
176+
OrganizationMember.objects.create(
177+
organization=self.org, user=self.caller, role="admin"
178+
)
179+
180+
def _row(self, metadata: object, org_id: str = "org-a") -> dict:
181+
org = SimpleNamespace(
182+
id=org_id, display_name="Org A", name=org_id, metadata=metadata
183+
)
184+
return with_member_details([org], self.caller)[0]
185+
186+
def test_home_organisation_keeps_this_regions_details(self) -> None:
187+
row = self._row({"region": "US"})
188+
self.assertEqual((row["region"], row["member_count"]), ("US", 1))
189+
190+
def test_local_copy_shows_its_home_region(self) -> None:
191+
row = self._row({"region": "EU"})
192+
self.assertEqual(row["region"], "EU")
193+
# This deployment's count covers only the people who opened the copy here.
194+
self.assertNotIn("member_count", row)
195+
self.assertEqual(row["role"], "admin")
196+
197+
def test_organisation_with_no_data_here_still_shows_its_region(self) -> None:
198+
row = self._row({"region": "EU"}, org_id="org-x")
199+
self.assertEqual(row["region"], "EU")
200+
self.assertNotIn("member_count", row)
201+
self.assertNotIn("role", row)
202+
203+
def test_unstamped_organisation_falls_back_to_this_region(self) -> None:
204+
# Organizations created before the stamp, until they are back-filled.
205+
row = self._row({"environment": "cloud-production"})
206+
self.assertEqual((row["region"], row["member_count"]), ("US", 1))
207+
208+
@override_settings(REGION=None)
209+
def test_deployment_without_regions_ignores_the_stamp(self) -> None:
210+
# OSS and on-prem: a region key in the provider's metadata changes nothing.
211+
row = self._row({"region": "EU"})
212+
self.assertNotIn("region", row)
213+
self.assertEqual(row["member_count"], 1)
214+
215+
def test_stamp_that_is_not_a_non_empty_string_is_ignored(self) -> None:
216+
for stamp in ("", " ", ["EU"], 7):
217+
with self.subTest(stamp=repr(stamp)):
218+
with self.assertLogs("account_v2.organization_details", "WARNING"):
219+
row = self._row({"region": stamp})
220+
self.assertEqual((row["region"], row["member_count"]), ("US", 1))
221+
222+
def test_stamp_is_compared_without_case_or_padding(self) -> None:
223+
for stamp, region, count in (
224+
("us", "US", 1),
225+
(" US ", "US", 1),
226+
("eu", "EU", None),
227+
):
228+
with self.subTest(stamp=stamp):
229+
row = self._row({"region": stamp})
230+
self.assertEqual(
231+
(row["region"], row.get("member_count")), (region, count)
232+
)
233+
234+
@override_settings(REGION="us")
235+
def test_this_deployments_region_is_compared_without_case(self) -> None:
236+
row = self._row({"region": "US"})
237+
self.assertEqual((row["region"], row["member_count"]), ("US", 1))
238+
239+
def test_metadata_that_is_not_a_dict_counts_as_unstamped(self) -> None:
240+
for metadata in (None, "EU", ["EU"]):
241+
with self.subTest(metadata=repr(metadata)):
242+
row = self._row(metadata)
243+
self.assertEqual((row["region"], row["member_count"]), ("US", 1))
244+
self.assertEqual(row["metadata"], metadata)
245+
246+
def test_this_regions_organisation_with_no_data_here(self) -> None:
247+
row = self._row({"region": "US"}, org_id="org-x")
248+
self.assertEqual(row["region"], "US")
249+
self.assertNotIn("member_count", row)
250+
self.assertNotIn("role", row)
251+
252+
def test_mixed_list_keeps_order_and_queries(self) -> None:
253+
_org("org-b", "Org B")
254+
orgs = [
255+
CloudOrg(id="org-a", display_name="A", name="a", metadata={"region": "EU"}),
256+
CloudOrg(id="org-b", display_name="B", name="b", metadata=None),
257+
CloudOrg(id="org-x", display_name="X", name="x", metadata={"region": "US"}),
258+
]
259+
with self.assertNumQueries(2):
260+
rows = with_member_details(orgs, self.caller)
261+
self.assertEqual(
262+
[
263+
(r["id"], r.get("region"), r.get("member_count"), r.get("role"))
264+
for r in rows
265+
],
266+
[
267+
("org-a", "EU", None, "admin"),
268+
("org-b", "US", 0, None),
269+
("org-x", "US", None, None),
270+
],
271+
)

0 commit comments

Comments
 (0)