Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,6 @@ create_test_user.sql
Taskfile.yml
.env.local
/.logs
/logs
webapp_logs.zip
/.schemas
189 changes: 189 additions & 0 deletions docs/user_groups_issue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
# User Groups Discrepancy Issue

## Summary

We've identified a discrepancy between the database state and API responses when retrieving user groups. When a user requests their groups, they're not receiving all groups where they are members. Specifically, when user ID `1062` requests their groups, they should receive 3 groups (IDs 22, 25, and 28), but only receive 2 groups (IDs 25 and 28).

## Investigation Details

### Debug Logs

The logs show only two groups being returned:

```
2025-05-21 21:38:17,604 - src.routers.user_groups - DEBUG - Fetching groups for user 1062...
2025-05-21 21:38:17,604 - src.database.user_groups - DEBUG - Getting user groups with users for user_id: 1062
2025-05-21 21:38:17,604 - src.database.user_groups - DEBUG - Fetching groups where user 1062 is a member...
2025-05-21 21:38:18,656 - src.database.user_groups - DEBUG - Found 1 groups where user 1062 is a member: [28]
2025-05-21 21:38:18,656 - src.database.user_groups - DEBUG - Fetching groups where user 1062 is an admin...
2025-05-21 21:38:19,606 - src.database.user_groups - DEBUG - Found 1 groups where user 1062 is an admin: [25]
2025-05-21 21:38:19,606 - src.database.user_groups - DEBUG - Added 1 additional groups as admin (not already as member): [25]
2025-05-21 21:38:19,606 - src.database.user_groups - DEBUG - Total: Found 2 user groups with users for user_id: 1062, Group IDs: [25, 28]
```

### Database State

Our database queries confirm that the user should be in 3 groups:

```sql
SELECT id, name, user_ids, admin_ids
FROM user_groups
WHERE 1062 = ANY(user_ids) OR 1062 = ANY(admin_ids)
ORDER BY id;
```

Result:
```
id | name | user_ids | admin_ids
----+-------------+-----------------+-----------
22 | UNDP Studio | {1062,774,1067} | {}
25 | GROUP 3 | {1062} | {1062}
28 | test4 | {774,1062} | {774}
```

### Code Analysis

The code in `user_groups.py` uses the correct SQL syntax to retrieve groups where a user is a member or admin:

1. First query: `SELECT ... FROM user_groups WHERE %s = ANY(user_ids) ORDER BY created_at DESC;`
2. Second query: `SELECT ... FROM user_groups WHERE %s = ANY(admin_ids) ORDER BY created_at DESC;`

These queries should correctly find all groups, but the first query is only returning group 28, not both 22 and 28 as expected.

## Impact

Users may not see all groups they belong to in the application, which could lead to:

1. Reduced access to signals shared in "missing" groups
2. Confusion about group membership
3. Workflow disruptions if users expect to find signals in specific groups

## Possible Causes

1. SQL query execution issues
2. Application-level filtering not visible in the code
3. A caching or stale data issue
4. Transaction isolation level issues
5. Potential race condition if groups are being modified simultaneously

## Fix Implemented

We've implemented a comprehensive solution with multiple layers of improvements:

### 1. Enhanced Primary Functions

Modified the approach in the affected functions to use a single combined query with explicit array handling:

```python
# Run a direct SQL query to ensure array type handling is consistent
query = """
SELECT
id,
name,
signal_ids,
user_ids,
admin_ids,
collaborator_map,
created_at
FROM
user_groups
WHERE
%s = ANY(user_ids) OR %s = ANY(admin_ids)
ORDER BY
created_at DESC;
"""

await cursor.execute(query, (user_id, user_id))
```

We also added explicit type conversion when checking for user membership:

```python
# Track membership rigorously by explicitly converting IDs to integers
is_member = False
if data['user_ids']:
is_member = user_id in [int(uid) for uid in data['user_ids']]

is_admin = False
if data['admin_ids']:
is_admin = user_id in [int(aid) for aid in data['admin_ids']]
```

### 2. Debug Functions

Added a `debug_user_groups` function that runs multiple direct queries to diagnose issues:

```python
async def debug_user_groups(cursor: AsyncCursor, user_id: int) -> dict:
# Various direct SQL queries to check database state
# and array position functions
# ...
```

### 3. Fallback Implementation

Created a completely separate direct SQL implementation in `user_groups_direct.py` as a fallback:

```python
async def get_user_groups_direct(cursor: AsyncCursor, user_id: int) -> List[UserGroup]:
"""
Get all groups that a user is a member of or an admin of using direct SQL.
"""
# Simple, direct SQL with minimal processing
# ...
```

### 4. Automatic Fallback in API

Modified the user groups router to automatically detect and handle discrepancies:

```python
# Check if there's a mismatch between direct query and regular function
if missing_ids:
logger.warning(f"MISMATCH! Direct query found groups {direct_group_ids} but function returned only {fetched_ids}")
logger.warning(f"Missing groups: {missing_ids}")

# Fall back to direct SQL implementation
logger.warning("Falling back to direct SQL implementation")
user_groups = await user_groups_direct.get_user_groups_with_users_direct(cursor, user.id)
logger.info(f"Direct SQL implementation returned {len(user_groups)} groups")
```

