-
Notifications
You must be signed in to change notification settings - Fork 242
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
chore_: Remove old status community from the database #5114
Conversation
Jenkins BuildsClick to see older builds (30)
|
@@ -0,0 +1,3 @@ | |||
DELETE FROM communities_communities WHERE id = X'039b2da47552aa117a96ea8f1d4d108ba66637c7517a3c94a57b99dbb8a002eda2'; | |||
DELETE FROM communities_events WHERE id = X'039b2da47552aa117a96ea8f1d4d108ba66637c7517a3c94a57b99dbb8a002eda2'; | |||
DELETE FROM communities_shards WHERE community_id = X'039b2da47552aa117a96ea8f1d4d108ba66637c7517a3c94a57b99dbb8a002eda2'; |
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.
@Parveshdhull I'm not sure about the validity of my feedback, but to be correct, shouldn't the migration also delete records in the following tables? They all contain a column named community_id
.
activity_center_notifications
applied_community_events
collapsed_community_categories
communities_check_channel_permission_responses
communities_control_node
communities_permission_token_criteria_results
communities_requests_to_leave
community_grants
community_tokens
curated_communities
encrypted_community_description_cache
encrypted_community_description_id_and_clock
encrypted_community_description_missing_keys
profile_showcase_communities_contacts
profile_showcase_communities_preferences
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.
Hi @ilmotta, Thank you very much for your valuable feedback and sharing these tables.
Probably we should also delete from these table too, but I am not sure as DeleteCommunity function only deletes from above three 🤔
status-go/protocol/communities/persistence.go
Line 219 in 9c2c638
func (p *Persistence) DeleteCommunity(id types.HexBytes) error { |
Maybe @cammellos will have more info.
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.
Maybe @cammellos will have more info.
Confirmed. Removing from these 3 tables should be enough.
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.
Great! Thanks for taking the time to check @Parveshdhull
da94812
to
0a8d7c9
Compare
0a8d7c9
to
e50deb2
Compare
closing the PR as removal of status old community is breaking the tests and needs some workaround |
reopen this PR, I'll continue work based on this branch |
835c683
to
551a818
Compare
6ca2ac3
to
01c161a
Compare
01c161a
to
eb7876f
Compare
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.
Thanks for this. It was confusing to always have at least one community in the newly created messenger.
fixes status-im/status-mobile#19868
required for status-im/status-mobile#19905
status: ready