refactor merchant_id and shop_identifier#516
refactor merchant_id and shop_identifier#516sharifajahanshaik wants to merge 1 commit intojuspay:releasefrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughA comprehensive refactoring that renames merchant-scoped identifiers to reseller-scoped identifiers throughout the Breeze Buddy module, including function names, parameter names, database columns, and related schema fields. Introduces backward-compatibility fields in data structures. Updates RBAC logic, analytics, templates, and API handlers to reflect reseller-centric terminology. Adds database schema migration to support both old and new identifier columns. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
sharifajahanshaik
force-pushed
the
refactor-merchantid-and-shopidentifier
branch
from
637a052 to
498e751
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
app/core/config/static.py (1)
516-521:⚠️ Potential issue | 🟠 MajorAvoid shipping localhost in default CORS for production.
Including
http://localhost:5173in the default list means it’s allowed wheneverCORS_ALLOWED_ORIGINSisn’t explicitly set, including production. This is a permissive default that weakens CORS guarantees. Prefer gating localhost by environment or an explicit opt‑in flag.🔧 Suggested guard
CORS_ALLOWED_ORIGINS = [ origin.strip() for origin in os.environ.get( "CORS_ALLOWED_ORIGINS", - "https://buddy.breezelabs.app,https://portal.breeze.in,https://portal.breezesdk.store,http://localhost:5173", + "https://buddy.breezelabs.app,https://portal.breeze.in,https://portal.breezesdk.store", ).split(",") if origin.strip() ] + +if ENVIRONMENT != "production": + CORS_ALLOWED_ORIGINS.append("http://localhost:5173")app/ai/voice/agents/breeze_buddy/tts/__init__.py (1)
131-212:⚠️ Potential issue | 🟠 MajorAvoid clobbering caller input; this bypasses config-based routing.
Line 147 forces
voice_name = "sara", so"rhea"/"mira"and theBB_TTS_SERVICEpath never run. This is a behavior-breaking change and makes thevoice_nameargument ineffective. Remove the override (or gate it behind an explicit default flag) and set the default in config instead.💡 Suggested fix
- voice_name = "sara" + # Respect caller-provided voice_name; fall back to BB_TTS_SERVICE when None.app/database/queries/breeze_buddy/users.py (1)
25-52:⚠️ Potential issue | 🔴 CriticalField name mismatch:
merchant_identifiersin code does not match schema.The UserInDB schema at
app/schemas/breeze_buddy/auth.py:147defines the field asmerchant_identifier(singular), but this code passesmerchant_identifiers(plural) to the constructor on line 48. This will raise a TypeError at runtime. Update line 25 and lines 48-52 to usemerchant_identifierinstead ofmerchant_identifiers. Thereseller_idsfield is correctly named in both the code and schema.app/api/routers/breeze_buddy/leads/rbac.py (1)
107-109:⚠️ Potential issue | 🟡 MinorRemove extraneous
fprefix from f-string without placeholders.Same issue as above - the f-string has no placeholders.
Proposed fix
raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail=f"Lead not found" + status_code=status.HTTP_404_NOT_FOUND, detail="Lead not found" )app/api/security/breeze_buddy/rbac_token.py (1)
99-114:⚠️ Potential issue | 🟠 MajorBreaking change: JWT payload keys renamed - existing tokens will lose RBAC assignments.
The token payload now uses
reseller_idsandmerchant_identifierskeys instead of the previousmerchant_idsandshop_identifiers. When existing tokens are verified:
payload.get("reseller_ids", [])returns[](empty) for old tokenspayload.get("merchant_identifiers", [])returns[](empty) for old tokensThis means users with valid but pre-migration tokens will effectively have no merchant/shop access until they re-authenticate.
Consider one of these mitigations:
- Add backward-compatible fallback to read old keys if new keys are missing
- Coordinate deployment with token invalidation/forced re-authentication
- If tokens have short expiry, ensure deployment timing minimizes user impact
Proposed backward-compatible fix
# Extract Breeze Buddy RBAC data user_info = UserInfo( id=payload.get("sub"), username=payload.get("username"), role=UserRole(payload.get("role")), email=payload.get("email"), - reseller_ids=payload.get("reseller_ids", []), - merchant_identifiers=payload.get("merchant_identifiers", []), + reseller_ids=payload.get("reseller_ids") or payload.get("merchant_ids", []), + merchant_identifiers=payload.get("merchant_identifiers") or payload.get("shop_identifiers", []), permissions=payload.get("permissions", []), )app/api/routers/breeze_buddy/templates/handlers.py (1)
34-144:⚠️ Potential issue | 🔴 CriticalFix
create_templatecall to usereseller_idkeyword.
create_templateexpectsreseller_id; passingresellerwill raiseTypeErrorand break template creation.🛠️ Proposed fix
template = await create_template( template_id=str(uuid4()), - reseller=template_data.reseller, + reseller_id=template_data.reseller, identifier=template_data.identifier, name=template_data.template_name, flow=flow,app/database/queries/breeze_buddy/template.py (1)
152-179:⚠️ Potential issue | 🔴 CriticalSelect
reseller_idinstead ofmerchant_id.
The query still selectsmerchant_id, which will break once the schema is renamed and will not match decoder expectations.🛠️ Proposed fix
- SELECT id, merchant_id, merchant_identifier, name, flow, expected_payload_schema, expected_callback_response_schema, configurations, secrets, outbound_number_id, is_active, created_at, updated_at + SELECT id, reseller_id, merchant_identifier, name, flow, expected_payload_schema, expected_callback_response_schema, configurations, secrets, outbound_number_id, is_active, created_at, updated_atapp/database/accessor/breeze_buddy/template.py (1)
82-145:⚠️ Potential issue | 🔴 CriticalFix undefined
resellervariable in create_template.
reselleris not defined; this raisesNameErrorand thereseller_idargument is ignored.🛠️ Proposed fix
query, values = create_template_query( template_id, - reseller, + reseller_id, identifier, name, flow_json,app/api/security/breeze_buddy/authorization.py (1)
165-192:⚠️ Potential issue | 🔴 CriticalUse
merchant_keyconsistently to avoidNameError.
The list comprehension referencesshop_key, which is undefined.🛠️ Proposed fix
- shop_key: Key name for shop identifier in each dict (default: "merchant_identifier") + merchant_key: Key name for shop identifier in each dict (default: "merchant_identifier") @@ - if shop_key in item and item[shop_key] in accessible_shops + if merchant_key in item and item[merchant_key] in accessible_shops
🤖 Fix all issues with AI agents
In `@app/api/routers/breeze_buddy/leads/__init__.py`:
- Line 85: The logger.info call currently logs the entire PushLeadRequest object
(logger.info(f"Received push lead request: {req}")), which may expose PII;
update the logging to only include a safe subset or derived metadata (e.g.,
request id, source, timestamp, or status) or redact sensitive fields (name,
phone, email, address) before logging—use PushLeadRequest attributes or a helper
like to_safe_dict()/redact_sensitive_fields(req) to construct the sanitized log
entry and replace the existing logger.info call.
In `@app/api/routers/breeze_buddy/leads/handlers.py`:
- Around line 120-122: The log message uses the wrong variable: change the
logger.info that currently interpolates template.expected_payload_schema to
instead include the reseller identifier (req.reseller) so it reads that a
template was found for the reseller; locate the logger.info call near the
template-not-found raise (references template, req, and logger) and update the
interpolated value accordingly to log req.reseller.
In `@app/api/routers/breeze_buddy/leads/rbac.py`:
- Around line 93-95: The HTTPException raise uses an unnecessary f-string;
update the raise HTTPException call that sets
status_code=status.HTTP_404_NOT_FOUND and detail=f"Lead not found" to use a
plain string (detail="Lead not found") by removing the leading "f" so the
literal has no unused f-string prefix (refer to the HTTPException invocation and
status.HTTP_404_NOT_FOUND usage).
In `@app/api/routers/breeze_buddy/templates/__init__.py`:
- Line 130: Update the example query value for the reseller_id in the templates
route documentation to use a reseller-style identifier rather than a shop-style
one: change the example for the GET /templates/list?reseller_id=shop_123 line in
__init__.py to use a reseller-like value (e.g., reseller_123 or reseller_abc) so
it accurately reflects the expected parameter format for the templates/list
handler.
In `@app/api/security/breeze_buddy/authorization.py`:
- Around line 270-339: In apply_merchant_shop_filter: merchant_filter is never
initialized and the code uses the wrong name accessible_merchants and bare list
expressions that do nothing; initialize merchant_filter = None near the top,
replace accessible_merchants with accessible_resellers, and assign
merchant_filter = [requested_reseller_id] when a specific reseller is allowed
(and merchant_filter = accessible_resellers when the user has restricted
resellers), leaving merchant_filter as None for wildcard access; ensure returned
tuple uses (merchant_filter, shop_filter).
In `@app/database/accessor/breeze_buddy/lead_call_tracker.py`:
- Line 78: The log message uses "merchant ID" while logging the variable
reseller_id; update the logger.info call in the function/method that creates the
lead call tracker (where reseller_id is passed/used, e.g.,
create_lead_call_tracker or LeadCallTracker initializer) to reference "reseller
ID" (or interpolate the parameter name) so the message matches the variable
being logged and avoids confusion.
In `@app/schemas/breeze_buddy/auth.py`:
- Around line 110-112: Rename the singular field merchant_identifier to the
plural merchant_identifiers across the user models to match AuthTokenData and
UserInfo: update UserCreate, UserUpdate, UserInDB, and User to use
merchant_identifiers: List[str] = Field(default_factory=list) (or equivalent
Pydantic list default) and adjust any code that maps JWT token data to these
models (mapping logic that consumes AuthTokenData or UserInfo) so it
reads/writes merchant_identifiers instead of merchant_identifier to avoid silent
data loss.
In `@docs/BREEZE_BUDDY_ARCHITECTURE.md`:
- Around line 110-117: Documentation snippets reference the old field name
shop_identifier but the code uses merchant_identifier; update all occurrences in
the docs (for example in the create_lead_call_tracker call and any other
snippets that set shop_identifier) to use merchant_identifier, ensuring
parameter names and any explanatory text (e.g., comments like "# Scheduled
time") match the current code API and naming (search for
create_lead_call_tracker and replace shop_identifier -> merchant_identifier
across the referenced examples).
🧹 Nitpick comments (10)
app/api/routers/breeze_buddy/merchants/__init__.py (1)
30-30: Align terminology in docstring with merchant_identifier.Line 30 still references “shop,” which now conflicts with the new merchant_identifier naming and could reintroduce the old concept. Consider updating it to “merchant” (or “merchant location”) for consistency.
As per coding guidelines: “Each shop_identifier represents a distinct merchant… shop_identifier uniquely identifies individual merchant locations/shops.”app/ai/voice/agents/breeze_buddy/types/models.py (1)
29-32: Add backward-compat + clarify semantics formerchant_identifier.If existing producers still send
shop_identifier, this rename will break deserialization. Also, the new name can be misread as a parent/reseller identifier; consider adding a description to preserve the “shop/location” meaning.💡 Example shim + schema clarity (adjust for Pydantic v1/v2)
-from pydantic import BaseModel +from pydantic import BaseModel, Field ... class LeadData(BaseModel): customer_mobile_number: str - merchant_identifier: Optional[str] = None + merchant_identifier: Optional[str] = Field( + default=None, + alias="shop_identifier", + description="Unique identifier for a specific merchant location (formerly shop_identifier).", + )Based on learnings: “shop_identifier uniquely identifies individual merchant locations/shops; merchant_id is a parent/group identifier.”
app/database/migrations/rename_columns_names.sql (2)
1-1: Filename mismatch in comment.The filepath comment references
009_rename_merchant_id_to_reseller_id.sql, but the actual filename isrename_columns_names.sql. Update the comment to match the actual filename or rename the file to follow the numbered migration convention if your migration framework requires it.
14-122: Consider wrapping migration in a transaction for atomicity.This migration performs multiple ALTER TABLE operations across 5 tables. If any statement fails midway (e.g., due to a lock timeout or missing column), the database will be left in a partially migrated state. Consider wrapping the entire migration in a transaction block to ensure atomicity.
Additionally, consider providing a rollback migration script for reverting these changes if needed.
🛡️ Proposed transaction wrapper
+BEGIN; + -- ===================================================== -- 1. Rename merchant_id to reseller_id in call_execution_config -- ===================================================== ... -- (all existing statements) ... + +COMMIT;app/database/accessor/breeze_buddy/call_execution_config.py (1)
149-149: Consider removing verbose debug log.Logging the entire
decoded_resultat INFO level could produce excessive log output and potentially expose configuration details. Consider removing this line or reducing log level to DEBUG.🔧 Proposed fix
- logger.info(f"decoded_result: {decoded_result}") + logger.debug(f"decoded_result: {decoded_result}")app/api/routers/breeze_buddy/merchants/handlers.py (1)
50-50: Consider using explicit conversion flag.Static analysis suggests using
{e!s}instead of{str(e)}for cleaner f-string formatting.✨ Suggested fix
- detail=f"Error listing resellers: {str(e)}", + detail=f"Error listing resellers: {e!s}",app/api/routers/breeze_buddy/auth/handlers.py (1)
93-93: Minor terminology inconsistency in log message.The log message uses
shops: {user.merchant_identifiers}but the field is now calledmerchant_identifiers. Consider updating the log label for consistency:- f"(role: {user.role}, resellers: {user.reseller_ids}, shops: {user.merchant_identifiers})" + f"(role: {user.role}, resellers: {user.reseller_ids}, merchants: {user.merchant_identifiers})"app/api/routers/breeze_buddy/leads/__init__.py (1)
58-63: Documentation example may need clarification.The permissions correctly describe "Reseller" role. However, the example shows
"reseller": "shop_123"which uses a shop-like identifier for the reseller field. Consider updating the example to use a more representative reseller identifier:- "reseller": "shop_123", + "reseller": "reseller_abc",app/api/routers/breeze_buddy/configurations/rbac.py (2)
3-3: Consider consistent terminology in module docstring.The docstring mentions "reseller + shop" but the codebase is moving to "reseller + merchant" terminology. Consider updating for consistency:
-Handles reseller + shop access control based on JWT token. +Handles reseller + merchant access control based on JWT token.
50-63: Minor terminology inconsistency in comments and error messages.The comment on line 50 says "Check merchant access" but the log message (line 58) and error detail (line 62) reference "shop". Consider aligning terminology:
📝 Proposed fix for consistency
- # Check merchant access (only if merchant_identifier is provided) + # Check merchant/shop access (only if merchant_identifier is provided) if ( merchant_identifier and merchant_identifier not in current_user.merchant_identifiers and "*" not in current_user.merchant_identifiers ): logger.warning( f"User {current_user.username} attempted to {operation} configuration " - f"for unauthorized shop: {merchant_identifier}" + f"for unauthorized merchant: {merchant_identifier}" ) raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, - detail=f"Access denied to shop {merchant_identifier}", + detail=f"Access denied to merchant {merchant_identifier}", )
| } | ||
| """ | ||
| # RBAC: Check permission to push leads for this merchant/shop | ||
| logger.info(f"Received push lead request: {req}") |
There was a problem hiding this comment.
Consider limiting logged request data.
Logging the entire PushLeadRequest object ({req}) may expose sensitive payload data (customer names, phone numbers, etc.) in logs. Consider logging only non-sensitive fields:
- logger.info(f"Received push lead request: {req}")
+ logger.info(f"Received push lead request for reseller={req.reseller}, template={req.template}, identifier={req.identifier}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.info(f"Received push lead request: {req}") | |
| logger.info(f"Received push lead request for reseller={req.reseller}, template={req.template}, identifier={req.identifier}") |
🤖 Prompt for AI Agents
In `@app/api/routers/breeze_buddy/leads/__init__.py` at line 85, The logger.info
call currently logs the entire PushLeadRequest object (logger.info(f"Received
push lead request: {req}")), which may expose PII; update the logging to only
include a safe subset or derived metadata (e.g., request id, source, timestamp,
or status) or redact sensitive fields (name, phone, email, address) before
logging—use PushLeadRequest attributes or a helper like
to_safe_dict()/redact_sensitive_fields(req) to construct the sanitized log entry
and replace the existing logger.info call.
| raise HTTPException( | ||
| status_code=status.HTTP_404_NOT_FOUND, detail=f"Lead not found" | ||
| ) |
There was a problem hiding this comment.
Remove extraneous f prefix from f-string without placeholders.
The f-string f"Lead not found" has no placeholders, so the f prefix is unnecessary.
Proposed fix
raise HTTPException(
- status_code=status.HTTP_404_NOT_FOUND, detail=f"Lead not found"
+ status_code=status.HTTP_404_NOT_FOUND, detail="Lead not found"
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| raise HTTPException( | |
| status_code=status.HTTP_404_NOT_FOUND, detail=f"Lead not found" | |
| ) | |
| raise HTTPException( | |
| status_code=status.HTTP_404_NOT_FOUND, detail="Lead not found" | |
| ) |
🧰 Tools
🪛 Ruff (0.14.14)
[error] 94-94: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In `@app/api/routers/breeze_buddy/leads/rbac.py` around lines 93 - 95, The
HTTPException raise uses an unnecessary f-string; update the raise HTTPException
call that sets status_code=status.HTTP_404_NOT_FOUND and detail=f"Lead not
found" to use a plain string (detail="Lead not found") by removing the leading
"f" so the literal has no unused f-string prefix (refer to the HTTPException
invocation and status.HTTP_404_NOT_FOUND usage).
399c27f to
a7c3969
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/schemas/breeze_buddy/analytics.py (1)
29-71:⚠️ Potential issue | 🟠 MajorAPI backward compatibility broken by removing old field names from
AnalyticsFilters.The schema no longer accepts
merchant_id,merchant_ids,shop_identifier, orshop_identifiers. When the router convertsAnalyticsFiltersto dict viamodel_dump(exclude_none=True)(line 68 in__init__.py), these old field names are not included. The backward-compatibility branches inbuild_analytics_where_clause(lines 124-165 inapp/database/queries/breeze_buddy/analytics.py) will never trigger through the standard API path—old clients sending these field names will have them silently dropped by Pydantic.Add a
model_validatorto remap legacy field names to new ones before schema validation completes, preserving API compatibility:Backward compatibility solution
from pydantic import model_validator class AnalyticsFilters(BaseModel): """Filters for analytics queries - all filters applied with AND logic""" # ... existing fields ... `@model_validator`(mode="before") `@classmethod` def remap_legacy_fields(cls, values): """Remap old field names to new ones for backward compatibility""" if isinstance(values, dict): # Remap merchant_id → reseller_id if "merchant_id" in values and not values.get("reseller_id"): values["reseller_id"] = values.pop("merchant_id") # Remap merchant_ids → reseller_ids if "merchant_ids" in values and not values.get("reseller_ids"): values["reseller_ids"] = values.pop("merchant_ids") # Remap shop_identifier → merchant_identifier if "shop_identifier" in values and not values.get("merchant_identifier"): values["merchant_identifier"] = values.pop("shop_identifier") # Remap shop_identifiers → merchant_identifiers if "shop_identifiers" in values and not values.get("merchant_identifiers"): values["merchant_identifiers"] = values.pop("shop_identifiers") return valuesThis validator transforms incoming requests before Pydantic validation, allowing the query builder's backward-compat branches to work.
🤖 Fix all issues with AI agents
In `@app/api/routers/breeze_buddy/analytics/handlers.py`:
- Around line 331-343: The handler is using the incoming group_by string to
extract the DB row value but the query aliases legacy group_by values to
different column names, causing None results; update the logic around group_by
(the block that sets group_value before results.append) to normalize legacy keys
to the actual DB column or to try both aliases: when group_by is "merchant_id"
or "reseller_id" prefer row.get("reseller_id") then row.get("merchant_id"); when
group_by is "shop_identifier" or "merchant_identifier" prefer
row.get("merchant_identifier") then row.get("shop_identifier"); for all other
group_by values keep the existing row.get(group_by) behavior so group_value is
never None due to aliasing.
In `@app/api/routers/breeze_buddy/deprecated/leads.py`:
- Around line 187-205: The error messages reference req.reseller which may be
falsy when you resolved the ID into reseller_id (reseller_id = req.reseller or
req.merchant); update all error/log responses that currently interpolate
req.reseller to use the resolved reseller_id instead (for example the 404
JSONResponse after get_template_by_reseller and the other error messages that
mention the reseller), keeping the rest of the message intact; ensure you still
use req.template and any other req fields as before and only replace
req.reseller with reseller_id in those f-strings or message constructions.
In `@app/api/routers/breeze_buddy/leads/__init__.py`:
- Around line 85-90: reseller_id is computed as `reseller_id = req.reseller or
req.merchant or ""`, which allows an empty string to bypass downstream logic
(admins pass RBAC) and create leads with no reseller; update the handler to
explicitly validate presence of a reseller identifier before calling
`validate_lead_access` by raising/returning a 4xx error when `reseller_id` is
falsy, or require `req.identifier`/other fallback as appropriate; ensure the
check is performed immediately prior to calling
`validate_lead_access(current_user, reseller_id, req.identifier, operation="push
leads for")` so that no empty `reseller_id` can reach lead creation.
In `@app/api/routers/breeze_buddy/leads/handlers.py`:
- Around line 113-114: The code currently sets reseller_id = req.reseller or
req.merchant or "" which allows an empty string to be used in DB queries; update
the PushLeadRequest handling to validate that at least one of req.reseller or
req.merchant is present before building reseller_id (e.g., check if not
(req.reseller or req.merchant) and immediately return/raise a 400 Bad Request
with a clear message), then set reseller_id = req.reseller or req.merchant;
ensure this validation occurs prior to any use of reseller_id in queries
(references: variable reseller_id and the request object PushLeadRequest/req
used in the handler).
In `@app/api/routers/breeze_buddy/merchants/__init__.py`:
- Line 30: Update the stale "shop" terminology on line 30 (and any nearby
occurrences in app/api/routers/breeze_buddy/merchants/__init__.py) to use
"merchant" or "merchant_identifier" to match the rename; specifically replace
phrases like "every shop" with "every merchant" and ensure any references to
shop_identifier are merchant_identifier so variable names and comments stay
consistent with the rest of the PR.
In `@app/api/security/breeze_buddy/authorization.py`:
- Around line 155-158: The HTTPException raised uses a mismatched message
("merchants") for the shop-level check; update the error detail in the branch
that checks merchant_identifiers to reference shops (e.g., "shop" or "shops")
instead of "merchants" so it matches the identifiers being validated (look for
the HTTPException with detail f"Access denied to merchants:
{unauthorized_shops}" and change it to reference shops/unauthorized_shops
accordingly).
In `@app/api/security/breeze_buddy/rbac_token.py`:
- Around line 116-117: The code that constructs RBAC fields from the JWT payload
(where reseller_ids and merchant_identifiers are read) must fall back to the old
key names so old tokens don’t lose permissions: update the places that call
payload.get("reseller_ids", []) and payload.get("merchant_identifiers", []) to
try the legacy keys first (e.g. payload.get("reseller_ids") or
payload.get("merchant_ids") or [] and payload.get("merchant_identifiers") or
payload.get("shop_identifiers") or []) so both new and old tokens are supported;
locate this logic in rbac_token.py where reseller_ids and merchant_identifiers
are assigned and change the get calls accordingly.
In `@app/database/accessor/breeze_buddy/analytics.py`:
- Around line 73-80: The current or-chain for computing group_value can
substitute unrelated columns when the requested group_by key exists but is
falsy; change to first detect which column to read by existence rather than
truthiness: if group_by in data then set group_value = data[group_by] (even if
None/empty), else iterate the fallback column names
['shop_identifier','merchant_identifier','merchant_id','reseller_id'] and pick
the first key that exists in data and use its value; remove the boolean or-chain
and use membership checks on the data dict (referencing the variables group_by,
group_value, and data in analytics.py) so schema-level presence drives fallback
instead of falsy values.
In `@app/database/decoder/breeze_buddy/call_execution_config.py`:
- Around line 23-26: The current fallback uses truthy-or chains (e.g., in
reseller_id_value and merchant_identifier_value) which treat empty strings and
other falsy values as missing; change these to explicit None-aware fallbacks so
a stored empty string or 0/False is preserved. Update reseller_id_value and
merchant_identifier_value (and the similar block around lines 62-65) to check
"is not None" for each candidate or introduce a helper like _first_non_none(...)
that returns the first value that is not None (with default ""), and use that
helper to select between row.get("reseller_id") and row.get("merchant_id") and
between row.get("merchant_identifier") and row.get("shop_identifier").
In `@app/database/decoder/breeze_buddy/lead_call_tracker.py`:
- Around line 28-30: Multiple SELECT/UPDATE-returning queries feeding the
decoder need COALESCE aliases so the decoder's accessors (row["reseller_id"],
row["merchant_identifier"]) never get NULL for older rows; update each query
that returns these columns—notably update_lead_call_details_query,
update_lead_call_completion_details_query, acquire_lock_on_lead_by_id_query (and
any other queries returning the full lead row)—to replace/respecify the columns
with COALESCE(reseller_id, merchant_id) AS reseller_id and
COALESCE(merchant_identifier, shop_identifier) AS merchant_identifier so the
decoder receives consistent fallback values.
In `@app/database/migrations/add_reseller_id_and_merchant_identifier_columns.sql`:
- Line 1: The top-of-file filepath comment references a different migration name
("009_add_reseller_id_and_merchant_identifier.sql") which conflicts with the
actual filename (add_reseller_id_and_merchant_identifier_columns.sql); update
the comment to match the real filename and migration numbering or remove the
comment entirely so migration ordering is not ambiguous—edit the comment string
in this file to either "009_add_reseller_id_and_merchant_identifier_columns.sql"
(if numbering 009 is correct) or delete the filepath comment.
- Around line 14-143: Wrap the whole migration in an explicit transaction
(BEGIN; ... COMMIT;) so all ALTER/UPDATE/CREATE INDEX steps for
call_execution_config, lead_call_tracker, outbound_number, template and users
run atomically; then add a mechanism to keep old and new columns in sync for
future writes — e.g., create per-table triggers (or generated columns or update
the application to write both) that mirror merchant_id <-> reseller_id and
shop_identifier <-> merchant_identifier (and merchant_ids <-> reseller_ids,
shop_identifiers <-> merchant_identifiers) on INSERT and UPDATE for the tables
call_execution_config, lead_call_tracker, outbound_number, template and users.
- Around line 24-25: Add composite indexes matching the old multi-column
patterns for the affected tables: create IF NOT EXISTS indexes on (reseller_id,
merchant_identifier) for outbound_number and lead_call_tracker, on (reseller_id,
merchant_identifier, name) for template, and on (reseller_id, workflow,
merchant_identifier) for call_execution_config; place the CREATE INDEX IF NOT
EXISTS statements near the existing single-column index statements (e.g.,
alongside idx_call_execution_config_reseller_id,
idx_lead_call_tracker_reseller_id, idx_outbound_number_reseller_id,
idx_template_reseller_id) and choose unique, descriptive index names (e.g.,
idx_outbound_number_reseller_merchant_identifier,
idx_template_reseller_merchant_identifier_name,
idx_call_execution_config_reseller_workflow_merchant_identifier,
idx_lead_call_tracker_reseller_merchant_identifier). Ensure the SQL uses the
exact column names reseller_id, merchant_identifier, workflow, and name as in
the diff and includes IF NOT EXISTS.
In `@app/database/queries/breeze_buddy/call_execution_config.py`:
- Around line 288-306: The function name get_all_resellers_query is misleading
because it returns merchant_identifier/shop_identifier (individual merchant
locations); rename the function to get_all_merchants_query (or
get_all_merchant_identifiers_query) and update its docstring accordingly, then
update all references/calls to get_all_resellers_query throughout the codebase
(including tests and imports) to the new name; keep the SQL query, COALESCE
usage and return signature unchanged (query, []), and ensure symbols like
CALL_EXECUTION_CONFIG_TABLE remain referenced as before.
In `@app/database/queries/breeze_buddy/users.py`:
- Around line 53-62: The code calls json.loads(...) unconditionally when the DB
value isn't a list, which will raise TypeError for NULL (None) and causes valid
rows to be swallowed; update the handling for reseller_ids and
merchant_identifiers to treat None as an empty list: fetch the raw value
(row["reseller_ids"] / row["merchant_identifiers"]), if it's already a list keep
it, else if the raw is falsy/None use [] as the default, otherwise call
json.loads(raw) to parse the JSON string; apply the same pattern to both
reseller_ids and merchant_identifiers.
In `@app/schemas/breeze_buddy/merchants.py`:
- Around line 11-16: The field name resellers in
app/schemas/breeze_buddy/merchants.py is misleading because get_all_resellers in
call_execution_config.py actually populates merchant_identifier values; rename
resellers to merchant_identifiers (and keep merchants as an alias for backward
compatibility) and update the type annotation and any serialisation/usage
references to use merchant_identifiers instead of resellers; also search for and
update usages of the old resellers attribute (including schema initialisation
and tests) to the new merchant_identifiers name or access merchants where
backward compatibility is required.
In `@app/schemas/breeze_buddy/template.py`:
- Around line 16-18: The schema changed `reseller_id` from required to Optional
(reseller_id: Optional[str] = None) which may break consumers expecting a
non-null value: update the template-listing/serialization flow to either (A)
ensure the database/query layer always populates a non-empty string for
reseller_id before constructing the response object (e.g., in the function that
builds Template responses or in the ORM mapping that returns
id/reseller_id/merchant_identifier), or (B) make callers tolerant of None by
handling reseller_id == None where templates are consumed (e.g., in
list_templates, Template serializer or response formatter) and providing a safe
fallback (empty string or explicit "unknown") so consumers never assume a
non-null value. Ensure changes touch the code that instantiates the schema (the
Template response builder / list_templates handler) and include tests that cover
reseller_id == None.
🧹 Nitpick comments (31)
app/core/config/static.py (1)
524-531:localhostorigin in default CORS config could be permissive in production.If the
CORS_ALLOWED_ORIGINSenvironment variable is not explicitly set in production,http://localhost:5173will be allowed as a CORS origin. Consider gating this on theENVIRONMENTvariable (line 18) or ensuring deployment always setsCORS_ALLOWED_ORIGINS.💡 Suggested conditional approach
+_default_cors_origins = "https://buddy.breezelabs.app,https://portal.breeze.in,https://portal.breezesdk.store" +if ENVIRONMENT != "production": + _default_cors_origins += ",http://localhost:5173" + CORS_ALLOWED_ORIGINS = [ origin.strip() for origin in os.environ.get( "CORS_ALLOWED_ORIGINS", - "https://buddy.breezelabs.app,https://portal.breeze.in,https://portal.breezesdk.store,http://localhost:5173", + _default_cors_origins, ).split(",") if origin.strip() ]app/database/decoder/breeze_buddy/call_execution_config.py (2)
21-49: DRY:decode_call_execution_config_listduplicates the record-decoding logic fromdecode_call_execution_config.The
CallExecutionConfigconstruction (including the fallback-chain logic) is copy-pasted between both functions. If a field is added or the fallback logic changes, both sites must be updated in lockstep.♻️ Reuse the single-record decoder
def decode_call_execution_config_list( result: List[asyncpg.Record], ) -> List[CallExecutionConfig]: """ Decode multiple call execution config records from database result using Pydantic models. """ if not result: return [] - configs = [] - for row in result: - reseller_id_value = row.get("reseller_id") or row.get("merchant_id") or "" - merchant_identifier_value = ( - row.get("merchant_identifier") or row.get("shop_identifier") or "" - ) - - configs.append( - CallExecutionConfig( - id=row["id"], - initial_offset=row["initial_offset"], - retry_offset=row["retry_offset"], - call_start_time=row["call_start_time"], - call_end_time=row["call_end_time"], - max_retry=row["max_retry"], - calling_provider=CallProvider(row["calling_provider"]), - reseller_id=reseller_id_value, - merchant_id=reseller_id_value, # Backward compatibility - template=row["template"], - merchant_identifier=merchant_identifier_value, - shop_identifier=merchant_identifier_value, # Backward compatibility - enable_international_call=row["enable_international_call"], - enable_calling=row["enable_calling"], - created_at=row["created_at"], - updated_at=row["updated_at"], - ) - ) - - return configs + return [ + config + for row in result + if (config := decode_call_execution_config([row])) is not None + ]
58-58: Redundant guard:not result or len(result) == 0.
not resultisTruefor bothNoneand[], makinglen(result) == 0unreachable. Minor nit but worth simplifying.✏️ Simplify
- if not result or len(result) == 0: + if not result:app/database/queries/breeze_buddy/users.py (1)
25-37: Minor: inconsistent SQL indentation inside the query string.The column list is indented at one level, but
FROMandWHEREare flush-left within the triple-quoted string. This is purely cosmetic but harms readability.✏️ Suggested formatting fix
query = """ - SELECT - id, - username, - password_hash, - role, - email, - COALESCE(reseller_ids, merchant_ids) AS reseller_ids, - COALESCE(merchant_identifiers, shop_identifiers) AS merchant_identifiers, - is_active, - created_at, - updated_at -FROM users -WHERE username = $1 + SELECT + id, + username, + password_hash, + role, + email, + COALESCE(reseller_ids, merchant_ids) AS reseller_ids, + COALESCE(merchant_identifiers, shop_identifiers) AS merchant_identifiers, + is_active, + created_at, + updated_at + FROM users + WHERE username = $1 """app/database/accessor/breeze_buddy/analytics.py (1)
90-94: Inconsistent dict access:data["average_duration"]vsdata.get("average_duration").Line 91 uses direct key access (
data["average_duration"]) while line 92 uses.get(). The ternary evaluates the condition first, so this is safe at runtime, but it's inconsistent with the rest of the refactored block. Use.get()in both places for consistency.Suggested fix
"average_duration": ( - round(float(data["average_duration"]), 2) - if data.get("average_duration") + round(float(data.get("average_duration")), 2) + if data.get("average_duration") else None ),app/database/migrations/add_reseller_id_and_merchant_identifier_columns.sql (1)
46-48: Consider batched updates for large tables.
lead_call_trackeris likely a high-volume table. A singleUPDATE ... WHERE reseller_id IS NULLwill lock all matching rows in one transaction, which can cause extended lock contention, replication lag, and statement timeouts in production. Consider batching the update (e.g., updating in chunks of 10k–50k rows) or running the backfill as a separate, throttled operation outside the DDL migration.This applies to all the
UPDATEstatements in this migration, but is most critical for high-volume tables likelead_call_tracker.app/database/queries/breeze_buddy/analytics.py (3)
118-167: COALESCE in WHERE clauses will prevent index scans onreseller_id,merchant_id,merchant_identifier, andshop_identifier.Wrapping columns in
COALESCE(lct.reseller_id, lct.merchant_id)prevents PostgreSQL from using B-tree indexes on either column individually. For largelead_call_trackertables this could cause full table scans. Consider adding expression indexes if query volume is significant:CREATE INDEX idx_lct_reseller_coalesce ON lead_call_tracker (COALESCE(reseller_id, merchant_id)); CREATE INDEX idx_lct_merchant_ident_coalesce ON lead_call_tracker (COALESCE(merchant_identifier, shop_identifier));Alternatively, once the migration is complete and all rows have
reseller_id/merchant_identifierpopulated, the COALESCE wrappers can be dropped in favor of direct column filters.
234-240: Mapping logic is duplicated acrossget_analytics_summary_queryandget_analytics_lead_based_query.The group_by → column mapping (lines 234–240 and 547–558) is repeated. Extracting a small helper (e.g.,
resolve_group_by(group_by) -> (group_by_select, group_by_column)) would reduce duplication and keep both paths in sync.
119-142: Consider usingfilters.get(...)instead of"key" in filters and filters["key"].Static analysis (Ruff RUF019) flags these as unnecessary key checks. Since
filters.get("reseller_id")returnsNonefor missing keys (which is falsy), it's functionally equivalent and more concise. This applies to the new filter blocks at lines 119, 124, 131, 137, 145, 150, 157, 162.However, since the existing (unchanged) filter blocks in this file follow the same pattern, this is purely a style nit and can be deferred to a broader cleanup.
app/schemas/breeze_buddy/analytics.py (1)
127-163:model_dumpoverride is consistent with the pattern used in other schemas — looks good.The backward-compat
model_dumpoverride matches the approach intemplate.pyandmerchants.py. The logic correctly populatesmerchant_idfromreseller_idandshop_identifierfrommerchant_identifierwhen they're missing.One minor note:
reseller_id: str(required, non-optional) means the handler must always provide a value. The handler defaults to""when both DB fields are null (line 201 in handlers.py). An empty string will serialize in the API response, which may confuse consumers expecting either a real ID ornull. Consider making itOptional[str] = Noneif null is a valid state.app/api/routers/breeze_buddy/analytics/handlers.py (1)
200-215: Backward-compat fields set both at construction and viamodel_dumpoverride — minor redundancy.Lines 213–215 explicitly set
merchant_id=reseller_idandshop_identifier=merchant_identifier, butCallDetailResult.model_dump()also populates these when they'reNone. Since you're setting them explicitly here, themodel_dumpoverride becomes a no-op for these fields. This is harmless but worth noting — either approach alone would suffice.app/schemas/breeze_buddy/merchants.py (1)
19-24: Inconsistent falsiness check vs.is Noneused elsewhere.Other
model_dumpoverrides in this PR (e.g.,analytics.py,template.py) useif data.get("field") is Nonefor backward-compat population. Here,if not data.get("merchants")also triggers on an empty list[]. This is likely fine for this use case but worth noting the inconsistency with the pattern used elsewhere.app/api/routers/breeze_buddy/merchants/handlers.py (1)
55-55: Nit: redundantstr()in f-string (flagged by Ruff RUF010).Suggested fix
- detail=f"Error listing resellers: {str(e)}", + detail=f"Error listing resellers: {e}",app/database/queries/breeze_buddy/outbound_number.py (1)
72-77:SELECT *combined withCOALESCE(...) AS reseller_idproduces duplicate column names.
SELECT *, COALESCE(reseller_id, merchant_id) AS reseller_idyields two columns namedreseller_idin the result set. Withasyncpg, accessingrow["reseller_id"]returns the last column with that name (the COALESCE alias), which is the desired behavior here. This works but is fragile — if asyncpg or PostgreSQL behavior changes, or if the result is converted to a dict, it could break. A more robust approach would be to either enumerate columns explicitly or exclude the raw columns.app/database/queries/breeze_buddy/lead_call_tracker.py (1)
179-187:SELECT *combined withCOALESCE ... AS reseller_idproduces duplicate column names.The
*expansion already includes the rawreseller_idandmerchant_identifiercolumns. The appended COALESCE aliases shadow them, which works with asyncpg (last column wins by name), but is fragile and driver-dependent. Consider explicitly listing only the columns you need, or using different alias names and mapping in the decoder.app/api/security/breeze_buddy/rbac_token.py (1)
121-124: Logging potentially sensitive RBAC data (reseller_ids, merchant_identifiers).The log line emits all reseller IDs and merchant identifiers on every token verification. Consider whether this is appropriate for production log volume and data sensitivity.
app/database/accessor/breeze_buddy/call_execution_config.py (2)
149-149: Debug log dumps entire decoded config list — consider removing or reducing verbosity.
logger.info(f"decoded_result: {decoded_result}")serializes all config objects on every call. This is noisy in production and may expose sensitive configuration data.Suggested fix
- logger.info(f"decoded_result: {decoded_result}")
305-332:get_all_resellersis misleadingly named — it returns merchant identifiers (individual shops), not resellers.The underlying query returns
COALESCE(merchant_identifier, shop_identifier), which identifies individual merchant locations. Per the domain model,merchant_identifier(formerlyshop_identifier) represents distinct merchants, whilereseller_id(formerlymerchant_id) is the parent/group identifier. This function should be named something likeget_all_merchant_identifiersto avoid confusion.Based on learnings: "In Breeze Buddy, treat shop_identifier as identifying individual merchant locations, while merchant_id acts as a parent/group identifier. Therefore, functions that retrieve shop_identifiers should be named as 'merchants' functions."
app/api/security/breeze_buddy/authorization.py (2)
14-28: Function nameget_accessible_merchantsno longer matches its semantics.After the refactor, this function accepts
reseller_idsand returns reseller-level access, yet the name still says "merchants." Consider renaming toget_accessible_resellersfor clarity, sinceget_accessible_shops(Line 31) already handles the merchant_identifiers tier.
195-218: Confusing naming betweenhas_wildcard_accessandhas_wildcard_merchant_access.
has_wildcard_accesschecksmerchant_identifiers(shop-level), whilehas_wildcard_merchant_accesschecksreseller_ids(reseller-level). The names suggest the opposite hierarchy. Consider renaming tohas_wildcard_shop_access/has_wildcard_reseller_access(or similar) to reduce confusion for callers.app/ai/voice/agents/breeze_buddy/agent/flow.py (1)
37-42: Redundantif lead else Noneguard on Line 40.
leadis already dereferenced without a guard on Line 38 (lead.reseller_id), so ifleadwereNone, anAttributeErrorwould be raised before reaching Line 40. The conditional is dead code and may mislead readers into thinkingleadcan beNonehere.Proposed simplification
template, template_vars = await flow_loader.load_template( reseller_id=lead.reseller_id, template=lead.template, - merchant_identifier=lead.merchant_identifier if lead else None, + merchant_identifier=lead.merchant_identifier, call_payload=lead.payload, )app/api/routers/breeze_buddy/configurations/rbac.py (1)
86-101: Mixed terminology:has_shop_accesschecksmerchant_identifiers.The variable
has_shop_access(line 94) still uses "shop" terminology while now checking againstcurrent_user.merchant_identifiersandconfig.merchant_identifier. Similarly, the comment on line 93 says "Check shop access". Consider renaming for consistency with the broader refactor.🔧 Suggested rename
- # Check shop access - has_shop_access = ( + # Check merchant access + has_merchant_access = ( "*" in current_user.merchant_identifiers or config.merchant_identifier in current_user.merchant_identifiers ) - if has_reseller_access and has_shop_access: + if has_reseller_access and has_merchant_access: filtered.append(config)app/api/routers/breeze_buddy/auth/handlers.py (1)
91-94: Log label mismatch: "shops" label formerchant_identifiers.Line 93 logs
shops: {user.merchant_identifiers}. Since the field is now calledmerchant_identifiers, the label should reflect this to avoid confusion when debugging.🔧 Proposed fix
logger.info( f"Successful login for database user: {user.username} " - f"(role: {user.role}, resellers: {user.reseller_ids}, shops: {user.merchant_identifiers})" + f"(role: {user.role}, resellers: {user.reseller_ids}, merchant_identifiers: {user.merchant_identifiers})" )app/api/routers/breeze_buddy/demo/handlers.py (1)
29-34: Remove unused constantsDEMO_MERCHANT_IDandDEMO_SHOP_IDENTIFIER.Lines 29 and 31 define these constants, but they are not referenced anywhere in the codebase. All call sites now use
DEMO_RESELLER_IDandDEMO_MERCHANT_IDENTIFIERinstead. Removing these unused constants will reduce confusion and clarify the current intent of the code.app/ai/voice/agents/breeze_buddy/template/loader.py (1)
96-101: Stale docstring:template_varsparam documented but doesn't exist.Line 99 documents a
template_varsparameter that isn't part of the signature. This predates this PR but is worth a quick fix while you're touching this block.Suggested fix
Args: reseller_id: Reseller identifier - template: str type - template_vars: Variables for template rendering + template: Template name string merchant_identifier: Optional merchant-specific identifier + call_payload: Optional payload dictionary for template renderingapp/api/routers/breeze_buddy/configurations/handlers.py (1)
110-124: Consider passingmerchant_identifierto the DB query for efficiency.
get_call_execution_config_by_reseller_idaccepts an optionalmerchant_identifierparameter that can narrow results at the DB level. Currently, you're fetching all configs for the reseller and filtering in Python (Line 121-124). Passing it through would reduce the result set from the DB.Suggested optimization
if reseller_id: - configs = await get_call_execution_config_by_reseller_id(reseller_id) + configs = await get_call_execution_config_by_reseller_id(reseller_id, merchant_identifier) else: configs = await get_all_call_execution_configs() # Apply additional filters if template: configs = [c for c in configs if c.template == template] - if merchant_identifier: - configs = [ - c for c in configs if c.merchant_identifier == merchant_identifier - ]app/ai/voice/agents/breeze_buddy/managers/calls.py (2)
62-68: Minor terminology inconsistency in log message.Line 67 logs
"shop: {lead.merchant_identifier}"— the label "shop" is the old terminology. For consistency with the refactoring, consider using "merchant_identifier" or "merchant".Suggested fix
logger.warning( - f"No call execution config found for reseller: {lead.reseller_id} and shop: {lead.merchant_identifier}" + f"No call execution config found for reseller: {lead.reseller_id} and merchant_identifier: {lead.merchant_identifier}" )
159-168: Same "shop" label in several log messages.Lines 130, 162, and 168 still use "shop" to label
merchant_identifiervalues. Since this is a terminology refactoring PR, these labels should be updated for consistency.app/database/accessor/breeze_buddy/template.py (2)
178-213: Auto-detect heuristic inget_templates_listis fragile.The domain-pattern detection (Lines 183-184) checks for
"myshopify.com"and"http"to distinguish a merchant_identifier from a reseller_id. This is a pre-existing concern, but since this code is being touched: if a new integration uses a different domain, this heuristic silently breaks. Consider documenting the known limitation or making the detection configurable.
244-258: Backward-compatrow.get("merchant_id")fallback is redundant given COALESCE.Since
get_templates_list_queryalready aliases viaCOALESCE(reseller_id, merchant_id) AS reseller_id,row.get("reseller_id")will always have the resolved value. Theor row.get("merchant_id")fallback on Line 246 would never trigger. Not harmful, but worth noting as dead code.app/database/queries/breeze_buddy/template.py (1)
93-151:get_templates_list_queryfilter key checks can be simplified (minor nit).Lines 114, 117, 122, 127 use
"key" in filters and filters["key"]which can be simplified tofilters.get("key"). Functionally equivalent but more idiomatic Python.Example simplification
- if "reseller_ids" in filters and filters["reseller_ids"]: + if filters.get("reseller_ids"): values.append(filters["reseller_ids"]) conditions.append(f"COALESCE(reseller_id, merchant_id) = ANY(${len(values)})") - elif "reseller_id" in filters and filters["reseller_id"]: + elif filters.get("reseller_id"): values.append(filters["reseller_id"]) conditions.append(f"COALESCE(reseller_id, merchant_id) = ${len(values)}")
| # Inline fallbacks for backward compatibility | ||
| if group_by == "reseller_id": | ||
| group_value = row.get("reseller_id") or row.get("merchant_id") | ||
| elif group_by == "merchant_identifier": | ||
| group_value = row.get("merchant_identifier") or row.get( | ||
| "shop_identifier" | ||
| ) | ||
| else: | ||
| group_value = row.get(group_by) | ||
|
|
||
| results.append( | ||
| { | ||
| group_by: row[group_by], | ||
| group_by: group_value, |
There was a problem hiding this comment.
Bug: legacy group_by values ("merchant_id", "shop_identifier") produce None group values.
When a consumer sends group_by="merchant_id", the query builder maps it to group_by_column="reseller_id" and the DB result column is aliased as reseller_id. But here the handler checks group_by == "reseller_id" (line 332), which won't match. It falls to the else branch and does row.get("merchant_id"), which won't exist in the result — yielding None.
The same issue applies to group_by="shop_identifier" → DB column is "merchant_identifier", but the handler tries row.get("shop_identifier").
Proposed fix
- if group_by == "reseller_id":
+ if group_by in ("reseller_id", "merchant_id"):
group_value = row.get("reseller_id") or row.get("merchant_id")
- elif group_by == "merchant_identifier":
+ elif group_by in ("merchant_identifier", "shop_identifier"):
group_value = row.get("merchant_identifier") or row.get(
"shop_identifier"
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Inline fallbacks for backward compatibility | |
| if group_by == "reseller_id": | |
| group_value = row.get("reseller_id") or row.get("merchant_id") | |
| elif group_by == "merchant_identifier": | |
| group_value = row.get("merchant_identifier") or row.get( | |
| "shop_identifier" | |
| ) | |
| else: | |
| group_value = row.get(group_by) | |
| results.append( | |
| { | |
| group_by: row[group_by], | |
| group_by: group_value, | |
| # Inline fallbacks for backward compatibility | |
| if group_by in ("reseller_id", "merchant_id"): | |
| group_value = row.get("reseller_id") or row.get("merchant_id") | |
| elif group_by in ("merchant_identifier", "shop_identifier"): | |
| group_value = row.get("merchant_identifier") or row.get( | |
| "shop_identifier" | |
| ) | |
| else: | |
| group_value = row.get(group_by) | |
| results.append( | |
| { | |
| group_by: group_value, |
🤖 Prompt for AI Agents
In `@app/api/routers/breeze_buddy/analytics/handlers.py` around lines 331 - 343,
The handler is using the incoming group_by string to extract the DB row value
but the query aliases legacy group_by values to different column names, causing
None results; update the logic around group_by (the block that sets group_value
before results.append) to normalize legacy keys to the actual DB column or to
try both aliases: when group_by is "merchant_id" or "reseller_id" prefer
row.get("reseller_id") then row.get("merchant_id"); when group_by is
"shop_identifier" or "merchant_identifier" prefer row.get("merchant_identifier")
then row.get("shop_identifier"); for all other group_by values keep the existing
row.get(group_by) behavior so group_value is never None due to aliasing.
| # Get reseller_id with backward compatibility | ||
| reseller_id = req.reseller or req.merchant | ||
|
|
||
| if not reseller_id: | ||
| return JSONResponse( | ||
| status_code=400, | ||
| content={"detail": "Either 'reseller' or 'merchant' must be provided"}, | ||
| ) | ||
|
|
||
| # Fetch template to get expected payload schema | ||
| template = await get_template_by_merchant( | ||
| req.merchant, req.identifier, req.template | ||
| template = await get_template_by_reseller( | ||
| reseller_id, req.identifier, req.template | ||
| ) | ||
|
|
||
| if not template: | ||
| return JSONResponse( | ||
| status_code=404, | ||
| content={ | ||
| "detail": f"Template '{req.template}' not found for merchant: {req.merchant}" | ||
| "detail": f"Template '{req.template}' not found for reseller: {req.reseller}" |
There was a problem hiding this comment.
Error messages use req.reseller instead of reseller_id, which may log None.
When req.reseller is falsy and req.merchant is used (line 188), all subsequent error/log messages on lines 205, 218, 228, and 238 reference req.reseller — which would be None or empty — instead of the resolved reseller_id.
Proposed fix (apply same pattern to lines 218, 228, 238)
content={
- "detail": f"Template '{req.template}' not found for reseller: {req.reseller}"
+ "detail": f"Template '{req.template}' not found for reseller: {reseller_id}"
},- f"Payload validation failed for reseller {req.reseller}: {error_message}"
+ f"Payload validation failed for reseller {reseller_id}: {error_message}"- logger.info(f"Payload validation successful for reseller {req.reseller}")
+ logger.info(f"Payload validation successful for reseller {reseller_id}")- "detail": f"Call execution config not found for reseller_id: {req.reseller}"
+ "detail": f"Call execution config not found for reseller_id: {reseller_id}"🤖 Prompt for AI Agents
In `@app/api/routers/breeze_buddy/deprecated/leads.py` around lines 187 - 205, The
error messages reference req.reseller which may be falsy when you resolved the
ID into reseller_id (reseller_id = req.reseller or req.merchant); update all
error/log responses that currently interpolate req.reseller to use the resolved
reseller_id instead (for example the 404 JSONResponse after
get_template_by_reseller and the other error messages that mention the
reseller), keeping the rest of the message intact; ensure you still use
req.template and any other req fields as before and only replace req.reseller
with reseller_id in those f-strings or message constructions.
| # Get reseller_id with backward compatibility | ||
| reseller_id = req.reseller or req.merchant or "" | ||
| # RBAC: Check permission to push leads for this reseller/shop | ||
| validate_lead_access( | ||
| current_user, req.merchant, req.identifier, operation="push leads for" | ||
| current_user, reseller_id, req.identifier, operation="push leads for" | ||
| ) |
There was a problem hiding this comment.
Empty reseller_id falls through to handler without validation.
If both req.reseller and req.merchant are None/empty, reseller_id becomes "". For admin users, RBAC passes unconditionally, and the lead would be created with an empty reseller identifier. Consider adding an explicit check:
Proposed fix
# Get reseller_id with backward compatibility
reseller_id = req.reseller or req.merchant or ""
+ if not reseller_id:
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail="reseller (or merchant for backward compatibility) is required",
+ )
# RBAC: Check permission to push leads for this reseller/shop
validate_lead_access(
current_user, reseller_id, req.identifier, operation="push leads for"
)🤖 Prompt for AI Agents
In `@app/api/routers/breeze_buddy/leads/__init__.py` around lines 85 - 90,
reseller_id is computed as `reseller_id = req.reseller or req.merchant or ""`,
which allows an empty string to bypass downstream logic (admins pass RBAC) and
create leads with no reseller; update the handler to explicitly validate
presence of a reseller identifier before calling `validate_lead_access` by
raising/returning a 4xx error when `reseller_id` is falsy, or require
`req.identifier`/other fallback as appropriate; ensure the check is performed
immediately prior to calling `validate_lead_access(current_user, reseller_id,
req.identifier, operation="push leads for")` so that no empty `reseller_id` can
reach lead creation.
| # Get reseller_id with backward compatibility (prefer reseller, fallback to merchant) | ||
| reseller_id = req.reseller or req.merchant or "" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the PushLeadRequest model definition
rg -n 'class PushLeadRequest' --type=py -A 30Repository: juspay/clairvoyance
Length of output: 2465
🏁 Script executed:
# Search for where reseller/merchant fields are defined and their validation
rg -n 'reseller.*Field\|merchant.*Field' --type=py | head -50Repository: juspay/clairvoyance
Length of output: 45
🏁 Script executed:
# Check the handlers.py file context around lines 113-114
cat -n app/api/routers/breeze_buddy/leads/handlers.py | sed -n '100,160p'Repository: juspay/clairvoyance
Length of output: 2695
🏁 Script executed:
# Check the database query functions to understand how they handle empty strings
rg -n 'def get_template_by_reseller|def get_call_execution_config_by_reseller_id' --type=py -A 15Repository: juspay/clairvoyance
Length of output: 6023
🏁 Script executed:
# Look for LeadCallTracker model definition and its fields
rg -n 'class LeadCallTracker' --type=py -A 25Repository: juspay/clairvoyance
Length of output: 1985
Add validation to prevent empty reseller_id from being passed to database queries.
Both reseller and merchant in PushLeadRequest are optional with no validation ensuring at least one is provided. If both are None, reseller_id becomes "" and is passed to queries like COALESCE(reseller_id, merchant_id) = '', which silently return no results without a clear error message to the caller.
Validate upfront and reject the request:
Suggested fix
# Get reseller_id with backward compatibility (prefer reseller, fallback to merchant)
reseller_id = req.reseller or req.merchant or ""
+ if not reseller_id:
+ raise HTTPException(
+ status_code=status.HTTP_400_BAD_REQUEST,
+ detail="Either 'reseller' or 'merchant' must be provided",
+ )🤖 Prompt for AI Agents
In `@app/api/routers/breeze_buddy/leads/handlers.py` around lines 113 - 114, The
code currently sets reseller_id = req.reseller or req.merchant or "" which
allows an empty string to be used in DB queries; update the PushLeadRequest
handling to validate that at least one of req.reseller or req.merchant is
present before building reseller_id (e.g., check if not (req.reseller or
req.merchant) and immediately return/raise a 400 Bad Request with a clear
message), then set reseller_id = req.reseller or req.merchant; ensure this
validation occurs prior to any use of reseller_id in queries (references:
variable reseller_id and the request object PushLeadRequest/req used in the
handler).
| Returns all unique merchant_identifiers from call_execution_config. | ||
| Each merchant_identifier represents a distinct merchant in the system. | ||
|
|
||
| This assumes every shop has at least one call execution config. |
There was a problem hiding this comment.
Stale "shop" terminology remains.
Line 30 still references "every shop" which is inconsistent with the rename from shop_identifier to merchant_identifier throughout this PR.
Proposed fix
- This assumes every shop has at least one call execution config.
+ This assumes every merchant has at least one call execution config.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| This assumes every shop has at least one call execution config. | |
| This assumes every merchant has at least one call execution config. |
🤖 Prompt for AI Agents
In `@app/api/routers/breeze_buddy/merchants/__init__.py` at line 30, Update the
stale "shop" terminology on line 30 (and any nearby occurrences in
app/api/routers/breeze_buddy/merchants/__init__.py) to use "merchant" or
"merchant_identifier" to match the rename; specifically replace phrases like
"every shop" with "every merchant" and ensure any references to shop_identifier
are merchant_identifier so variable names and comments stay consistent with the
rest of the PR.
| CREATE INDEX IF NOT EXISTS idx_call_execution_config_reseller_id | ||
| ON call_execution_config(reseller_id); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -type f -name "*add_reseller_id_and_merchant_identifier*"Repository: juspay/clairvoyance
Length of output: 233
🏁 Script executed:
find . -type f -name "*.sql" -path "*/migrations/*" | head -20Repository: juspay/clairvoyance
Length of output: 1012
🏁 Script executed:
cat -n app/database/migrations/add_reseller_id_and_merchant_identifier_columns.sqlRepository: juspay/clairvoyance
Length of output: 6466
🏁 Script executed:
# Search for existing indexes on merchant_id and shop_identifier in migrations
rg -n "CREATE INDEX.*merchant_id\|CREATE INDEX.*shop_identifier" app/database/migrations/ -A 2Repository: juspay/clairvoyance
Length of output: 45
🏁 Script executed:
# Search for all index definitions on merchant_id or shop_identifier
rg -n "CREATE INDEX" app/database/migrations/ -A 2 | grep -E "merchant_id|shop_identifier"Repository: juspay/clairvoyance
Length of output: 5003
🏁 Script executed:
# Search for indexes with both columns
rg -n "merchant_id.*shop_identifier|shop_identifier.*merchant_id" app/database/migrations/Repository: juspay/clairvoyance
Length of output: 2719
🏁 Script executed:
# Search application code for queries filtering on both reseller_id AND merchant_identifier
rg -n "reseller_id.*merchant_identifier|merchant_identifier.*reseller_id" --glob "*.js" --glob "*.ts" --glob "*.py" -C 2 | head -50Repository: juspay/clairvoyance
Length of output: 3774
🏁 Script executed:
# Search for WHERE clauses filtering on reseller_id AND merchant_identifier or their old equivalents
rg -n "WHERE.*reseller_id.*merchant_identifier|WHERE.*merchant_identifier.*reseller_id" --glob "*.py" --glob "*.sql" -B 2 -A 2 | head -40Repository: juspay/clairvoyance
Length of output: 547
🏁 Script executed:
# Also search for call_execution_config, lead_call_tracker, outbound_number, template queries to see filtering patterns
rg -n "call_execution_config|lead_call_tracker|outbound_number|template" --glob "*.py" -A 3 | grep -E "WHERE|reseller_id|merchant_identifier" | head -50Repository: juspay/clairvoyance
Length of output: 5469
🏁 Script executed:
cat -n app/database/queries/breeze_buddy/call_execution_config.py | head -250Repository: juspay/clairvoyance
Length of output: 9593
🏁 Script executed:
# Get the full context around the WHERE clause building
sed -n '200,250p' app/database/queries/breeze_buddy/call_execution_config.pyRepository: juspay/clairvoyance
Length of output: 1819
🏁 Script executed:
# Check if there are any migrations after the current one that might add composite indexes
ls -la app/database/migrations/ | grep -E "add_reseller|add_merchant"Repository: juspay/clairvoyance
Length of output: 261
🏁 Script executed:
# Search for any other composite index creations in more recent migrations
git ls-files app/database/migrations/ | sort | tail -10Repository: juspay/clairvoyance
Length of output: 688
🏁 Script executed:
# Look for composite index patterns on the new columns anywhere
rg -n "reseller_id.*merchant_identifier|merchant_identifier.*reseller_id" app/database/migrations/Repository: juspay/clairvoyance
Length of output: 590
Add composite indexes on (reseller_id, merchant_identifier) for the affected tables.
The old columns (merchant_id, shop_identifier) had composite indexes: (merchant_id, shop_identifier) on outbound_number, (merchant_id, shop_identifier, name) on template, and (merchant_id, workflow, shop_identifier) on call_execution_config. Queries filter on both new columns together (e.g., WHERE reseller_id = ? AND merchant_identifier = ?), so matching the old index structure with composite indexes will improve query performance and avoid index-merge overhead.
Applies to all affected tables: call_execution_config (lines 24-25, 37-38), lead_call_tracker (lines 50-51, 63-64), outbound_number (lines 76-77, 89-90), and template (lines 102-103, 115-116).
🤖 Prompt for AI Agents
In `@app/database/migrations/add_reseller_id_and_merchant_identifier_columns.sql`
around lines 24 - 25, Add composite indexes matching the old multi-column
patterns for the affected tables: create IF NOT EXISTS indexes on (reseller_id,
merchant_identifier) for outbound_number and lead_call_tracker, on (reseller_id,
merchant_identifier, name) for template, and on (reseller_id, workflow,
merchant_identifier) for call_execution_config; place the CREATE INDEX IF NOT
EXISTS statements near the existing single-column index statements (e.g.,
alongside idx_call_execution_config_reseller_id,
idx_lead_call_tracker_reseller_id, idx_outbound_number_reseller_id,
idx_template_reseller_id) and choose unique, descriptive index names (e.g.,
idx_outbound_number_reseller_merchant_identifier,
idx_template_reseller_merchant_identifier_name,
idx_call_execution_config_reseller_workflow_merchant_identifier,
idx_lead_call_tracker_reseller_merchant_identifier). Ensure the SQL uses the
exact column names reseller_id, merchant_identifier, workflow, and name as in
the diff and includes IF NOT EXISTS.
| def get_all_resellers_query() -> Tuple[str, List[Any]]: | ||
| """ | ||
| Generate query to get all unique merchants (shop_identifiers). | ||
| Generate query to get all unique resellers (merchant_identifiers). | ||
| Uses COALESCE for backward compatibility with old column names. | ||
|
|
||
| Returns all unique shop_identifier values from call_execution_config. | ||
| Each shop_identifier represents a distinct merchant in the system. | ||
| Returns all unique merchant_identifier values from call_execution_config. | ||
| Each merchant_identifier represents a distinct reseller in the system. | ||
|
|
||
| Returns: | ||
| Tuple of (query string, empty values list) | ||
| """ | ||
| query = f""" | ||
| SELECT DISTINCT shop_identifier | ||
| SELECT DISTINCT COALESCE(merchant_identifier, shop_identifier) AS merchant_identifier | ||
| FROM {CALL_EXECUTION_CONFIG_TABLE} | ||
| WHERE shop_identifier IS NOT NULL | ||
| ORDER BY shop_identifier ASC | ||
| WHERE COALESCE(merchant_identifier, shop_identifier) IS NOT NULL | ||
| ORDER BY merchant_identifier ASC | ||
| """ | ||
|
|
||
| return query, [] |
There was a problem hiding this comment.
Function name get_all_resellers_query is misleading — it returns merchant_identifier values (individual merchants), not resellers.
This query returns distinct merchant_identifier values (formerly shop_identifier), which represent individual merchant locations — not resellers (parent groups). Based on learnings, the previous name get_all_merchants_query was semantically correct. The rename to get_all_resellers_query inverts the meaning.
Consider renaming to get_all_merchants_query or get_all_merchant_identifiers_query to preserve the correct semantics.
Based on learnings: "In Breeze Buddy, treat shop_identifier as identifying individual merchant locations, while merchant_id acts as a parent/group identifier. Therefore, functions that retrieve shop_identifiers should be named as 'merchants' functions."
🧰 Tools
🪛 Ruff (0.14.14)
[error] 299-304: Possible SQL injection vector through string-based query construction
(S608)
🤖 Prompt for AI Agents
In `@app/database/queries/breeze_buddy/call_execution_config.py` around lines 288
- 306, The function name get_all_resellers_query is misleading because it
returns merchant_identifier/shop_identifier (individual merchant locations);
rename the function to get_all_merchants_query (or
get_all_merchant_identifiers_query) and update its docstring accordingly, then
update all references/calls to get_all_resellers_query throughout the codebase
(including tests and imports) to the new name; keep the SQL query, COALESCE
usage and return signature unchanged (query, []), and ensure symbols like
CALL_EXECUTION_CONFIG_TABLE remain referenced as before.
| reseller_ids=( | ||
| row["reseller_ids"] | ||
| if isinstance(row["reseller_ids"], list) | ||
| else json.loads(row["reseller_ids"]) | ||
| ), | ||
| shop_identifiers=( | ||
| row["shop_identifiers"] | ||
| if isinstance(row["shop_identifiers"], list) | ||
| else json.loads(row["shop_identifiers"]) | ||
| merchant_identifiers=( | ||
| row["merchant_identifiers"] | ||
| if isinstance(row["merchant_identifiers"], list) | ||
| else json.loads(row["merchant_identifiers"]) | ||
| ), |
There was a problem hiding this comment.
json.loads(None) will raise TypeError if both columns are NULL.
COALESCE returns NULL when both the old and new columns are NULL. In that case, isinstance(None, list) is False, so the code falls through to json.loads(None), which throws TypeError. While this is caught by the outer except, it silently swallows a valid user row and returns None — appearing as "user not found" instead of surfacing the data issue.
🛡️ Proposed fix — default to empty list on NULL
reseller_ids=(
- row["reseller_ids"]
- if isinstance(row["reseller_ids"], list)
- else json.loads(row["reseller_ids"])
+ row["reseller_ids"]
+ if isinstance(row["reseller_ids"], list)
+ else json.loads(row["reseller_ids"])
+ if row["reseller_ids"] is not None
+ else []
),
merchant_identifiers=(
- row["merchant_identifiers"]
- if isinstance(row["merchant_identifiers"], list)
- else json.loads(row["merchant_identifiers"])
+ row["merchant_identifiers"]
+ if isinstance(row["merchant_identifiers"], list)
+ else json.loads(row["merchant_identifiers"])
+ if row["merchant_identifiers"] is not None
+ else []
),🤖 Prompt for AI Agents
In `@app/database/queries/breeze_buddy/users.py` around lines 53 - 62, The code
calls json.loads(...) unconditionally when the DB value isn't a list, which will
raise TypeError for NULL (None) and causes valid rows to be swallowed; update
the handling for reseller_ids and merchant_identifiers to treat None as an empty
list: fetch the raw value (row["reseller_ids"] / row["merchant_identifiers"]),
if it's already a list keep it, else if the raw is falsy/None use [] as the
default, otherwise call json.loads(raw) to parse the JSON string; apply the same
pattern to both reseller_ids and merchant_identifiers.
There was a problem hiding this comment.
COALESCE(reseller_ids, merchant_ids) AS reseller_ids,
COALESCE(merchant_identifiers, shop_identifiers) AS merchant_identifiers
both we will not be null because we have NOT NULL constraint in DB
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the migration file for NOT NULL constraints on the users table columns
grep -A 50 "users" app/database/migrations/add_reseller_id_and_merchant_identifier_columns.sql | grep -E "(reseller_ids|merchant_ids|merchant_identifiers|shop_identifiers)" | head -20
Length of output: 710
🏁 Script executed:
#!/bin/bash
# Search for the original users table schema definition
rg -n "CREATE TABLE.*users" app/database/migrations/ -A 30 | head -50
Length of output: 3057
@sharifajahanshaik You're absolutely right! I can see from the original schema that merchant_ids and shop_identifiers have NOT NULL constraints with DEFAULT '[]'::jsonb, so the COALESCE will always fall back to a non-NULL value even if the new columns are NULL. My concern about json.loads(None) was unfounded. Thank you for the clarification!
🧬 Code Graph Analysis Results
- File: app/database/init.py (lines 66-78)
async def get_db_connection():
"""
Get a database connection from the pool.
"""
global pool
if pool is None:
await init_db_pool()
if pool is None:
raise RuntimeError("Database pool is not initialized")
async with pool.acquire() as connection:
yield connection
- File: app/schemas/breeze_buddy/auth.py (lines 156-174)
class UserInDB(BaseModel):
"""User model as stored in database"""
id: str
username: str
password_hash: str
role: UserRole
email: Optional[str] = None
reseller_ids: List[str] = Field(default_factory=list)
merchant_ids: Optional[List[str]] = Field(
default_factory=list
) # Backward compatibility: same as reseller_ids (will be removed)
merchant_identifiers: List[str] = Field(default_factory=list)
shop_identifiers: Optional[List[str]] = Field(
default_factory=list
) # Backward compatibility: same as merchant_identifiers (will be removed)
is_active: bool = True
created_at: Optional[datetime] = None
updated_at: Optional[datetime] = None
- File: app/schemas/breeze_buddy/auth.py (lines 19-25)
class UserRole(str, Enum):
"""User role enum for RBAC"""
ADMIN = "admin"
RESELLER = "reseller"
MERCHANT = "merchant"
SHOP = "shop"
🧠 Learnings used
Learnt from: narsimhaReddyJuspay
Repo: juspay/clairvoyance PR: 508
File: app/api/routers/breeze_buddy/user_accounts/handlers.py:111-119
Timestamp: 2026-02-04T13:26:51.610Z
Learning: In the user account management system at app/api/routers/breeze_buddy/user_accounts/handlers.py, merchant_ids are always provided during user account creation, so target_user.merchant_ids will never be an empty list in practice.
Learnt from: badri-singhal
Repo: juspay/clairvoyance PR: 445
File: app/database/migrations/009_add_merchant_template_to_outbound_number.sql:38-38
Timestamp: 2025-12-24T07:52:29.756Z
Learning: In the outbound_number table in app/database/migrations/009_add_merchant_template_to_outbound_number.sql, duplicate phone numbers across merchant_id and shop_identifier combinations are intentionally allowed. No composite unique constraint on (number, merchant_id, shop_identifier) is required.
Learnt from: swaroopvarma1
Repo: juspay/clairvoyance PR: 447
File: app/database/queries/breeze_buddy/call_execution_config.py:268-285
Timestamp: 2025-12-23T12:11:46.856Z
Learning: In the Breeze Buddy system, each `shop_identifier` represents a distinct merchant. The `merchant_id` is a parent/group identifier, while `shop_identifier` uniquely identifies individual merchant locations/shops. Therefore, functions that retrieve shop_identifiers are correctly named as "merchants" functions.
| Returns unique merchant_identifiers from call_execution_config. | ||
| Each merchant_identifier represents a distinct merchant in the system. | ||
| """ | ||
|
|
||
| merchants: List[str] | ||
| resellers: List[str] | ||
| merchants: List[str] = [] # Backward compatibility: same as resellers |
There was a problem hiding this comment.
Naming mismatch: resellers field actually contains merchant_identifier values.
Based on the accessor (get_all_resellers in call_execution_config.py lines 304–331), this field is populated with merchant_identifier values extracted from DB rows. But the field is named resellers, which in the new terminology corresponds to reseller_id (the parent/group identifier). This inverted naming could confuse future developers.
Based on learnings: in Breeze Buddy, merchant_identifier (formerly shop_identifier) identifies individual merchant locations, while reseller_id (formerly merchant_id) is the parent/group identifier. The function get_all_resellers returning merchant_identifier values but naming them "resellers" conflates these two concepts.
🤖 Prompt for AI Agents
In `@app/schemas/breeze_buddy/merchants.py` around lines 11 - 16, The field name
resellers in app/schemas/breeze_buddy/merchants.py is misleading because
get_all_resellers in call_execution_config.py actually populates
merchant_identifier values; rename resellers to merchant_identifiers (and keep
merchants as an alias for backward compatibility) and update the type annotation
and any serialisation/usage references to use merchant_identifiers instead of
resellers; also search for and update usages of the old resellers attribute
(including schema initialisation and tests) to the new merchant_identifiers name
or access merchants where backward compatibility is required.
app/schemas/breeze_buddy/template.py
Outdated
| id: str | ||
| merchant_id: str | ||
| shop_identifier: Optional[str] = None | ||
| reseller_id: Optional[str] = None | ||
| merchant_identifier: Optional[str] = None |
There was a problem hiding this comment.
reseller_id changed from required to optional.
The AI summary indicates the previous merchant_id field was str (required). The new reseller_id is Optional[str] = None. Since this is a response schema used for listing templates, ensure the database always provides a non-null reseller_id value, or that consumers gracefully handle None.
🤖 Prompt for AI Agents
In `@app/schemas/breeze_buddy/template.py` around lines 16 - 18, The schema
changed `reseller_id` from required to Optional (reseller_id: Optional[str] =
None) which may break consumers expecting a non-null value: update the
template-listing/serialization flow to either (A) ensure the database/query
layer always populates a non-empty string for reseller_id before constructing
the response object (e.g., in the function that builds Template responses or in
the ORM mapping that returns id/reseller_id/merchant_identifier), or (B) make
callers tolerant of None by handling reseller_id == None where templates are
consumed (e.g., in list_templates, Template serializer or response formatter)
and providing a safe fallback (empty string or explicit "unknown") so consumers
never assume a non-null value. Ensure changes touch the code that instantiates
the schema (the Template response builder / list_templates handler) and include
tests that cover reseller_id == None.
a7c3969 to
a7cf6c1
Compare
sharifajahanshaik
force-pushed
the
refactor-merchantid-and-shopidentifier
branch
from
a7cf6c1 to
50c7ccf
Compare
There was a problem hiding this comment.
Pull request overview
Refactors Breeze Buddy’s identifier terminology and access model by renaming merchant_id → reseller_id and shop_identifier → merchant_identifier across API schemas, DB queries/decoders, and RBAC-enforced routes, with several backward-compatibility shims.
Changes:
- Rename identifier fields across schemas, routers/handlers, RBAC helpers, and analytics filters/grouping.
- Update DB query builders/decoders to read/write new columns (often via
COALESCE) and adjust template/config/lead flows accordingly. - Update architecture docs and various runtime defaults/configs touched in the same PR (CORS default origins, TTS default service).
Reviewed changes
Copilot reviewed 54 out of 54 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/BREEZE_BUDDY_ARCHITECTURE.md | Docs updated to reseller/merchant_identifier terminology |
| app/schemas/breeze_buddy/template.py | Template metadata schema adds reseller/merchant_identifier + legacy fields |
| app/schemas/breeze_buddy/merchants.py | Merchants response reshaped to include resellers + legacy field handling |
| app/schemas/breeze_buddy/core.py | Core domain schemas renamed to reseller/merchant_identifier with some legacy fields retained |
| app/schemas/breeze_buddy/auth.py | JWT user RBAC fields renamed to reseller_ids/merchant_identifiers with legacy fields retained |
| app/schemas/breeze_buddy/analytics.py | Analytics filter/result schemas renamed + backward-compatible dump logic |
| app/database/queries/breeze_buddy/users.py | User query updated to prefer new reseller/merchant_identifier arrays |
| app/database/queries/breeze_buddy/template.py | Template queries rewritten for reseller/merchant_identifier + COALESCE compatibility |
| app/database/queries/breeze_buddy/outbound_number.py | Outbound number insert/select updated for new columns + COALESCE selects |
| app/database/queries/breeze_buddy/lead_call_tracker.py | Lead call tracker insert/select updated for new columns + COALESCE selects |
| app/database/queries/breeze_buddy/call_execution_config.py | Call execution config queries renamed + COALESCE filtering + renamed helpers |
| app/database/queries/breeze_buddy/analytics.py | Analytics SQL where-clause and group-by updated with COALESCE mapping |
| app/database/decoder/breeze_buddy/template.py | Template decoder now derives reseller/merchant_identifier with fallbacks |
| app/database/decoder/breeze_buddy/outbound_number.py | Outbound number decoder updated for reseller/merchant_identifier fields |
| app/database/decoder/breeze_buddy/lead_call_tracker.py | Lead decoder updated for reseller/merchant_identifier fields |
| app/database/decoder/breeze_buddy/call_execution_config.py | Call config decoder updated with reseller/merchant_identifier + legacy fields populated |
| app/database/accessor/breeze_buddy/template.py | Template accessors renamed and updated to reseller/merchant_identifier + fallback logic |
| app/database/accessor/breeze_buddy/outbound_number.py | Outbound number accessor updated + added additional logging |
| app/database/accessor/breeze_buddy/lead_call_tracker.py | Lead tracker accessor updated to reseller/merchant_identifier |
| app/database/accessor/breeze_buddy/call_execution_config.py | Call config accessors renamed and updated to reseller/merchant_identifier |
| app/database/accessor/breeze_buddy/analytics.py | Analytics accessor maps group_by to new query output aliases |
| app/database/accessor/init.py | Export names updated (merchant → reseller variants) |
| app/core/config/static.py | Default CORS origins changed |
| app/core/config/dynamic.py | Default TTS service changed |
| app/api/security/breeze_buddy/rbac_token.py | JWT payload fields renamed to reseller_ids/merchant_identifiers |
| app/api/security/breeze_buddy/authorization.py | Authorization helpers renamed and updated for reseller/merchant_identifier |
| app/api/security/breeze_buddy/init.py | Security exports updated for renamed helper |
| app/api/routers/breeze_buddy/templates/rbac.py | Template RBAC updated to reseller/merchant_identifier |
| app/api/routers/breeze_buddy/templates/handlers.py | Template handlers updated to reseller/merchant_identifier |
| app/api/routers/breeze_buddy/templates/init.py | Template routes updated to reseller/merchant_identifier |
| app/api/routers/breeze_buddy/telephony/inbound/handlers.py | Inbound telephony template lookup updated to reseller/merchant_identifier |
| app/api/routers/breeze_buddy/numbers/handlers.py | Outbound number creation updated to reseller/merchant_identifier |
| app/api/routers/breeze_buddy/merchants/handlers.py | Merchants listing handler now returns resellers list (with legacy field) |
| app/api/routers/breeze_buddy/merchants/init.py | Merchants endpoint docs updated |
| app/api/routers/breeze_buddy/leads/rbac.py | Lead RBAC updated to reseller/merchant_identifier |
| app/api/routers/breeze_buddy/leads/handlers.py | Lead push/recording handlers updated to reseller/merchant_identifier + fallback logic |
| app/api/routers/breeze_buddy/leads/init.py | Leads route docs and entry logic updated for reseller fields |
| app/api/routers/breeze_buddy/demo/handlers.py | Demo flow updated to reseller/merchant_identifier naming |
| app/api/routers/breeze_buddy/configurations/rbac.py | Config RBAC updated to reseller/merchant_identifier |
| app/api/routers/breeze_buddy/configurations/handlers.py | Config CRUD/activation handlers updated to reseller/merchant_identifier |
| app/api/routers/breeze_buddy/configurations/init.py | Config routes/docs updated to reseller/merchant_identifier |
| app/api/routers/breeze_buddy/auth/handlers.py | Auth token creation updated to reseller_ids/merchant_identifiers |
| app/api/routers/breeze_buddy/auth/init.py | Auth route docs updated for renamed RBAC fields |
| app/api/routers/breeze_buddy/analytics/rbac.py | Analytics RBAC filters updated to reseller/merchant_identifier |
| app/api/routers/breeze_buddy/analytics/handlers.py | Analytics handlers add backward-compatible fallbacks for reseller/merchant_identifier |
| app/ai/voice/agents/breeze_buddy/utils/secrets.py | Secret masking updated for reseller/merchant_identifier |
| app/ai/voice/agents/breeze_buddy/types/models.py | PushLeadRequest updated to accept reseller/merchant legacy; adds LeadData fields |
| app/ai/voice/agents/breeze_buddy/template/types.py | Template model and create request updated to reseller/merchant_identifier |
| app/ai/voice/agents/breeze_buddy/template/loader.py | Template loader updated to reseller/merchant_identifier |
| app/ai/voice/agents/breeze_buddy/observability/tracing_setup.py | Tracing attributes updated (merchant_id → reseller_id) |
| app/ai/voice/agents/breeze_buddy/managers/calls.py | Call manager updated to reseller/merchant_identifier + conditional template loading |
| app/ai/voice/agents/breeze_buddy/callbacks/service_callback.py | Callback checks max retry using reseller-based config lookup |
| app/ai/voice/agents/breeze_buddy/agent/inbound.py | Inbound agent lead creation updated to reseller/merchant_identifier |
| app/ai/voice/agents/breeze_buddy/agent/flow.py | Flow loading updated to reseller/merchant_identifier |
Comments suppressed due to low confidence (2)
app/api/routers/breeze_buddy/numbers/handlers.py:49
create_number_handlerrejects requests that provide only the legacymerchant_id, even thoughCreateOutboundNumberRequeststill definesmerchant_idfor backward compatibility. Use a fallback likereseller_id = number.reseller_id or number.merchant_id(and similarly for identifier fields) so older clients keep working during the transition.
if not number.reseller_id:
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail="merchant_id is required",
)
app/api/routers/breeze_buddy/numbers/handlers.py:49
- The 400 error detail still says
merchant_id is requiredeven though the handler now checksreseller_id. This will be confusing for API consumers (especially during migration). Update the message to referencereseller_id(and optionally mentionmerchant_idis accepted for backward compatibility if you keep that behavior).
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail="merchant_id is required",
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| logger.info(f"Result of outbound number query: {result}") | ||
|
|
||
| if result: | ||
| decoded_result = decode_outbound_number_list(result) | ||
| logger.info(f"decoded result is: {decoded_result}") | ||
| logger.info(f"Found {len(decoded_result)} outbound numbers") |
There was a problem hiding this comment.
Logging the full raw DB result and full decoded_result at INFO level can leak sensitive data (phone numbers, identifiers) and can be very noisy in production. Consider removing these logs, downgrading to DEBUG, or logging only counts/IDs with redaction.
| CORS_ALLOWED_ORIGINS = [ | ||
| origin.strip() | ||
| for origin in os.environ.get( | ||
| "CORS_ALLOWED_ORIGINS", | ||
| "https://buddy.breezelabs.app,https://portal.breeze.in,https://portal.breezesdk.store", | ||
| "https://buddy.breezelabs.app,https://portal.breeze.in,https://portal.breezesdk.store,http://localhost:5173", | ||
| ).split(",") |
There was a problem hiding this comment.
The default CORS_ALLOWED_ORIGINS now includes http://localhost:5173. If CORS_ALLOWED_ORIGINS is not explicitly set in an environment, this could unintentionally allow cross-origin requests from localhost in production. Consider keeping localhost origins only in dev configs / local env overrides rather than in the default fallback.
app/core/config/dynamic.py
Outdated
| async def BB_TTS_SERVICE() -> str: | ||
| """Returns BREEZE_BUDDY_TTS_SERVICE from Redis""" | ||
| return await get_config("BREEZE_BUDDY_TTS_SERVICE", "elevenlabs", str) | ||
| return await get_config("BREEZE_BUDDY_TTS_SERVICE", "sarvam", str) |
There was a problem hiding this comment.
This PR is described as renaming merchant/shop identifiers, but it also changes the default BREEZE_BUDDY_TTS_SERVICE from elevenlabs to sarvam. If this is not intentional, it should be reverted or moved to a separate PR to avoid surprising behavior changes during an identifier refactor.
| resellers: List[str] | ||
| merchants: List[str] = [] # Backward compatibility: same as resellers | ||
| total: int |
There was a problem hiding this comment.
Avoid using a mutable default ([]) for merchants. Even though Pydantic often copies defaults, this pattern is error-prone and inconsistent with other schemas in this repo that use Field(default_factory=list). Use Field(default_factory=list) here as well.
| row = result[0] | ||
|
|
||
| # Explicit None check (safe) | ||
| reseller_id = ( | ||
| row["reseller_id"] | ||
| if "reseller_id" in row and row["reseller_id"] is not None | ||
| else row.get("merchant_id") | ||
| ) | ||
|
|
||
| merchant_identifier = ( | ||
| row["merchant_identifier"] | ||
| if "merchant_identifier" in row and row["merchant_identifier"] is not None | ||
| else row.get("shop_identifier") | ||
| ) |
There was a problem hiding this comment.
In decode_outbound_number_list, reseller_id and merchant_identifier are computed from only the first row (result[0]) and then reused for every item. This can return incorrect reseller/merchant identifiers when the query returns outbound numbers across multiple resellers/merchants. Compute these fallback values per-row inside the loop/comprehension.
| return OutboundNumber( | ||
| id=row["id"], | ||
| number=row["number"], | ||
| provider=CallProvider(row["provider"]), | ||
| status=OutboundNumberStatus(row["status"]), | ||
| channels=row["channels"], | ||
| maximum_channels=row["maximum_channels"], | ||
| merchant_id=row["merchant_id"], | ||
| shop_identifier=row["shop_identifier"], | ||
| reseller_id=reseller_id, | ||
| merchant_identifier=merchant_identifier, | ||
| created_at=row["created_at"], | ||
| updated_at=row["updated_at"], | ||
| ) |
There was a problem hiding this comment.
OutboundNumber schema still includes legacy merchant_id/shop_identifier fields for backward compatibility, but decode_outbound_number only populates reseller_id/merchant_identifier. This means API responses will serialize legacy fields as null, breaking backward-compatible clients. Consider populating the legacy fields (or adding a model_dump override similar to CallDetailResult).
sharifajahanshaik
force-pushed
the
refactor-merchantid-and-shopidentifier
branch
2 times, most recently
from
05ec7ca to
5ce1308
Compare
sharifajahanshaik
force-pushed
the
refactor-merchantid-and-shopidentifier
branch
from
5ce1308 to
5523c9a
Compare
Summary by CodeRabbit
Release Notes
Breaking Changes
merchant_id→reseller_id,shop_identifier→merchant_identifieracross all endpointsConfiguration & Access
Database
Chores