-
-
Notifications
You must be signed in to change notification settings - Fork 4
feat(friendship)!: redesign and extend friendship management #247
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
base: main
Are you sure you want to change the base?
Conversation
friendshipIn the table `friendship` the columns `a_account_id` and `b_account_id`were renamed to `account_id`and `friend_account_id`, a new column `is_close_friend` was added, the policies were updated. Several friendship related functions were added. Test scripts were updated accordingly.
fe052dc to
aee2cde
Compare
There is a new table `friendship_request` holding a single row per friendship request. If the request is accepted then 2 records will be inserted into table `friendship` and the row in `friendship_request` will be deleted. If the request is rejected then the row in `friendship_request`will also be deleted deleted. This makes the enum type `friendship_status` superfluous and also allows policies on table `friendship` in order to hide information about friendship closeness as set by other users.
If an account A marks account B as a close friend, B can now see this information. An account C cannot select any friendship in which C is not involved.
In the table `friendship` the columns `a_account_id` and `b_account_id`were renamed to `account_id`and `friend_account_id`, a new column `is_close_friend` was added, the policies were updated. Several friendship related functions were added. Test scripts were updated accordingly.
There is a new table `friendship_request` holding a single row per friendship request. If the request is accepted then 2 records will be inserted into table `friendship` and the row in `friendship_request` will be deleted. If the request is rejected then the row in `friendship_request`will also be deleted deleted. This makes the enum type `friendship_status` superfluous and also allows policies on table `friendship` in order to hide information about friendship closeness as set by other users.
If an account A marks account B as a close friend, B can now see this information. An account C cannot select any friendship in which C is not involved.
…eat/friendship-redesign
src/deploy/function_friendship.sql
Outdated
|
|
||
| -- create notification for a request | ||
|
|
||
| CREATE FUNCTION vibetype.friendship_notify_request( |
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.
Do we need a separate function for this? I only see it ever to be called in a single place.
In general: every new function is a new object to maintain regarding access permissions, testing, etc. It can broaden the api surface more than it benefits, which in turn increases security complexity. My aim is to keep the code concise so that any maintainer can feel confident that no unexpected loopholes exist. Each added function or table makes that harder.
For example, what keeps a user from directly inserting into the friendship table right now instead of using friendship_accept?
Before, I could review about 80 lines; now it's 140 plus 180 for the functions. I understand that splitting logic makes each piece easier to digest on its own, but from an auditing perspective, checking 1 table, 3 policies and 1 enum feels more manageable than reviewing 6 functions, 9 policies, and 2 tables; 5 objects vs. 17.
Maybe you can walk me through on Discord which scenarios the previous implementation couldn’t represent (excluding close friends). If the feature set hasn’t changed much, it might be worth sticking with fewer moving parts.
The newly added tests really help though, they'll prevent regressions for coming changes.
| CREATE POLICY friendship_request_select ON vibetype.friendship_request FOR SELECT | ||
| USING ( | ||
| account_id = vibetype.invoker_account_id() | ||
| OR | ||
| friend_account_id = vibetype.invoker_account_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.
It was confirmed that friendships should stay public
Friendships are visible to any account. Only the account that declared closeness to another account can see this information.
Function `friendship.request`reads the language from the account's own contact. The default language is null (in case the value is null in the contact).
In the table
friendshipthe columnsa_account_idandb_account_idwere renamed toaccount_idandfriend_account_id, a new columnis_close_friendwas added,the policies were updated. Several friendship related functions were added.
Test scripts were updated accordingly.