Skip to content

Commit

Permalink
Remove redundant rules code (#4430)
Browse files Browse the repository at this point in the history
  • Loading branch information
megrogan authored Sep 25, 2023
1 parent ed6a381 commit 3704d25
Show file tree
Hide file tree
Showing 29 changed files with 40 additions and 110 deletions.
3 changes: 0 additions & 3 deletions backend/canisters/community/api/can.did
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ type SelectedChannelInitialResponse = variant {
blocked_users : vec UserId;
invited_users : vec UserId;
pinned_messages : vec MessageIndex;
rules : Rules;
chat_rules : VersionedRules;
};
PrivateCommunity;
Expand Down Expand Up @@ -176,7 +175,6 @@ type SelectedInitialSuccess = record {
members : vec CommunityMember;
blocked_users : vec UserId;
invited_users : vec UserId;
rules : Rules;
chat_rules : VersionedRules;
user_groups : vec UserGroupDetails;
};
Expand Down Expand Up @@ -205,7 +203,6 @@ type SelectedUpdatesSuccess = record {
blocked_users_added : vec UserId;
blocked_users_removed : vec UserId;
invited_users : opt vec UserId;
rules : opt Rules;
chat_rules : opt VersionedRules;
user_groups : vec UserGroupDetails;
user_groups_deleted : vec nat32;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use candid::CandidType;
use serde::{Deserialize, Serialize};
use types::{ChannelId, EventIndex, GroupMember, MessageIndex, Rules, TimestampMillis, UserId, VersionedRules};
use types::{ChannelId, EventIndex, GroupMember, MessageIndex, TimestampMillis, UserId, VersionedRules};

#[derive(CandidType, Serialize, Deserialize, Debug)]
pub struct Args {
Expand All @@ -23,7 +23,5 @@ pub struct SuccessResult {
pub blocked_users: Vec<UserId>,
pub invited_users: Vec<UserId>,
pub pinned_messages: Vec<MessageIndex>,
// TODO: remove this field once the website is using `chat_rules` instead
pub rules: Rules,
pub chat_rules: VersionedRules,
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use candid::CandidType;
use serde::{Deserialize, Serialize};
use types::{CommunityMember, EventIndex, Rules, TimestampMillis, UserGroupDetails, UserId, VersionedRules};
use types::{CommunityMember, EventIndex, TimestampMillis, UserGroupDetails, UserId, VersionedRules};

#[derive(CandidType, Serialize, Deserialize, Debug)]
pub struct Args {
Expand All @@ -20,8 +20,6 @@ pub struct SuccessResult {
pub members: Vec<CommunityMember>,
pub blocked_users: Vec<UserId>,
pub invited_users: Vec<UserId>,
// TODO: remove this field once the website is using `chat_rules` instead
pub rules: Rules,
pub chat_rules: VersionedRules,
pub user_groups: Vec<UserGroupDetails>,
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use candid::CandidType;
use serde::{Deserialize, Serialize};
use types::{CommunityMember, Rules, TimestampMillis, UserGroupDetails, UserId, VersionedRules};
use types::{CommunityMember, TimestampMillis, UserGroupDetails, UserId, VersionedRules};

#[derive(CandidType, Serialize, Deserialize, Debug)]
pub struct Args {
Expand All @@ -24,8 +24,6 @@ pub struct SuccessResult {
pub blocked_users_added: Vec<UserId>,
pub blocked_users_removed: Vec<UserId>,
pub invited_users: Option<Vec<UserId>>,
// TODO: remove this field once the website is using `chat_rules` instead
pub rules: Option<Rules>,
pub chat_rules: Option<VersionedRules>,
pub user_groups: Vec<UserGroupDetails>,
pub user_groups_deleted: Vec<u32>,
Expand All @@ -38,6 +36,6 @@ impl SuccessResult {
|| !self.blocked_users_added.is_empty()
|| !self.blocked_users_removed.is_empty()
|| self.invited_users.is_some()
|| self.rules.is_some()
|| self.chat_rules.is_some()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ fn selected_channel_initial_impl(args: Args, state: &RuntimeState) -> Response {
blocked_users: chat.members.blocked(),
invited_users: chat.invited_users.users(),
pinned_messages: chat.pinned_messages.clone(),
rules: chat.rules.clone().into(),
chat_rules: chat.rules.clone().into(),
})
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ fn selected_initial_impl(args: Args, state: &RuntimeState) -> Response {
members: members.iter().map(|m| m.into()).collect(),
blocked_users: members.blocked(),
invited_users: state.data.invited_users.users(),
rules: state.data.rules.clone().into(),
chat_rules: state.data.rules.clone().into(),
user_groups: state.data.members.iter_user_groups().map(|u| u.into()).collect(),
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ fn selected_updates_impl(args: Args, state: &RuntimeState) -> Response {
blocked_users_added: vec![],
blocked_users_removed: vec![],
invited_users,
rules: None,
chat_rules: None,
user_groups: data
.members
Expand Down Expand Up @@ -105,9 +104,6 @@ fn selected_updates_impl(args: Args, state: &RuntimeState) -> Response {
}
}
CommunityEventInternal::RulesChanged(_) => {
if result.rules.is_none() {
result.rules = Some(data.rules.clone().into());
}
if result.chat_rules.is_none() {
result.chat_rules = Some(data.rules.clone().into());
}
Expand Down
1 change: 0 additions & 1 deletion backend/canisters/group/api/can.did
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,6 @@ type SelectedInitialSuccess = record {
blocked_users : vec UserId;
invited_users : vec UserId;
pinned_messages : vec MessageIndex;
rules : Rules;
chat_rules : VersionedRules;
};

Expand Down
4 changes: 1 addition & 3 deletions backend/canisters/group/api/src/queries/selected_initial.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use candid::CandidType;
use serde::{Deserialize, Serialize};
use types::{Empty, EventIndex, GroupMember, MessageIndex, Rules, TimestampMillis, UserId, VersionedRules};
use types::{Empty, EventIndex, GroupMember, MessageIndex, TimestampMillis, UserId, VersionedRules};

pub type Args = Empty;

Expand All @@ -18,7 +18,5 @@ pub struct SuccessResult {
pub blocked_users: Vec<UserId>,
pub invited_users: Vec<UserId>,
pub pinned_messages: Vec<MessageIndex>,
// TODO: remove this field once the website is using `chat_rules` instead
pub rules: Rules,
pub chat_rules: VersionedRules,
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ fn selected_initial_impl(state: &RuntimeState) -> Response {
.filter(|&m| *m >= min_visible_message_index)
.copied()
.collect(),
rules: chat.rules.clone().into(),
chat_rules: chat.rules.clone().into(),
})
} else {
Expand Down
10 changes: 0 additions & 10 deletions backend/canisters/user_index/impl/src/queries/users.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,6 @@ use crate::{read_state, RuntimeState};
use ic_cdk_macros::query;
use user_index_canister::users_v2::{Response::*, *};

#[query]
fn users(args: user_index_canister::users::Args) -> user_index_canister::users::Response {
let Success(response) = read_state(|state| users_impl(args, state));

user_index_canister::users::Response::Success(user_index_canister::users::Result {
users: response.users.into_iter().map(|summary| summary.into()).collect(),
timestamp: response.timestamp,
})
}

#[query]
fn users_v2(args: Args) -> Response {
read_state(|state| users_impl(args, state))
Expand Down
63 changes: 33 additions & 30 deletions backend/integration_tests/src/communities/send_message_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,14 @@ fn send_message_with_community_rules_not_accepted_fails() {

set_community_rules(env, &user1, community_id, "No heavy petting".to_string());

let _response = send_dummy_message_with_rules(env, &user2, community_id, channel_id, None, None);
let response = send_dummy_message_with_rules(env, &user2, community_id, channel_id, None, None);

// TODO: Re-enable check once RulesNotAccepted is returned
// if !matches!(response, community_canister::send_message::Response::RulesNotAccepted) {
// panic!("{response:?}");
// }
if !matches!(
response,
community_canister::send_message::Response::CommunityRulesNotAccepted
) {
panic!("{response:?}");
}
}

#[test]
Expand All @@ -151,12 +153,11 @@ fn send_message_with_channel_rules_not_accepted_fails() {

set_channel_rules(env, &user1, community_id, channel_id, "No running".to_string());

let _response = send_dummy_message_with_rules(env, &user2, community_id, channel_id, None, None);
let response = send_dummy_message_with_rules(env, &user2, community_id, channel_id, None, None);

// TODO: Re-enable check once RulesNotAccepted is returned
// if !matches!(response, community_canister::send_message::Response::RulesNotAccepted) {
// panic!("{response:?}");
// }
if !matches!(response, community_canister::send_message::Response::RulesNotAccepted) {
panic!("{response:?}");
}
}

#[test]
Expand Down Expand Up @@ -234,12 +235,11 @@ fn send_message_with_community_rules_but_not_channel_rules_accepted_fails() {
set_community_rules(env, &user1, community_id, "No heavy petting".to_string());
set_channel_rules(env, &user1, community_id, channel_id, "No running".to_string());

let _response = send_dummy_message_with_rules(env, &user2, community_id, channel_id, Some(Version::from(1)), None);
let response = send_dummy_message_with_rules(env, &user2, community_id, channel_id, Some(Version::from(1)), None);

// TODO: Re-enable check once RulesNotAccepted is returned
// if !matches!(response, community_canister::send_message::Response::RulesNotAccepted) {
// panic!("{response:?}");
// }
if !matches!(response, community_canister::send_message::Response::RulesNotAccepted) {
panic!("{response:?}");
}
}

#[test]
Expand All @@ -263,12 +263,14 @@ fn send_message_with_channel_rules_but_not_community_rules_accepted_fails() {
set_community_rules(env, &user1, community_id, "No heavy petting".to_string());
set_channel_rules(env, &user1, community_id, channel_id, "No running".to_string());

let _response = send_dummy_message_with_rules(env, &user2, community_id, channel_id, None, Some(Version::from(1)));
let response = send_dummy_message_with_rules(env, &user2, community_id, channel_id, None, Some(Version::from(1)));

// TODO: Re-enable check once RulesNotAccepted is returned
// if !matches!(response, community_canister::send_message::Response::RulesNotAccepted) {
// panic!("{response:?}");
// }
if !matches!(
response,
community_canister::send_message::Response::CommunityRulesNotAccepted
) {
panic!("{response:?}");
}
}

#[test]
Expand Down Expand Up @@ -364,7 +366,7 @@ fn send_message_with_old_community_rules_accepted_fails() {
set_community_rules(env, &user1, community_id, "No heavy petting".to_string());
set_community_rules(env, &user1, community_id, "No heavy petting or pets".to_string());

let _response = send_dummy_message_with_rules(
let response = send_dummy_message_with_rules(
env,
&user2,
community_id,
Expand All @@ -373,10 +375,12 @@ fn send_message_with_old_community_rules_accepted_fails() {
Some(Version::from(1)),
);

// TODO: Re-enable check once RulesNotAccepted is returned
// if !matches!(response, community_canister::send_message::Response::RulesNotAccepted) {
// panic!("{response:?}");
// }
if !matches!(
response,
community_canister::send_message::Response::CommunityRulesNotAccepted
) {
panic!("{response:?}");
}
}

#[test]
Expand All @@ -400,12 +404,11 @@ fn send_message_with_old_channel_rules_accepted_fails() {
set_channel_rules(env, &user1, community_id, channel_id, "No running".to_string());
set_channel_rules(env, &user1, community_id, channel_id, "No running or jumping".to_string());

let _response = send_dummy_message_with_rules(env, &user2, community_id, channel_id, None, Some(Version::from(1)));
let response = send_dummy_message_with_rules(env, &user2, community_id, channel_id, None, Some(Version::from(1)));

// TODO: Re-enable check once RulesNotAccepted is returned
// if !matches!(response, community_canister::send_message::Response::RulesNotAccepted) {
// panic!("{response:?}");
// }
if !matches!(response, community_canister::send_message::Response::RulesNotAccepted) {
panic!("{response:?}");
}
}

#[test]
Expand Down
3 changes: 1 addition & 2 deletions backend/libraries/group_chat_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,8 +378,7 @@ impl GroupChatCore {
}
}
ChatEventInternal::GroupRulesChanged(_) => {
if result.rules.is_none() {
result.rules = Some(self.rules.clone().into());
if result.chat_rules.is_none() {
result.chat_rules = Some(self.rules.clone().into());
}
}
Expand Down
1 change: 0 additions & 1 deletion backend/libraries/types/can.did
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,6 @@ type SelectedGroupUpdates = record {
invited_users : opt vec UserId;
pinned_messages_added : vec MessageIndex;
pinned_messages_removed : vec MessageIndex;
rules : opt Rules;
chat_rules : opt VersionedRules;
};

Expand Down
4 changes: 1 addition & 3 deletions backend/libraries/types/src/chat_summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,6 @@ pub struct SelectedGroupUpdates {
pub invited_users: Option<Vec<UserId>>,
pub pinned_messages_added: Vec<MessageIndex>,
pub pinned_messages_removed: Vec<MessageIndex>,
// TODO: remove this field once the website is using `chat_rules` instead
pub rules: Option<Rules>,
pub chat_rules: Option<VersionedRules>,
}

Expand All @@ -259,7 +257,7 @@ impl SelectedGroupUpdates {
|| self.invited_users.is_some()
|| !self.pinned_messages_added.is_empty()
|| !self.pinned_messages_removed.is_empty()
|| self.rules.is_some()
|| self.chat_rules.is_some()
}
}

Expand Down
6 changes: 0 additions & 6 deletions backend/libraries/types/src/group_roles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ pub struct GroupPermissions {
pub send_messages: GroupPermissionRole,
pub react_to_messages: GroupPermissionRole,
pub reply_in_thread: GroupPermissionRole,
// TODO: Remove this field once communities and groups have been released
#[serde(default = "group_permission_role_admin")]
pub mention_all_members: GroupPermissionRole,
}

Expand Down Expand Up @@ -87,7 +85,3 @@ pub enum GroupPermissionRole {
fn group_permission_role_owner() -> GroupPermissionRole {
GroupPermissionRole::Owner
}

fn group_permission_role_admin() -> GroupPermissionRole {
GroupPermissionRole::Admins
}
14 changes: 0 additions & 14 deletions backend/libraries/types/src/user_summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,3 @@ pub struct PartialUserSummary {
pub suspended: bool,
pub diamond_member: bool,
}

// TODO: Remove this once the website is using users_v2
impl From<UserSummary> for PartialUserSummary {
fn from(summary: UserSummary) -> Self {
PartialUserSummary {
user_id: summary.user_id,
username: Some(summary.username),
avatar_id: summary.avatar_id,
is_bot: summary.is_bot,
suspended: summary.suspended,
diamond_member: summary.diamond_member,
}
}
}
4 changes: 0 additions & 4 deletions frontend/openchat-agent/src/services/community/candid/idl.js
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,6 @@ export const idlFactory = ({ IDL }) => {
'timestamp' : TimestampMillis,
'pinned_messages' : IDL.Vec(MessageIndex),
'latest_event_index' : EventIndex,
'rules' : Rules,
}),
'PrivateCommunity' : IDL.Null,
'PrivateChannel' : IDL.Null,
Expand All @@ -1257,7 +1256,6 @@ export const idlFactory = ({ IDL }) => {
'members_removed' : IDL.Vec(UserId),
'timestamp' : TimestampMillis,
'latest_event_index' : EventIndex,
'rules' : IDL.Opt(Rules),
'blocked_users_added' : IDL.Vec(UserId),
});
const SelectedChannelUpdatesResponse = IDL.Variant({
Expand Down Expand Up @@ -1296,7 +1294,6 @@ export const idlFactory = ({ IDL }) => {
'user_groups' : IDL.Vec(UserGroupDetails),
'timestamp' : TimestampMillis,
'latest_event_index' : EventIndex,
'rules' : Rules,
});
const SelectedInitialResponse = IDL.Variant({
'Success' : SelectedInitialSuccess,
Expand All @@ -1315,7 +1312,6 @@ export const idlFactory = ({ IDL }) => {
'user_groups' : IDL.Vec(UserGroupDetails),
'members_removed' : IDL.Vec(UserId),
'timestamp' : TimestampMillis,
'rules' : IDL.Opt(Rules),
'blocked_users_added' : IDL.Vec(UserId),
});
const SelectedUpdatesResponse = IDL.Variant({
Expand Down
Loading

0 comments on commit 3704d25

Please sign in to comment.