Skip to content
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

feat : adding channel contract and tests #120

Closed
wants to merge 0 commits into from

Conversation

PavitraAgarwal21
Copy link
Contributor

No description provided.

@PavitraAgarwal21 PavitraAgarwal21 marked this pull request as ready for review October 4, 2024 09:44
channels: Map<u256, channelParams>,
channel_counter: u256,
channel_members: Map<ContractAddress, channelMember>,
channel_moderators: Map<(u256, ContractAddress), bool>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be a mapping of channel_id -> vec<Moderator addresses>

so it's easier to retrieve all associated moderators for a channel

token_id: u256,
block_timestamp: u64,
}
// this should also contain the IChannel & IChannelNFT implmentation , waht is the self what d.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you throw more light on what this comment means?

}


// mnaking the constructor to store the onwer , who can set the moderators
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we do not need a constructor to do this

// check that i prioor not baned
let channel_member: channelMember = self.channel_members.read(get_caller_address());
assert(!channel_member.ban_status, BANNED_FROM_CHANNEL);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we also need an extra check to ensure that the user is a member of the community, the channel belongs to. You shouldn't be able to join a channel of a community you do not belong to.

To do this, you need to inherit the community component and check the user's membership

// * @param channel_premium_status
// */
#[derive(Drop, Serde, Clone, starknet::Store)]
pub struct channelParams {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should also contain the community_id of the community the channel belongs to

src/channel/channel.cairo Outdated Show resolved Hide resolved
self.channels.read(channel_id).channel_owner == get_caller_address(),
NOT_CHANNEL_OWNER
);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you might also want to check that the address is an existing moderator

self.channels.read(channel_id).channel_owner == get_caller_address()
|| self.channel_moderators.read((channel_id, get_caller_address())),
NOT_CHANNEL_MODERATOR
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check that profile being banned exists

fn set_ban_status(
ref self: TState, channel_id: u256, profile: ContractAddress, ban_status: bool
);
// // *************************************************************************
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this have double starting comments?

tests/test_channel.cairo Outdated Show resolved Hide resolved
channel_nft_address: channel_params.channel_nft_address,
channel_total_members: 0,
channel_censorship: channel_params.channel_censorship,
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deploy channel nft when a new channel is created. make reference to the community component implementation

channel_total_members: 0,
channel_censorship: channel_params.channel_censorship,
};
// increment
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add channel owner as the first member of the channel

let channel_member: channelMember = self
.channel_members
.read((channel_id, get_caller_address()));
assert(!channel_member.ban_status, BANNED_FROM_CHANNEL);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also check that profile is not previously a member of the channel

.channel_members
.read((channel_id, get_caller_address()));
assert(!channel_member.ban_status, BANNED_FROM_CHANNEL);
let mut channel: channelParams = self.channels.read(channel_id);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mint a channel token to the new member

.channel_members
.write(
(channel_id, get_caller_address()),
channelMember {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instantiate a new struct outside and pass in as a parameter. much cleaner that way

// not owner of the channel
#[test]
#[should_panic(expected: ('Channel: not channel owner',))]
fn test_set_channel_censorship_status_not_owner() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to test_only_owner_can_set_censorship_status


// test for set_ban_status
#[test]
fn test_set_ban_status_owner() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to simply test_set_ban_status


#[test]
#[should_panic(expected: ('Karst : Unauthorized access',))]
fn test_set_ban_status_owner_or_moderator() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to test_only_owner_or_mod_can_set_ban_status


#[test]
#[should_panic(expected: ('Channel: not channel member',))]
fn test_set_ban_status_profile_is_not_member() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to test_you_can_only_ban_channel_members

stop_cheat_caller_address(channel_contract_address);
}

#[test]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just seeing you added some tests for leaving and joining channels. please move them above and correct accordingly to the review i made above as regards joining and leaving channels

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants