Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Project Updates System Schema #147

Closed

Conversation

VyuduInc
Copy link
Collaborator

@VyuduInc VyuduInc commented Feb 11, 2025

Project Updates System - Technical Overview

🎯 Purpose

This PR implements the database schema for the project updates system, enabling structured communication and engagement between project creators and their community.

📊 Core Tables

project_updates

  • Main table storing all project updates
  • Types: milestone/progress/announcement/general
  • States: draft/published/archived
  • Media attachments via JSONB
  • Linked to project and creator

project_update_comments

  • Community responses to updates
  • Linked to update and author
  • Tracks creation and modifications
  • Enables threaded discussions

project_update_notifications

  • Manages notification delivery
  • Creates record per follower on publish
  • Tracks read/unread status
  • Prevents duplicate notifications

⚡ Technical Features

🚀 Performance

  • Strategic database indices
  • Optimized notification delivery
  • Efficient media storage

🔒 Security

  • Row Level Security (RLS)
  • Team-only update creation
  • User-specific notification management
  • Published-only public access

⚙️ Automation

  • Auto-notification on publish
  • Timestamp management
  • Read status tracking

🧪 Testing

  • Migration can be tested by running it against a fresh database
  • RLS policies can be tested by attempting access with different user roles
  • Notification triggers can be tested by publishing updates

📝 Notes

  • Uses UUID for all IDs
  • Implements standard timestamps (created_at, updated_at)
  • All foreign keys have appropriate CASCADE rules
  • Comprehensive RLS policies for security

Closes #138

Summary by CodeRabbit

  • New Features
    • Introduced project update sharing, allowing project progress and details to be communicated effectively.
    • Added automatic notifications that alert users when new updates are posted.
    • Enabled interactive commenting on project updates to foster community engagement.
    • Enhanced project tracking with new statistics and notification settings for better user experience.

I've created a database structure that enables project communication and engagement.

Here's the technical implementation in simple terms:

Core Tables

project_updates

The main table storing all project updates
Each update has a type (milestone/progress/announcement/general)
Updates can be in different states (draft/published/archived)
Media attachments are supported through a flexible JSONB field
Every update is linked to a specific project and creator
project_update_comments
Handles all community responses to updates
Each comment is linked to both its update and author
Includes timestamps for creation and modifications
Enables threaded discussions on project updates
project_update_notifications
Manages the notification delivery system
Creates a record for each follower when an update is published
Tracks whether notifications have been read
Prevents duplicate notifications through unique constraints

Technical Features

Performance Optimization
Added strategic database indices for fast data retrieval
Optimized query performance for notification delivery
Efficient storage of media through JSONB

Security Implementation

Row Level Security (RLS) ensures data access control
Only project team members can create/edit updates
Users can only manage their own notifications
Public access limited to published content only

Automation

Automatic notification creation when updates are published
Timestamp management for content modifications
Built-in system for tracking read/unread status

This implementation provides a robust foundation for project communications while ensuring security, performance, and scalability. It's like a specialized social media system, but specifically designed for project updates and community engagement.
Copy link

vercel bot commented Feb 11, 2025

@VyuduInc is attempting to deploy a commit to the kindfi Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

coderabbitai bot commented Feb 11, 2025

Walkthrough

This migration script adds support for managing project updates by introducing three new tables: project_updates, project_update_notifications, and project_update_comments. It establishes foreign key relationships with existing tables, configures indices for performance, and applies row-level security with defined policies. Additionally, the script creates triggers and associated functions to automatically notify followers upon new updates and to update timestamp fields when records are modified.

Changes

File(s) Change Summary
services/supabase/migrations/20250211_create_project_updates.sql Creates new tables (project_updates, project_update_notifications, project_update_comments) with appropriate foreign keys; adds indices; enables row-level security with access policies; and defines triggers along with functions (notify_project_followers(), update_updated_at_column()) for automatic notifications and timestamp updates.
services/supabase/migrations/20250211_update_projects_table.sql Modifies the projects table by adding new columns (update_count, last_update_at, next_milestone_date, milestone_count, notification_settings, update_categories); creates an index on last_update_at and next_milestone_date; defines a function (update_project_stats()) and a trigger (project_stats_trigger) for automatic project statistics updates.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant DB
    participant Trigger as Project Update Trigger
    participant Notifier as notify_project_followers()

    User->>DB: Insert new project update record
    DB->>Trigger: Fire notification trigger
    Trigger->>Notifier: Execute notification function
    Notifier->>DB: Insert notification into project_update_notifications
