Conversation
…hments functionality - Introduced new roles (Technician, Manager) to the default tenant setup. - Created `work_order_time_entries` table for detailed time tracking, replacing the previous integer column. - Established `work_order_attachments` table for managing file attachments related to work orders. - Added RPCs for logging time entries and adding attachments. - Implemented row-level security policies for enhanced data access control.
- Updated authorization tests to ensure proper role assignments for technicians and managers. - Improved work order status transition logic to require admin intervention for certain transitions. - Enhanced test cases to reflect changes in role permissions and status transitions for work orders. - Added comments for clarity on role permissions and status transition requirements in tests.
- Added comprehensive tests for admin-only access to audit views, ensuring that only tenant admins can access audit logs. - Implemented tests to prevent unauthorized access to audit views by members, technicians, and managers. - Introduced tests for tenant isolation in audit logs, confirming that users can only see logs relevant to their tenant. - Enhanced existing tests for role permissions and retention configuration, ensuring proper error handling and access control. - Improved work order status transition tests to cover edge cases and ensure robust permission checks.
…e limiting - Improved ABAC scope tests to clarify expected behavior when accessing internal tables via service clients and RPC functions. - Updated concurrency tests to ensure proper handling of race conditions and verify that only one operation succeeds under rate limiting. - Enhanced input validation tests to confirm that SQL injection attempts are handled correctly, ensuring data integrity. - Refined rate limiting tests to enforce consistent application across all user roles, verifying that limits are respected regardless of permissions.
- Introduced `rpc_grant_scope` and `rpc_revoke_scope` functions to manage ABAC scopes for users within a tenant. - Implemented input validation for scope types and values, ensuring they conform to expected formats and belong to the correct tenant. - Added rate limiting to prevent excessive scope grants and revocations. - Updated tests to utilize the new RPC functions for granting and revoking scopes, ensuring proper permission checks and error handling.
…s. Clean up SQL migration by removing unnecessary view refresh command.
…yping and improved JSONB handling - Added new variables for assigned_to, location_id, and due_date in PM schedule functions. - Ensured explicit typing for variables to facilitate PostgreSQL function signature inference. - Updated the rpc_create_work_order function to accept new parameters and handle JSONB fields correctly. - Created public views for PM template checklist items to comply with ADR patterns, ensuring proper access control and discoverability for PostgREST.
…hecksdeletions, ensuring proper error handling for exceeded limits. - Updated setup scripts to load environment variables for testing, improving configuration management.
…VOKER and updating RLS policies to ensure proper tenant context handling. Added functions for clearing tenant context and auto-setting it during tenant creation. Updated tests to verify tenant context behavior.
…ng, technician roles, dashboard views, maintenance types, meters, and PM system. This cleanup enhances project maintainability by eliminating unused code and ensuring a streamlined migration process.
- Created the `app.profiles` table to store tenant-scoped user profiles, including first name, last name, display name, and avatar URL. - Implemented a function to build full names from first and last names, handling null values gracefully. - Added a profile sync function to update profiles from the `auth.users` table, ensuring data consistency. - Introduced views for tenant-scoped profiles and updated work order and time entry views to include user full names. - Added INSTEAD OF triggers for updatable views to manage updates and deletes correctly.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on March 27
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| -- Check if profile exists and if name is overridden | ||
| select exists(select 1 from app.profiles where user_id = p_user_id and tenant_id = p_tenant_id), | ||
| coalesce((select is_name_overridden from app.profiles where user_id = p_user_id and tenant_id = p_tenant_id), false) | ||
| into v_profile_exists, v_is_overridden; |
There was a problem hiding this comment.
Unused variable performs unnecessary database query
Low Severity
The v_profile_exists variable in util.sync_profile_from_auth_user is declared, assigned via a database exists() query, but never read. Only v_is_overridden from the same SELECT statement is used in the subsequent logic. The computed v_profile_exists value is discarded, resulting in an unnecessary database query execution. The upsert pattern with ON CONFLICT already handles both insert and update cases without needing to check profile existence beforehand.
|
|
||
| if v_user_meta is null then | ||
| return; -- User doesn't exist | ||
| end if; |
There was a problem hiding this comment.
Profile not created when user has null metadata
Medium Severity
The util.sync_profile_from_auth_user function checks if v_user_meta is null then return; to detect if a user doesn't exist. However, in PL/pgSQL, a SELECT INTO sets the variable to NULL both when no rows are found AND when a row is found with a NULL column value. When called from the on_tenant_membership_created trigger, the user is guaranteed to exist (foreign key constraint to auth.users), but if their raw_user_meta_data is NULL, the function returns early without creating a profile. This leaves the database in an inconsistent state where a user has a tenant membership but no corresponding profile record.
| comment on column app.profiles.full_name is 'Computed full name: uses display_name_override if set, otherwise combines first_name and last_name. Stored generated column for performance.'; | ||
| comment on column app.profiles.avatar_url is 'Optional avatar URL for user profile picture. Synced from auth.users.raw_user_meta_data.'; | ||
| comment on column app.profiles.synced_at is 'Timestamp when profile was last synced from auth.users. Used to track sync freshness.'; | ||
| comment on column app.profiles.is_active is 'True if user is currently a member of this tenant. Set to false when membership is removed, but profile is preserved for historical data.'; |
There was a problem hiding this comment.
Missing trigger leaves profiles marked active after membership removal
Medium Severity
The column comment for app.profiles.is_active explicitly states it should be "Set to false when membership is removed, but profile is preserved for historical data." However, only an INSERT trigger (on_tenant_membership_created) exists on app.tenant_memberships. There is no DELETE trigger to set is_active = false on the corresponding profile when a user's membership is removed. This causes profiles to incorrectly remain marked as active after users leave tenants, leading to incorrect data state that could affect queries filtering by is_active or features that depend on accurate membership status.
Additional Locations (1)
1 participant
Summary
Merges develop into main, bringing in the refactored migration layout, public API layer, meters & PM system, enterprise features, and expanded test coverage. This PR represents the consolidated development work since main.
Changes
01_foundation→08_enterprise_featurescore_tables,api_layer,security_hardening02_core_domain,05_work_order_extensions,06_meters_and_pm,07_public_api07_public_api.sql(views/RPCs, naming and versioning)06_meters_and_pm.sql; meter readings, PM schedules, validation and JSONB handling improvementssupabase/config.tomladjustmentsNotes
Note
High Risk
High risk because it rewrites/renumbers core database migrations, changes RLS/auth helper behavior, and introduces new PM logic that hooks into work-order creation/completion paths—any ordering or policy mistake can break auth or data integrity across the app.
Overview
Merges a large migration refactor: removes the legacy
core_tables,api_layer, andsecurity_hardeningmigrations and replaces them with a new ordered set (01_foundation,02_core_domain, updated03_authorization, updated04_workflows) that redefines schemas/functions/policies and shifts RLS patterns toward explicit tenant-membership checks.Adds the meters + preventive maintenance database layer via
06_meters_and_pm.sql, introducing new meter/PM tables, validation triggers, PM scheduling logic in the newpmschema, and awork_orders.pm_schedule_idlinkage (including a stubbedrpc_create_work_ordersignature supporting PM-generated work orders).Also disables Supabase DB seeding in
config.toml(moves to migration-only inserts) and adds a detailed implementation plan document for the meters/PM system under.cursor/plans.Written by Cursor Bugbot for commit ed66ad8. This will update automatically on new commits. Configure here.