feat(geo,admin): Qatar soft fallback + country picker + suppression UX#273
Merged
Conversation
Geo: - Restore Qatar as a soft visual fallback when geo detects an unsupported country (Germany, India, etc.). Backend returns the real Qatar row with source='unsupported-default' so the storefront has something to render, and the navbar country picker (new) lets visitors switch explicitly. This is NOT the silent-misattribution bug from before — the picker affordance makes the fallback user-fixable rather than invisible. - New CountryPicker component (desktop pill + mobile menu row) calls geo.setCountry() which already routes to source='manual' (never auto revalidated, never clobbered by IP geo). - geo-context: when API returns no country at all (Qatar row missing or source='unknown'), clear any stale cached country so the picker is the only forward path. Manual picks still survive (early-return preserved). Email Suppressions admin UX (no schema changes): - Add per-reason count chips (Bounces / Complaints / Manual / All) that double as filter chips. Counts come from a single Prisma groupBy in the same response as the page items. - Accept new ?reason=BOUNCE|COMPLAINT|MANUAL query param via a small EmailSuppressionsQueryDto extending PaginationDto. No DB migrations. Per-reason counts and filter live entirely off the existing schema.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds email suppression list filtering by reason with per-reason counts, and introduces a country picker component for geographic selection. The backend geo service implements a Qatar fallback for unsupported countries. The frontend integrates the new country picker into the navbar and adds filtering controls to the suppressions page. ChangesEmail Suppressions Filtering
Geo Detection and Country Picker
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/api/src/geo/geo.service.ts (1)
76-80: 💤 Low valueConsider a more intentional city ordering strategy.
The fallback city is selected as the first alphabetically by English name (
orderBy: { nameEn: 'asc' }). While deterministic, this may not be the most user-friendly default. Consider ordering by population (if available), a designatedisCapitalflag, or an explicitdisplayOrderfield to ensure the most relevant city (e.g., Doha) appears as the fallback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/geo/geo.service.ts` around lines 76 - 80, The current fallback city selection uses a simple alphabetical order in the prisma query (see prisma.client.city.findFirst and const firstCity) which may not pick the most relevant city; update the selection logic to prefer a more meaningful ordering such as isCapital, population, or an explicit displayOrder: modify the findFirst call to orderBy an ordered list (e.g., isCapital desc, displayOrder asc, population desc, nameEn asc) or implement a two-step fallback that first searches for city where isCapital = true (or displayOrder is set) for the fallbackCountry and only falls back to alphabetical if none found, ensuring the most relevant city (e.g., Doha) is chosen as the default.apps/web/src/components/country-picker.tsx (1)
59-63: ⚡ Quick winConsider adding error handling for the countries query.
The component doesn't handle query errors. If
/catalog/countriesfails, users will see an indefinite "Loading..." state with no explanation. Consider using theerrorproperty fromuseQueryto display a user-friendly error message or retry button.📡 Proposed enhancement with error state
- const { data: countries = [] } = useQuery<Country[]>({ + const { data: countries = [], error, isError } = useQuery<Country[]>({ queryKey: ['public-countries'], queryFn: () => api.get('/catalog/countries').then((r) => r.data), staleTime: 60 * 60 * 1000, });Then in the desktop dropdown (and similarly for mobile):
{countries.length === 0 ? ( - <div className="px-4 py-3 text-sm text-gray-400 dark:text-slate-500">{t('common.loading')}</div> + <div className="px-4 py-3 text-sm text-gray-400 dark:text-slate-500"> + {isError ? t('common.error', { defaultValue: 'Failed to load countries' }) : t('common.loading')} + </div> ) : (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/country-picker.tsx` around lines 59 - 63, The countries query currently only destructures data from useQuery (const { data: countries = [] } = useQuery(...)) so failures leave the component stuck on "Loading..."; update the useQuery call to also destructure error and isLoading (e.g., const { data: countries = [], error, isLoading, refetch } = useQuery(...)) and then add UI branches in country-picker.tsx (where the dropdowns are rendered) to show a user-friendly error message and a retry control (or fallback list) when error is present, and keep the existing loading state for isLoading; ensure you reference the same queryKey ['public-countries'] and the queryFn so behavior and caching remain unchanged.apps/api/src/admin/admin.service.ts (1)
2813-2833: ⚡ Quick winConsider adding EmailSuppression to the Prisma schema for type safety.
The repeated cast to
(this.prisma.client as any).emailSuppressionbypasses TypeScript's type checking. If the EmailSuppression table structure changes (e.g., column rename, type change), these queries would fail at runtime without any compile-time warning.🔧 Suggested improvement
Add the EmailSuppression model to your
schema.prismafile. If the table already exists in the database:# Run Prisma introspection to detect the existing table npx prisma db pull # Then regenerate the Prisma client npx prisma generateOr manually add the model to
schema.prismaif introspection isn't suitable. This eliminates the unsafe casts and provides full IntelliSense + compile-time checking for all three queries (lines 2813, 2829, 2830).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/admin/admin.service.ts` around lines 2813 - 2833, The code repeatedly uses unsafe casts like (this.prisma.client as any).emailSuppression for queries (emailSuppression.findMany, count, groupBy) which bypass TypeScript checks; add an EmailSuppression model to your Prisma schema (or run prisma db pull if the table already exists) and regenerate the Prisma client so you can remove the casts and use this.prisma.client.emailSuppression with proper types, allowing compile-time validation and IntelliSense for the findMany, count, and groupBy calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/api/src/admin/admin.service.ts`:
- Around line 2813-2833: The code repeatedly uses unsafe casts like
(this.prisma.client as any).emailSuppression for queries
(emailSuppression.findMany, count, groupBy) which bypass TypeScript checks; add
an EmailSuppression model to your Prisma schema (or run prisma db pull if the
table already exists) and regenerate the Prisma client so you can remove the
casts and use this.prisma.client.emailSuppression with proper types, allowing
compile-time validation and IntelliSense for the findMany, count, and groupBy
calls.
In `@apps/api/src/geo/geo.service.ts`:
- Around line 76-80: The current fallback city selection uses a simple
alphabetical order in the prisma query (see prisma.client.city.findFirst and
const firstCity) which may not pick the most relevant city; update the selection
logic to prefer a more meaningful ordering such as isCapital, population, or an
explicit displayOrder: modify the findFirst call to orderBy an ordered list
(e.g., isCapital desc, displayOrder asc, population desc, nameEn asc) or
implement a two-step fallback that first searches for city where isCapital =
true (or displayOrder is set) for the fallbackCountry and only falls back to
alphabetical if none found, ensuring the most relevant city (e.g., Doha) is
chosen as the default.
In `@apps/web/src/components/country-picker.tsx`:
- Around line 59-63: The countries query currently only destructures data from
useQuery (const { data: countries = [] } = useQuery(...)) so failures leave the
component stuck on "Loading..."; update the useQuery call to also destructure
error and isLoading (e.g., const { data: countries = [], error, isLoading,
refetch } = useQuery(...)) and then add UI branches in country-picker.tsx (where
the dropdowns are rendered) to show a user-friendly error message and a retry
control (or fallback list) when error is present, and keep the existing loading
state for isLoading; ensure you reference the same queryKey ['public-countries']
and the queryFn so behavior and caching remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f95901d7-fdd1-4937-8334-560df3a61d45
📒 Files selected for processing (8)
apps/api/src/admin/admin.controller.tsapps/api/src/admin/admin.service.tsapps/api/src/admin/dto/email-suppressions-query.dto.tsapps/api/src/geo/geo.service.tsapps/web/src/app/admin/email-suppressions/page.tsxapps/web/src/components/country-picker.tsxapps/web/src/components/navbar.tsxapps/web/src/context/geo-context.tsx
…t test CountryPicker imported @/lib/localize, which transitively imports @/lib/i18n. i18n.ts runs i18next.use(initReactI18next) at module load. The navbar unit test mocks react-i18next without exporting initReactI18next, so the mocked value is undefined and i18n.use() crashes the whole suite at import time. Replace localized() with a 3-line pickName() helper that takes the current language as an argument (read from useTranslation().i18n.language inside the component). No more module-load init in the navbar's graph.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
Per user feedback today:
Changes
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Improvements