-
Notifications
You must be signed in to change notification settings - Fork 31
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
feat: Project Updates System Schema #147
Conversation
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.
@VyuduInc is attempting to deploy a commit to the kindfi Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis migration script adds support for managing project updates by introducing three new tables: Changes
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
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
Assessment against linked issues
Possibly related issues
Poem
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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;
CREATE POLICY "System can create notifications" | ||
ON project_update_notifications FOR INSERT | ||
TO authenticated | ||
WITH CHECK (true); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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'); |
There was a problem hiding this comment.
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 🤔
There was a problem hiding this comment.
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:
- Allows notifications to be created by our trigger functions
- Prevents direct inserts from the webapp
- 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:
- 1: https://supabase.com/docs/guides/database/overview
- 2: https://supabase.com/docs/guides/database/custom-postgres-config
- 3: https://supabase.com/docs/guides/database/postgres/row-level-security
- 4: https://supabase.com/docs/guides/database/connecting-to-postgres
- 5: https://community.weweb.io/t/best-practices-for-dynamic-pages-multi-organization-and-authentication/8166
- 6: https://supabase.com/docs/guides/api/securing-your-api
- 7: https://supabase.com/database
- 8: https://github.com/orgs/supabase/discussions/9311
- 9: https://supabase.com/blog/supabase-realtime-broadcast-and-presence-authorization
- 10: https://supabase.com/docs/guides/functions/connect-to-postgres
@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:
- Allows notifications from both our trigger functions and system-level operations
- You can call
set_system_operation()
before performing admin operations in the webapp - 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.
-- 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); | ||
|
There was a problem hiding this comment.
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.
-- 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); |
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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!
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() | ||
); |
There was a problem hiding this comment.
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.
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() | |
); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
-- 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); | ||
|
There was a problem hiding this comment.
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.
-- 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); |
There was a problem hiding this comment.
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! 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 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
There was a problem hiding this 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
📒 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
andnext_milestone_date
are thoughtfully placed, with DESC ordering onlast_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.
-- Add comment for documentation | ||
COMMENT ON TABLE projects IS 'Projects table enhanced with update tracking and notification settings'; |
There was a problem hiding this comment.
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.
-- 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.'; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this 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.
-- 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); | ||
|
There was a problem hiding this comment.
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!
CREATE POLICY "System can create notifications" | ||
ON project_update_notifications FOR INSERT | ||
TO authenticated | ||
WITH CHECK (true); | ||
|
There was a problem hiding this comment.
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 🤔
-- 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); | ||
|
There was a problem hiding this comment.
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! 🙏
-- Add comment for documentation | ||
COMMENT ON TABLE projects IS 'Projects table enhanced with update tracking and notification settings'; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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!
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:
Please follow our guide. Raise a new PR with these observations! Additionally, make sure to read the 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. |
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
project_update_comments
project_update_notifications
⚡ Technical Features
🚀 Performance
🔒 Security
⚙️ Automation
🧪 Testing
📝 Notes
Closes #138
Summary by CodeRabbit