Skip to content

feat: infer country from member location (CM-1355) - #4434

Merged
skwowet merged 3 commits into
mainfrom
feat/CM-1355-infer-country-from-location
Aug 4, 2026
Merged

feat: infer country from member location (CM-1355)#4434
skwowet merged 3 commits into
mainfrom
feat/CM-1355-infer-country-from-location

Conversation

@skwowet

@skwowet skwowet commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

Infers a member country from free-text location when no country source is already set, and writes it to attributes.country.system on create/update paths.

Changes

  • Add getCountry(location) in @crowd/common (i18n-iso-countries + curated junk/ambiguous/US-state/alias maps); abstains when unsure
  • Add getAttributeValue / hasAttributeValue helpers for reading multi-source attributes
  • Wire inference before setAttributesDefaultValues on data-sink create/update, enrichment attribute updates, backend UI attributes PATCH, and legacy member upsert/update
  • UI attributes path also sets default when inferring (no priorities pass on that path)

Copilot AI balanced review requested due to automatic review settings August 4, 2026 09:38
Comment thread services/libs/common/src/country.ts Fixed
@skwowet
skwowet force-pushed the feat/CM-1355-infer-country-from-location branch from 66b8d20 to 5928c63 Compare August 4, 2026 09:42
@skwowet skwowet changed the title feat: infer member country from location when missing (CM-1355) feat: infer country from member location (CM-1355) Aug 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

The attribute helper signatures break data-sink type checking, and dependency loading and unrelated lockfile churn need correction.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Adds country inference from member location across ingestion, enrichment, and backend update paths.

Changes:

  • Adds country parsing, aliases, ambiguity filtering, and attribute helpers.
  • Applies inferred countries across member create/update flows.
  • Adds i18n-iso-countries.
File summaries
File Description
services/libs/common/src/member.ts Adds attribute value helpers.
services/libs/common/src/index.ts Exports country utilities.
services/libs/common/src/country.ts Implements country inference.
services/libs/common/src/constants/location.ts Defines location mappings and exclusions.
services/libs/common/src/constants/index.ts Exports location constants.
services/libs/common/package.json Adds country dependency.
services/apps/members_enrichment_worker/src/activities/enrichment.ts Infers country during enrichment updates.
services/apps/data_sink_worker/src/service/member.service.ts Infers country during ingestion.
backend/src/services/memberService.ts Adds inference to legacy update paths.
backend/src/services/member/memberAttributesService.ts Adds inference to attribute updates.
pnpm-lock.yaml Resolves the dependency and incidental lockfile changes.
Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (1)

services/libs/common/src/member.ts:90

  • For the same reason, attributes.country is unknown in both new data-sink paths and is not assignable to this parameter, causing type checking to fail. Narrow an unknown value inside the helper instead.
export function hasAttributeValue(attribute: Record<string, any> | null | undefined): boolean {
  return Object.values(attribute || {}).some((v) => typeof v === 'string' && v.trim().length > 0)
  • Files reviewed: 10/11 changed files
  • Comments generated: 3
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread services/libs/common/src/member.ts Outdated
Comment thread services/libs/common/src/country.ts
Comment thread pnpm-lock.yaml Outdated
Copilot AI review requested due to automatic review settings August 4, 2026 09:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

The English locale is not registered, so the new country lookups currently return no inferred country.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (4)

services/libs/common/src/member.ts:69

  • This new shared helper exposes any even though every value is explicitly narrowed before use. Use unknown so callers cannot consume unchecked values through this API, as required by the repository's type-safety guidance.
  attribute: Record<string, any> | null | undefined,

services/libs/common/src/member.ts:89

  • This second shared helper also introduces any unnecessarily; Object.values plus the existing typeof guard works with unknown and preserves type safety.
export function hasAttributeValue(attribute: Record<string, any> | null | undefined): boolean {

pnpm-lock.yaml:10957

  • These AWS peer-resolution rewrites (along with the changed glob deprecation metadata) are unrelated to adding i18n-iso-countries and can change dependency resolution outside this feature's scope. Regenerate with the repository's expected pnpm version or revert the unrelated lockfile hunks while retaining only the new dependency entries.
    services/libs/common/src/country.ts:1
  • i18n-iso-countries does not load name data automatically. No registerLocale call exists in the repository, so both getAlpha2Code(..., 'en') and getName(..., 'en') return no result and this inference path never resolves a country. Import the English locale and register it once in this module before using the lookup APIs.
import countries from 'i18n-iso-countries'
  • Files reviewed: 10/11 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Copilot AI review requested due to automatic review settings August 4, 2026 09:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

Inferred countries can be marked as manual, and the configured priorities do not enforce the documented system precedence.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (2)

backend/src/services/memberService.ts:831

  • This inferred value is added before MemberRepository.update, but the UI member update calls this method with manualChange: true. The repository treats every changed attribute default as manual (memberRepository.ts:794-845), so this system-derived country is persisted as attributes.country in manuallyChangedFields and later automated updates cannot replace it. Keep generated country data out of manual-change detection.
            data.attributes.country = {
              ...data.attributes.country,
              system: country,
              default: data.attributes.country?.default ?? country,
            }

services/apps/members_enrichment_worker/src/activities/enrichment.ts:369

  • The claimed system-over-enrichment precedence is not configured: getPlatformPriorityArray returns settings unchanged, while the default priority list in backend/src/database/models/settings.ts:34-46 omits system. Once a later enrichment country is merged, default recomputation selects enrichment and overwrites this inferred default. Add system ahead of enrichment for both default and existing persisted settings, or explicitly encode the precedence.
            attributes.country = {
              ...attributes.country,
              system: country,
            }
  • Files reviewed: 10/11 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Signed-off-by: Yeganathan S <[email protected]>
Copilot AI review requested due to automatic review settings August 4, 2026 10:01
@skwowet skwowet self-assigned this Aug 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

Inferred countries can become stale and may be incorrectly marked as manually changed.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (3)

services/libs/common/src/member.ts:96

  • Counting system and default here makes an inferred country permanent. After the location changes, every new guard sees the old inferred country.system/default as an existing value and never recomputes it, leaving the member with a stale country. Distinguish authoritative external/custom sources from the inferred system value so location updates can refresh the inference.
  return Object.values(attribute as Record<string, unknown>).some(
    (v) => typeof v === 'string' && v.trim().length > 0,
  )

backend/src/services/memberService.ts:830

  • When this inference runs during the UI member update (manualChange: true), MemberRepository.update sees the newly added attributes.country and records it in manuallyChangedFields. The value was generated by the system, not the user, and that flag causes later non-manual updates to preserve it as if it were user-owned. Exclude the inferred field from manual-change tracking or perform inference after the manual fields are determined.
            data.attributes.country = {
              ...data.attributes.country,
              system: country,
              default: data.attributes.country?.default ?? country,

services/libs/common/src/member.ts:76

  • On merged update paths this default is stale until the subsequent setAttributesDefaultValues call. For example, changing a source location from Germany to France leaves default: "Germany" during inference, so the code writes Germany as the country and then refreshes the location default to France. Resolve the priority-selected location before inferring (and then populate the country default) rather than preferring the pre-merge default here.
  if (typeof record.default === 'string' && record.default.trim()) {
    return record.default
  • Files reviewed: 10/11 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@skwowet
skwowet merged commit ed2b343 into main Aug 4, 2026
14 checks passed
@skwowet
skwowet deleted the feat/CM-1355-infer-country-from-location branch August 4, 2026 10:10
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.

3 participants