Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork520
core: forward based on relations vector#6464
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:member-status-vector
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| withStore' (\db-> setEmptyVectorForGroupId db gId)`catchAllErrors` eToView | ||
| liftIO$ threadDelay' stepDelay | ||
| migrateMembers=flip catchAllErrors eToView$do | ||
| lift waitChatStartedAndActivated |
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 already waited before, why twice?
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's a loop
| introducedRelations<- withStore'$\db-> getIntroducedRelations db gmId | ||
| introducedToRelations<- withStore'$\db-> getIntroducedToRelations db gmId | ||
| connectedRelations<- withStore'$\db-> getIntroConnectedRelations db gmId |
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.
why these are three queries and not one?
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 would try to pack it all in a single query avoiding the need to read members completely, as the change is not transactional.
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.
and we will need to make this migration anyway, so maybe we should start from it, and here use the same function just for one member.
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's inside member lock, logic mimicks forwarding logic
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| getIntroducedToRelations :: DB.Connection -> GroupMemberId -> IO [(Int64, MemberRelation)] | ||
| getIntroducedToRelations db gmId = | ||
| map ((,MRIntroducedTo) . fromOnly) <$> |
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.
if you read it separately anyway, then why map here?
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.
to avoid additional maps later
| JOIN group_members m ON m.group_member_id = i.to_group_member_id | ||
| WHERE i.re_group_member_id = ? AND i.intro_status IN (?,?,?) | ||
| |] | ||
| (gmId, GMIntroReConnected, GMIntroToConnected, GMIntroConnected) |
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.
why do they need to be two queries and not one query with OR in join?
JOIN group_members m ON m.group_member_id = i.re_group_member_id OR m.group_member_id = i.to_group_member_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.
and why map with status
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.
- because filter is also different
- to avoid additional maps later
| when (isJust sendingMemVec)$ | ||
| withGroupMemberLock"xGrpMemCon, sending member" (groupMemberId' sendingMem)$do | ||
| vec<- withStore$\db-> getMemberRelationsVector db sendingMem | ||
| let vec'= setMemberRelation (indexInGroup refMem)MRConnected vec | ||
| withStore'$\db-> updateMemberRelationsVector db sendingMem vec' |
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 should be a function, and most likely it can be moved inside sql
Uh oh!
There was an error while loading.Please reload this page.
| getMemberRelationsIndexes:: (MemberRelation->Bool)->MemberRelationsVector-> [Int64] | ||
| getMemberRelationsIndexes p (MemberRelationsVector v)= getRelationsIndexes p v | ||
| {-#INLINE getMemberRelationsIndexes #-} | ||
| setMemberRelation::Int64->MemberRelation->MemberRelationsVector->MemberRelationsVector | ||
| setMemberRelation i r (MemberRelationsVector v)=MemberRelationsVector$ setRelation i r v | ||
| {-#INLINE setMemberRelation #-} | ||
| setMemberRelations:: [(Int64,MemberRelation)]->MemberRelationsVector->MemberRelationsVector | ||
| setMemberRelations relations (MemberRelationsVector v)=MemberRelationsVector$ setRelations relations v | ||
| {-#INLINE setMemberRelations #-} |
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 unnecessary (same as newtype). If this is to encode it into JSON, I would probably just drop it from JSON entirely (via aeson options), it is not needed outside of Haskell. Also worth revising if this relation is used in all cases that load members, and if most places don't use it (which seems likely), then this should not be part of member record at all.
| FROM group_members mu | ||
| WHERE mu.member_category = 'user' | ||
| AND ( | ||
| mu.member_role NOT IN (CAST('admin' AS BLOB), CAST('owner' AS BLOB)) |
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.
member_role is written as BLOB (even though it's text)
No description provided.