Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
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