Loading
sequenceDiagram
    participant User
    participant DB
    participant TimestampTrigger as Update Timestamp Trigger

    User->>DB: Update project update or comment
    DB->>TimestampTrigger: Fire updated_at trigger
    TimestampTrigger->>DB: Update updated_at timestamp
Loading

Assessment against linked issues

Objective Addressed Explanation
Create project table with all required fields [#138] No modifications to the core project table schema were added.
Establish relationships between projects and project updates [#138] A foreign key from project_updates to projects is defined.
Configure row-level security policies for project-related tables [#138] RLS policies are enabled and defined for all new tables.
Add necessary indices for performance improvements [#138] Indices have been added to the new tables.

Possibly related issues

  • Create Project Updates Table Schema #141: The PR implements the project updates schema, including notifications and comments, which aligns with the objective of tracking project progress and community notifications.

Poem

In the realm of code where tables rise,
Project updates now take to the skies.
Triggers and policies work side by side,
Notifying kindlers far and wide.
Secure and swift, the schema gleams,
Bringing to life our data dreams!
🚀✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🔭 Outside diff range comments (1)
services/supabase/migrations/20250211_create_project_updates.sql (1)

141-159: 🧹 Nitpick (assertive)

Consider enhancing audit tracking.

The timestamp triggers are good, but we could improve audit tracking by also updating the updated_by field and adding an audit log.

 CREATE OR REPLACE FUNCTION update_updated_at_column()
 RETURNS TRIGGER AS $$
 BEGIN
     NEW.updated_at = NOW();
+    NEW.updated_by = auth.uid();
+    
+    -- Log the change in audit table
+    INSERT INTO audit_logs (
+        table_name,
+        record_id,
+        action,
+        old_data,
+        new_data,
+        changed_by
+    ) VALUES (
+        TG_TABLE_NAME,
+        NEW.id,
+        TG_OP,
+        CASE WHEN TG_OP = 'UPDATE' THEN row_to_json(OLD) ELSE NULL END,
+        row_to_json(NEW),
+        auth.uid()
+    );
+    
     RETURN NEW;
 END;
 $$ LANGUAGE plpgsql;
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 19fd0b5 and 9d24ffb.

📒 Files selected for processing (1)
  • services/supabase/migrations/20250211_create_project_updates.sql (1 hunks)

Comment on lines +96 to +100
CREATE POLICY "System can create notifications"
ON project_update_notifications FOR INSERT
TO authenticated
WITH CHECK (true);

Copy link
Contributor

@coderabbitai coderabbitai bot Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Consider restricting system notification creation policy.

The current policy allows any authenticated user to create notifications. We should restrict this to system-level operations only.

-CREATE POLICY "System can create notifications"
+CREATE POLICY "Only system functions can create notifications"
     ON project_update_notifications FOR INSERT
     TO authenticated
-    WITH CHECK (true);
+    WITH CHECK (current_setting('app.is_system_operation', true) = 'true');
📝 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.

Suggested change
CREATE POLICY "System can create notifications"
ON project_update_notifications FOR INSERT
TO authenticated
WITH CHECK (true);
CREATE POLICY "Only system functions can create notifications"
ON project_update_notifications FOR INSERT
TO authenticated
WITH CHECK (current_setting('app.is_system_operation', true) = 'true');

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai good point. These admin system level we want to be able to use it in the webapp. How this would apply if we want to insert into the project update notification? Are you sure that we are applying rules that supabase can hold? I think it must 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

@AndlerRL You raise a good point! Let me suggest a better approach. Instead of using PostgreSQL settings, we can modify the policy to specifically allow notifications created by our database functions/triggers while preventing direct inserts from the webapp:

CREATE POLICY "Only system functions can create notifications"
    ON project_update_notifications FOR INSERT
    TO authenticated
    WITH CHECK (
        -- Allow inserts from our notify_project_followers trigger function
        pg_trigger_depth() > 0
    );

This approach:

  1. Allows notifications to be created by our trigger functions
  2. Prevents direct inserts from the webapp
  3. Is fully supported by Supabase as it uses standard PostgreSQL features

The webapp would create project updates, and our notify_project_followers trigger would automatically handle creating the notifications.

(`・ω・´)ゞ


🌐 Web query:

Does Supabase support PostgreSQL current_setting and set_config for RLS policies?

💡 Result:

Supabase supports PostgreSQL's current_setting and set_config functions for Row Level Security (RLS) policies, enabling advanced authorization workflows by leveraging session variables and request metadata. Here's how it works:


1. Accessing Request Data

You can retrieve authentication tokens, headers, cookies, and other request details using current_setting:

-- Get JWT claims
SELECT current_setting('request.jwt.claims', true)::json->>'sub'; -- User ID [6][8]

-- Access headers/cookies
SELECT current_setting('request.headers', true)::json->>'user-agent'; [6]

2. Storing Session Variables

Use set_config to persist values like workspace IDs or user roles during a session:

-- Set workspace metadata from JWT claims
SELECT set_config(
  'app_metadata.workspace',
  (current_setting('request.jwt.claims', true)::json->'app_metadata'->>'workspace'),
  false -- persists for the transaction
); [5][8]

3. Policy Integration

These functions integrate directly into RLS policies for granular control:

-- Example policy allowing users to access their own data
CREATE POLICY user_data_policy ON my_table FOR SELECT USING (
  created_by = current_setting('request.jwt.claims', true)::json->>'sub'
); [6][8]

Key Use Cases

  • Dynamic Permissions: Derive access rules from JWT claims (e.g., user roles).
  • Multi-Tenancy: Isolate data by workspace/organization via session variables [5].
  • Request Context: Use headers/cookies/IP addresses in access decisions (x-forwarded-for) [6].

Limitations

  • Security: Always validate inputs to prevent SQL injection when using dynamic values.
  • Session Scope: Variables set with set_config persist only for the transaction unless explicitly configured otherwise [8].

For implementation details, see Supabase's documentation on Row Level Security and API security.

Citations:


@AndlerRL I apologize for my previous response. After checking Supabase's capabilities, I can confirm that we can implement this more elegantly! Here's a better approach:

-- First, create a function to set system operation context
CREATE OR REPLACE FUNCTION set_system_operation() RETURNS void AS $$
BEGIN
  PERFORM set_config('app.is_system_operation', 'true', false);
END;
$$ LANGUAGE plpgsql SECURITY DEFINER;

-- Then modify the policy to allow both system operations and trigger-based insertions
CREATE POLICY "System and trigger functions can create notifications"
    ON project_update_notifications FOR INSERT
    TO authenticated
    WITH CHECK (
        -- Allow notifications from system operations or triggers
        (current_setting('app.is_system_operation', true) = 'true')
        OR pg_trigger_depth() > 0
    );

This approach:

  1. Allows notifications from both our trigger functions and system-level operations
  2. You can call set_system_operation() before performing admin operations in the webapp
  3. Is fully supported by Supabase and uses standard PostgreSQL features

For webapp usage:

BEGIN;
SELECT set_system_operation();
-- Your notification insert operations here
COMMIT;

(`・ω・´)ゞ


✏️ Learnings added
Learnt from: AndlerRL
PR: kindfi-org/kindfi#147
File: services/supabase/migrations/20250211_create_project_updates.sql:96-100
Timestamp: 2025-02-12T08:42:41.651Z
Learning: In Supabase, use `pg_trigger_depth() > 0` in RLS policies to restrict operations to trigger functions only, instead of using custom settings.

Learnt from: AndlerRL
PR: kindfi-org/kindfi#147
File: services/supabase/migrations/20250211_create_project_updates.sql:96-100
Timestamp: 2025-02-12T08:42:41.651Z
Learning: Supabase supports PostgreSQL session variables through `set_config` and `current_setting`. Use these for system-level operations by setting transaction-scoped variables.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines +36 to +42
-- Add indices for performance
CREATE INDEX idx_project_updates_project_id ON project_updates(project_id);
CREATE INDEX idx_project_updates_created_at ON project_updates(created_at DESC);
CREATE INDEX idx_project_update_notifications_kindler_id ON project_update_notifications(kindler_id);
CREATE INDEX idx_project_update_notifications_update_id ON project_update_notifications(update_id);
CREATE INDEX idx_project_update_comments_update_id ON project_update_comments(update_id);

Copy link
Contributor

@coderabbitai coderabbitai bot Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider adding indices for common query patterns.

The current indices are good, but we could enhance query performance by adding indices for common filtering and sorting patterns.

 -- Add indices for performance
 CREATE INDEX idx_project_updates_project_id ON project_updates(project_id);
 CREATE INDEX idx_project_updates_created_at ON project_updates(created_at DESC);
+CREATE INDEX idx_project_updates_status_created_at ON project_updates(status, created_at DESC);
+CREATE INDEX idx_project_updates_type_created_at ON project_updates(update_type, created_at DESC);
 CREATE INDEX idx_project_update_notifications_kindler_id ON project_update_notifications(kindler_id);
 CREATE INDEX idx_project_update_notifications_update_id ON project_update_notifications(update_id);
+CREATE INDEX idx_project_update_notifications_unread ON project_update_notifications(kindler_id) WHERE read_at IS NULL;
 CREATE INDEX idx_project_update_comments_update_id ON project_update_comments(update_id);
📝 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.

Suggested change
-- Add indices for performance
CREATE INDEX idx_project_updates_project_id ON project_updates(project_id);
CREATE INDEX idx_project_updates_created_at ON project_updates(created_at DESC);
CREATE INDEX idx_project_update_notifications_kindler_id ON project_update_notifications(kindler_id);
CREATE INDEX idx_project_update_notifications_update_id ON project_update_notifications(update_id);
CREATE INDEX idx_project_update_comments_update_id ON project_update_comments(update_id);
-- Add indices for performance
CREATE INDEX idx_project_updates_project_id ON project_updates(project_id);
CREATE INDEX idx_project_updates_created_at ON project_updates(created_at DESC);
CREATE INDEX idx_project_updates_status_created_at ON project_updates(status, created_at DESC);
CREATE INDEX idx_project_updates_type_created_at ON project_updates(update_type, created_at DESC);
CREATE INDEX idx_project_update_notifications_kindler_id ON project_update_notifications(kindler_id);
CREATE INDEX idx_project_update_notifications_update_id ON project_update_notifications(update_id);
CREATE INDEX idx_project_update_notifications_unread ON project_update_notifications(kindler_id) WHERE read_at IS NULL;
CREATE INDEX idx_project_update_comments_update_id ON project_update_comments(update_id);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VyuduInc we can add this in the new PR!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Comment on lines +27 to +34
CREATE TABLE project_update_comments (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
update_id UUID NOT NULL REFERENCES project_updates(id) ON DELETE CASCADE,
kindler_id UUID NOT NULL REFERENCES kindlers(id),
content TEXT NOT NULL,
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
);
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider enhancing comment functionality with threading support.

The comments table could be improved by adding support for threaded discussions, which was mentioned in the PR objectives.

 CREATE TABLE project_update_comments (
     id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
     update_id UUID NOT NULL REFERENCES project_updates(id) ON DELETE CASCADE,
     kindler_id UUID NOT NULL REFERENCES kindlers(id),
+    parent_id UUID REFERENCES project_update_comments(id) ON DELETE CASCADE,
     content TEXT NOT NULL,
+    content_length INT GENERATED ALWAYS AS (length(content)) STORED,
+    CHECK (content_length BETWEEN 1 AND 5000),
     created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
     updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
 );
📝 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.

Suggested change
CREATE TABLE project_update_comments (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
update_id UUID NOT NULL REFERENCES project_updates(id) ON DELETE CASCADE,
kindler_id UUID NOT NULL REFERENCES kindlers(id),
content TEXT NOT NULL,
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
);
CREATE TABLE project_update_comments (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
update_id UUID NOT NULL REFERENCES project_updates(id) ON DELETE CASCADE,
kindler_id UUID NOT NULL REFERENCES kindlers(id),
parent_id UUID REFERENCES project_update_comments(id) ON DELETE CASCADE,
content TEXT NOT NULL,
content_length INT GENERATED ALWAYS AS (length(content)) STORED,
CHECK (content_length BETWEEN 1 AND 5000),
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW()
);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai good observation! Yes, the comments should be able to be related to another comment so we can create threads based on a parent_comment_id

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndlerRL Thanks for confirming! I'll make sure to remember this requirement for future reviews.


✏️ Learnings added
Learnt from: AndlerRL
PR: kindfi-org/kindfi#147
File: services/supabase/migrations/20250211_create_project_updates.sql:27-34
Timestamp: 2025-02-12T07:57:52.394Z
Learning: In the Kindfi project, comment tables should support threading functionality by including a `parent_comment_id` column that references the parent comment, allowing for nested discussions.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines +43 to +121
-- Add RLS policies
ALTER TABLE project_updates ENABLE ROW LEVEL SECURITY;
ALTER TABLE project_update_notifications ENABLE ROW LEVEL SECURITY;
ALTER TABLE project_update_comments ENABLE ROW LEVEL SECURITY;

-- Project Updates policies
CREATE POLICY "Project updates are viewable by authenticated users"
ON project_updates FOR SELECT
TO authenticated
USING (status = 'published');

CREATE POLICY "Project updates can be created by project team members"
ON project_updates FOR INSERT
TO authenticated
WITH CHECK (
EXISTS (
SELECT 1 FROM projects p
WHERE p.id = project_id
AND (
p.creator_id = auth.uid() OR
EXISTS (
SELECT 1 FROM project_team_members ptm
WHERE ptm.project_id = p.id
AND ptm.kindler_id = auth.uid()
)
)
)
);

CREATE POLICY "Project updates can be modified by project team members"
ON project_updates FOR UPDATE
TO authenticated
USING (
EXISTS (
SELECT 1 FROM projects p
WHERE p.id = project_id
AND (
p.creator_id = auth.uid() OR
EXISTS (
SELECT 1 FROM project_team_members ptm
WHERE ptm.project_id = p.id
AND ptm.kindler_id = auth.uid()
)
)
)
);

-- Notification policies
CREATE POLICY "Users can view their own notifications"
ON project_update_notifications FOR SELECT
TO authenticated
USING (kindler_id = auth.uid());

CREATE POLICY "System can create notifications"
ON project_update_notifications FOR INSERT
TO authenticated
WITH CHECK (true);

CREATE POLICY "Users can mark their own notifications as read"
ON project_update_notifications FOR UPDATE
TO authenticated
USING (kindler_id = auth.uid());

-- Comment policies
CREATE POLICY "Comments are viewable by authenticated users"
ON project_update_comments FOR SELECT
TO authenticated
USING (true);

CREATE POLICY "Authenticated users can create comments"
ON project_update_comments FOR INSERT
TO authenticated
WITH CHECK (auth.uid() = kindler_id);

CREATE POLICY "Users can update their own comments"
ON project_update_comments FOR UPDATE
TO authenticated
USING (auth.uid() = kindler_id);

Copy link
Contributor

@coderabbitai coderabbitai bot Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding deletion policies for complete CRUD coverage.

The RLS policies cover SELECT, INSERT, and UPDATE operations, but deletion policies are missing.

+-- Add deletion policies
+CREATE POLICY "Project updates can be deleted by project team members"
+    ON project_updates FOR DELETE
+    TO authenticated
+    USING (
+        EXISTS (
+            SELECT 1 FROM projects p
+            WHERE p.id = project_id
+            AND p.creator_id = auth.uid()
+        )
+    );
+
+CREATE POLICY "Users can delete their own comments"
+    ON project_update_comments FOR DELETE
+    TO authenticated
+    USING (auth.uid() = kindler_id);
📝 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.

Suggested change
-- Add RLS policies
ALTER TABLE project_updates ENABLE ROW LEVEL SECURITY;
ALTER TABLE project_update_notifications ENABLE ROW LEVEL SECURITY;
ALTER TABLE project_update_comments ENABLE ROW LEVEL SECURITY;
-- Project Updates policies
CREATE POLICY "Project updates are viewable by authenticated users"
ON project_updates FOR SELECT
TO authenticated
USING (status = 'published');
CREATE POLICY "Project updates can be created by project team members"
ON project_updates FOR INSERT
TO authenticated
WITH CHECK (
EXISTS (
SELECT 1 FROM projects p
WHERE p.id = project_id
AND (
p.creator_id = auth.uid() OR
EXISTS (
SELECT 1 FROM project_team_members ptm
WHERE ptm.project_id = p.id
AND ptm.kindler_id = auth.uid()
)
)
)
);
CREATE POLICY "Project updates can be modified by project team members"
ON project_updates FOR UPDATE
TO authenticated
USING (
EXISTS (
SELECT 1 FROM projects p
WHERE p.id = project_id
AND (
p.creator_id = auth.uid() OR
EXISTS (
SELECT 1 FROM project_team_members ptm
WHERE ptm.project_id = p.id
AND ptm.kindler_id = auth.uid()
)
)
)
);
-- Notification policies
CREATE POLICY "Users can view their own notifications"
ON project_update_notifications FOR SELECT
TO authenticated
USING (kindler_id = auth.uid());
CREATE POLICY "System can create notifications"
ON project_update_notifications FOR INSERT
TO authenticated
WITH CHECK (true);
CREATE POLICY "Users can mark their own notifications as read"
ON project_update_notifications FOR UPDATE
TO authenticated
USING (kindler_id = auth.uid());
-- Comment policies
CREATE POLICY "Comments are viewable by authenticated users"
ON project_update_comments FOR SELECT
TO authenticated
USING (true);
CREATE POLICY "Authenticated users can create comments"
ON project_update_comments FOR INSERT
TO authenticated
WITH CHECK (auth.uid() = kindler_id);
CREATE POLICY "Users can update their own comments"
ON project_update_comments FOR UPDATE
TO authenticated
USING (auth.uid() = kindler_id);
-- Add RLS policies
ALTER TABLE project_updates ENABLE ROW LEVEL SECURITY;
ALTER TABLE project_update_notifications ENABLE ROW LEVEL SECURITY;
ALTER TABLE project_update_comments ENABLE ROW LEVEL SECURITY;
-- Project Updates policies
CREATE POLICY "Project updates are viewable by authenticated users"
ON project_updates FOR SELECT
TO authenticated
USING (status = 'published');
CREATE POLICY "Project updates can be created by project team members"
ON project_updates FOR INSERT
TO authenticated
WITH CHECK (
EXISTS (
SELECT 1 FROM projects p
WHERE p.id = project_id
AND (
p.creator_id = auth.uid() OR
EXISTS (
SELECT 1 FROM project_team_members ptm
WHERE ptm.project_id = p.id
AND ptm.kindler_id = auth.uid()
)
)
)
);
CREATE POLICY "Project updates can be modified by project team members"
ON project_updates FOR UPDATE
TO authenticated
USING (
EXISTS (
SELECT 1 FROM projects p
WHERE p.id = project_id
AND (
p.creator_id = auth.uid() OR
EXISTS (
SELECT 1 FROM project_team_members ptm
WHERE ptm.project_id = p.id
AND ptm.kindler_id = auth.uid()
)
)
)
);
-- Notification policies
CREATE POLICY "Users can view their own notifications"
ON project_update_notifications FOR SELECT
TO authenticated
USING (kindler_id = auth.uid());
CREATE POLICY "System can create notifications"
ON project_update_notifications FOR INSERT
TO authenticated
WITH CHECK (true);
CREATE POLICY "Users can mark their own notifications as read"
ON project_update_notifications FOR UPDATE
TO authenticated
USING (kindler_id = auth.uid());
-- Comment policies
CREATE POLICY "Comments are viewable by authenticated users"
ON project_update_comments FOR SELECT
TO authenticated
USING (true);
CREATE POLICY "Authenticated users can create comments"
ON project_update_comments FOR INSERT
TO authenticated
WITH CHECK (auth.uid() = kindler_id);
CREATE POLICY "Users can update their own comments"
ON project_update_comments FOR UPDATE
TO authenticated
USING (auth.uid() = kindler_id);
-- Add deletion policies
CREATE POLICY "Project updates can be deleted by project team members"
ON project_updates FOR DELETE
TO authenticated
USING (
EXISTS (
SELECT 1 FROM projects p
WHERE p.id = project_id
AND p.creator_id = auth.uid()
)
);
CREATE POLICY "Users can delete their own comments"
ON project_update_comments FOR DELETE
TO authenticated
USING (auth.uid() = kindler_id);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai true, it is missing the deletion policies. @VyuduInc can you add the committable suggestion on the new PR? Thanks! 🙏

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndlerRL Thanks for reviewing! Looking forward to seeing the deletion policies implemented. 🚀


If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

- Add update_count and milestone_count for statistics
- Add last_update_at and next_milestone_date for timeline tracking
- Add notification_settings for user preferences
- Add update_categories for allowed update types
- Create trigger to maintain project statistics automatically
- Add indices for performance optimization
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9d24ffb and a351bb6.

📒 Files selected for processing (1)
  • services/supabase/migrations/20250211_update_projects_table.sql (1 hunks)
🔇 Additional comments (2)
services/supabase/migrations/20250211_update_projects_table.sql (2)

10-12: Well-designed indices for common query patterns!

The indices on last_update_at and next_milestone_date are thoughtfully placed, with DESC ordering on last_update_at to optimize recent updates queries.


34-39: Clean and well-structured trigger setup!

The trigger is properly configured with appropriate timing (AFTER INSERT) and the existing trigger is safely dropped before recreation.

Comment on lines +41 to +42
-- Add comment for documentation
COMMENT ON TABLE projects IS 'Projects table enhanced with update tracking and notification settings';
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider enhancing table documentation.

While the current comment is good, we could provide more detailed documentation about the new columns and their purposes.

Consider this enhanced comment:

-COMMENT ON TABLE projects IS 'Projects table enhanced with update tracking and notification settings';
+COMMENT ON TABLE projects IS 'Projects table with update tracking capabilities. Includes counters for updates and milestones, timestamp tracking for updates and milestones, configurable notification preferences, and customizable update categories.';
📝 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.

Suggested change
-- Add comment for documentation
COMMENT ON TABLE projects IS 'Projects table enhanced with update tracking and notification settings';
-- Add comment for documentation
COMMENT ON TABLE projects IS 'Projects table with update tracking capabilities. Includes counters for updates and milestones, timestamp tracking for updates and milestones, configurable notification preferences, and customizable update categories.';

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai good catch! Yes, we have to improve this a little.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AndlerRL Great! The enhanced documentation will make it easier for other developers to understand the table's capabilities.


If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

Copy link

vercel bot commented Feb 12, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
kindfi ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 12, 2025 5:22am

Copy link
Member

@AndlerRL AndlerRL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes looks very-very good! I like how you handled the SQL migrations and added the constrains and index performance 😏 that's PRO! 👍 I see the rules are well applied, some of them a little loose but it is ok for now, since it is secure enough to go though project updates.

I left some @coderabbitai observations that we can take into another PR due I will have to close this one. I will provide more insights about it in a few.

Comment on lines +36 to +42
-- Add indices for performance
CREATE INDEX idx_project_updates_project_id ON project_updates(project_id);
CREATE INDEX idx_project_updates_created_at ON project_updates(created_at DESC);
CREATE INDEX idx_project_update_notifications_kindler_id ON project_update_notifications(kindler_id);
CREATE INDEX idx_project_update_notifications_update_id ON project_update_notifications(update_id);
CREATE INDEX idx_project_update_comments_update_id ON project_update_comments(update_id);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VyuduInc we can add this in the new PR!

Comment on lines +96 to +100
CREATE POLICY "System can create notifications"
ON project_update_notifications FOR INSERT
TO authenticated
WITH CHECK (true);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai good point. These admin system level we want to be able to use it in the webapp. How this would apply if we want to insert into the project update notification? Are you sure that we are applying rules that supabase can hold? I think it must 🤔

Comment on lines +43 to +121
-- Add RLS policies
ALTER TABLE project_updates ENABLE ROW LEVEL SECURITY;
ALTER TABLE project_update_notifications ENABLE ROW LEVEL SECURITY;
ALTER TABLE project_update_comments ENABLE ROW LEVEL SECURITY;

-- Project Updates policies
CREATE POLICY "Project updates are viewable by authenticated users"
ON project_updates FOR SELECT
TO authenticated
USING (status = 'published');

CREATE POLICY "Project updates can be created by project team members"
ON project_updates FOR INSERT
TO authenticated
WITH CHECK (
EXISTS (
SELECT 1 FROM projects p
WHERE p.id = project_id
AND (
p.creator_id = auth.uid() OR
EXISTS (
SELECT 1 FROM project_team_members ptm
WHERE ptm.project_id = p.id
AND ptm.kindler_id = auth.uid()
)
)
)
);

CREATE POLICY "Project updates can be modified by project team members"
ON project_updates FOR UPDATE
TO authenticated
USING (
EXISTS (
SELECT 1 FROM projects p
WHERE p.id = project_id
AND (
p.creator_id = auth.uid() OR
EXISTS (
SELECT 1 FROM project_team_members ptm
WHERE ptm.project_id = p.id
AND ptm.kindler_id = auth.uid()
)
)
)
);

-- Notification policies
CREATE POLICY "Users can view their own notifications"
ON project_update_notifications FOR SELECT
TO authenticated
USING (kindler_id = auth.uid());

CREATE POLICY "System can create notifications"
ON project_update_notifications FOR INSERT
TO authenticated
WITH CHECK (true);

CREATE POLICY "Users can mark their own notifications as read"
ON project_update_notifications FOR UPDATE
TO authenticated
USING (kindler_id = auth.uid());

-- Comment policies
CREATE POLICY "Comments are viewable by authenticated users"
ON project_update_comments FOR SELECT
TO authenticated
USING (true);

CREATE POLICY "Authenticated users can create comments"
ON project_update_comments FOR INSERT
TO authenticated
WITH CHECK (auth.uid() = kindler_id);

CREATE POLICY "Users can update their own comments"
ON project_update_comments FOR UPDATE
TO authenticated
USING (auth.uid() = kindler_id);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai true, it is missing the deletion policies. @VyuduInc can you add the committable suggestion on the new PR? Thanks! 🙏

Comment on lines +41 to +42
-- Add comment for documentation
COMMENT ON TABLE projects IS 'Projects table enhanced with update tracking and notification settings';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai good catch! Yes, we have to improve this a little.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not 100% sure if this is OK to add it, mostly because this table creation is at another issue, which is already assigned. I see that you linked the issue #138 which is fine however, you have to notify and sync with @Benjtalkshow since he will be doing the rest of the table. I see that you added only what it is related to your task, which is very-very good! 👍 only the coordination and communication is what worries me.

What also worries me is the sequence of these migrations. Did supabase provided that date for the migrations? I thought it provides the date in miliseconds, I see a year, month and day 👀 that could lead into issues in future!

@AndlerRL
Copy link
Member

Hey @VyuduInc I like the changes that you did in this PR, they look good to me however, I would have to close this PR for various reasons:

  • We did not follow the "How to contribute" guide: There were various points that lead into this conclusion.
    • No signed commits found.
    • Supabase wasn't started in the local development.
    • Supabase types wasn't added in this PR.
    • The PR is from a Fork and not directly from a branch. This seems logic, I just added you as contributor. Please check 👀
    • The supabase sequence looks different from traditional sequence. The sequence are in miliseconds i.e.: 1739349898099_alter_project_updates. This is automated and it is with milisecond to enhance accuracy in the migration sequence.

image

Please follow our guide. Raise a new PR with these observations! Additionally, make sure to read the README.md files in every app/service that are within the monorepo and that are related with your task.

Anything please let me know! 🙌

cc - @Bran18

@AndlerRL AndlerRL closed this Feb 12, 2025
@AndlerRL
Copy link
Member

Hey @VyuduInc I like the changes that you did in this PR, they look good to me however, I would have to close this PR for various reasons:

  • We did not follow the "How to contribute" guide: There were various points that lead into this conclusion.

    • No signed commits found.
    • Supabase wasn't started in the local development.
    • Supabase types wasn't added in this PR.
    • The PR is from a Fork and not directly from a branch. This seems logic, I just added you as contributor. Please check 👀
    • The supabase sequence looks different from traditional sequence. The sequence are in miliseconds i.e.: 1739349898099_alter_project_updates. This is automated and it is with milisecond to enhance accuracy in the migration sequence.

image

Please follow our guide. Raise a new PR with these observations! Additionally, make sure to read the README.md files in every app/service that are within the monorepo and that are related with your task.

Anything please let me know! 🙌

cc - @Bran18

We want also to be directly from the branch because there is a CI/CD from supabase that triggers when we open a PR to develop. This is for checking the code integrity and deployment with the migration sequence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Project Table Schema in Supabase
2 participants