-
Notifications
You must be signed in to change notification settings - Fork 2
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
add helper_parties and helper_party_network tables #36
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce new tables for managing helper parties, networks, and their members in the Supabase backend. These modifications include defining row structures, insert and update types, and establishing relationships with foreign key constraints. Additionally, row-level security policies are implemented to control access to these tables, ensuring only authenticated users can perform select, insert, and update operations. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- server/data/supabaseTypes.ts (1 hunks)
- server/supabase/migrations/20240608204813_helper_party.sql (1 hunks)
- server/supabase/seed.sql (1 hunks)
Files skipped from review due to trivial changes (1)
- server/supabase/seed.sql
Additional comments not posted (6)
server/supabase/migrations/20240608204813_helper_party.sql (3)
1-24
: The table creation and policies forhelper_parties
are well-defined, ensuring proper access control and data integrity.
26-50
: The table creation and policies forhelper_party_networks
are correctly implemented, ensuring data integrity and secure access.
52-76
: The creation ofhelper_party_network_members
and its policies are correctly set up, ensuring relational integrity and secure data access.server/data/supabaseTypes.ts (3)
37-54
: The TypeScript definitions forhelper_parties
are comprehensive and align well with the SQL table structure.
55-87
: The TypeScript definitions forhelper_party_network_members
are detailed and correctly reflect the relationships and data structure.
88-108
: The TypeScript definitions forhelper_party_networks
are well-structured and ensure type safety and consistency with the database schema.
helper_party_networks ( | ||
uuid uuid default gen_random_uuid() primary key, | ||
display_name varchar(255) unique not null, | ||
size smallint not null, |
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.
It feels to me that an enum will be more appropriate here - you may want to support semi-honest/malicious 2 party, TEE, etc.. Size will require a migration in the future
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.
yea, that makes sense. at this point, I don't think this is used so maybe it makes sense to just drop it until we need it.
helper_parties ( | ||
uuid uuid default gen_random_uuid() primary key, | ||
display_name varchar(255) unique not null, | ||
created_at timestamp default current_timestamp not null |
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 standard way is to define both created_at
, modified_at
and modified_by
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.
adding modified_at
. I'll figure out modified_by
(and I suppose created_by
) later.
helper_party_uuid: string | ||
} | ||
Update: { | ||
created_at?: string |
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.
what does this mean for an update operation if someone specifies created_at
?
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.
this is an autogenerated file, generating types for the tables. I don't believe there is a way at the DB level to assure that created_at
isn't modified.
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.
yea unless this generates a JS API for DB, it is not possible to prevent that
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: CHILL
Files selected for processing (3)
- server/data/supabaseTypes.ts (1 hunks)
- server/supabase/migrations/20240608204813_helper_party.sql (1 hunks)
- server/supabase/seed.sql (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- server/data/supabaseTypes.ts
- server/supabase/seed.sql
alter table helper_parties enable row level security; | ||
|
||
create policy "Helper Parties are visible to authenticated users" | ||
on helper_parties for select | ||
to authenticated | ||
using ( true ); | ||
|
||
create policy "Helper Parties are only created by authenticated users" | ||
on helper_parties for insert | ||
to authenticated | ||
with check ( true ); | ||
|
||
create policy "Helper Parties are only updated by authenticated users" | ||
on helper_parties for update | ||
to authenticated | ||
using ( true ) | ||
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 refining the policy conditions for better security.
The policies for helper_parties
use a simple true
condition for select, insert, and update operations. This might not be secure enough as it allows any authenticated user to perform these operations. Consider adding more specific conditions based on user roles or attributes.
alter table helper_party_networks enable row level security; | ||
|
||
create policy "Helper Party Networks are visible to authenticated users" | ||
on helper_party_networks for select | ||
to authenticated | ||
using ( true ); | ||
|
||
create policy "Helper Party Networks are only created by authenticated users" | ||
on helper_party_networks for insert | ||
to authenticated | ||
with check ( true ); | ||
|
||
create policy "Helper Party Networks are only updated by authenticated users" | ||
on helper_party_networks for update | ||
to authenticated | ||
using ( true ) | ||
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 refining the policy conditions for better security.
The policies for helper_party_networks
also use a simple true
condition for select, insert, and update operations. This might expose sensitive network data to any authenticated user. Consider adding more specific conditions based on user roles or attributes.
alter table helper_party_network_members enable row level security; | ||
|
||
create policy "Helper Party Network Members are visible to authenticated users" | ||
on helper_party_network_members for select | ||
to authenticated | ||
using ( true ); | ||
|
||
create policy "Helper Party Network Members are only created by authenticated users" | ||
on helper_party_network_members for insert | ||
to authenticated | ||
with check ( true ); | ||
|
||
create policy "Helper Party Network Members are only updated by authenticated users" | ||
on helper_party_network_members for update | ||
to authenticated | ||
using ( true ) | ||
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 refining the policy conditions for better security.
The policies for helper_party_network_members
use a simple true
condition for select, insert, and update operations. This might not be secure enough as it allows any authenticated user to perform these operations on sensitive linkage data. Consider adding more specific conditions based on user roles or attributes.
this creates 3 new tables:
changes to
server/data/supabaseTypes.ts
are autogenerated, and the seed data is just for testing.Summary by CodeRabbit