These changes ensure:
- More reliable querying of user group memberships
- Better debug information if issues persist
- Automatic fallback to a simpler implementation if needed
- More detailed logging throughout the process

After these changes, users should see all groups where they are members or admins consistently.

## Additional Fix: Signal Entity can_edit Attribute

During testing, we discovered that the Signal entity was missing the `can_edit` attribute that's dynamically added in user group contexts. This caused AttributeError exceptions when trying to access `signal.can_edit`.

### Issue
```python
AttributeError: 'Signal' object has no attribute 'can_edit'
```

### Solution
Added the `can_edit` field to the Signal entity definition:

```python
can_edit: bool = Field(
default=False,
description="Whether the current user can edit this signal (set dynamically based on group membership and collaboration).",
)
```

This ensures that:
- The Signal model accepts the `can_edit` attribute when created
- The attribute defaults to `False` for signals that don't have edit permissions
- Both the regular and direct SQL implementations can properly set this attribute
- Frontend code can safely access `signal.can_edit` without errors

The fix has been applied to both endpoints:
1. `/user-groups/me` (user groups without signals)
2. `/user-groups/me/with-signals` (user groups with signals)

Both endpoints now include the same debug checks and automatic fallback to direct SQL if discrepancies are detected.
73 changes: 65 additions & 8 deletions main.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from fastapi import Depends, FastAPI, Request
from fastapi.middleware.cors import CORSMiddleware
from fastapi.responses import JSONResponse
from starlette.middleware.base import BaseHTTPMiddleware

from src import routers
from src.authentication import authenticate_user
Expand All @@ -23,6 +24,9 @@
# Get application version
app_version = os.environ.get("RELEASE_VERSION", "dev")
app_env = os.environ.get("ENVIRONMENT", "development")
# Override environment setting if in local mode
if os.environ.get("ENV_MODE") == "local":
app_env = "local"
logging.info(f"Starting application - version: {app_version}, environment: {app_env}")

# Configure Bugsnag for error tracking
Expand Down Expand Up @@ -90,14 +94,61 @@ async def global_exception_handler(request: Request, exc: Exception):
content={"detail": "Internal server error"},
)

# allow cors
app.add_middleware(
CORSMiddleware,
allow_origins=["*"],
allow_credentials=True,
allow_methods=["*"],
allow_headers=["*"],
)
# Configure CORS - simplified for local development
local_origins = [
"http://localhost:5175",
"http://127.0.0.1:5175",
"http://localhost:3000",
"http://127.0.0.1:3000"
]

# Create a custom middleware class for handling CORS
class CORSHandlerMiddleware(BaseHTTPMiddleware):
async def dispatch(self, request: Request, call_next):
# Handle OPTIONS preflight requests
if request.method == "OPTIONS":
headers = {
"Access-Control-Allow-Origin": "*",
"Access-Control-Allow-Methods": "GET, POST, PUT, DELETE, OPTIONS, PATCH",
"Access-Control-Allow-Headers": "access_token, Authorization, Content-Type, Accept, X-API-Key",
"Access-Control-Allow-Credentials": "true",
"Access-Control-Max-Age": "600", # Cache preflight for 10 minutes
}

# Set specific origin if in local mode
origin = request.headers.get("origin")
if os.environ.get("ENV_MODE") == "local" and origin:
headers["Access-Control-Allow-Origin"] = origin

return JSONResponse(content={}, status_code=200, headers=headers)

# Process all other requests normally
response = await call_next(request)
return response

# Apply custom CORS middleware BEFORE the standard CORS middleware
app.add_middleware(CORSHandlerMiddleware)

# Standard CORS middleware (as a backup)
if os.environ.get("ENV_MODE") == "local":
logging.info(f"Local mode: using specific CORS origins: {local_origins}")
app.add_middleware(
CORSMiddleware,
allow_origins=local_origins,
allow_credentials=True,
allow_methods=["*"],
allow_headers=["*", "access_token", "Authorization", "Content-Type"],
expose_headers=["*"],
)
else:
# Production mode - use more restrictive CORS
app.add_middleware(
CORSMiddleware,
allow_origins=["*"],
allow_credentials=True,
allow_methods=["*"],
allow_headers=["*", "access_token", "Authorization", "Content-Type"],
)

# Add Bugsnag exception handling middleware
# Important: Add middleware AFTER registering exception handlers
Expand Down Expand Up @@ -137,5 +188,11 @@ async def test_error():
else:
return {"status": "disabled", "message": "Bugsnag is not enabled"}

# Add special route for handling OPTIONS requests to /users/me
@app.options("/users/me", include_in_schema=False)
async def options_users_me():
"""Handle OPTIONS requests to /users/me specifically."""
return {}

# Use the Bugsnag middleware wrapped app for ASGI
app = bugsnag_app
Loading