membership module by mnaamani · Pull Request #3 · Joystream/joystream

mnaamani · 2019-03-13T08:24:31Z

Adding membership module.

siman · 2019-03-13T10:06:44Z

+#[derive(Encode, Decode)]
+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,


Suggested change
handle:u32,
handle:Vec<u8>,

siman · 2019-03-13T10:09:08Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


I would suggest naming this field as about such as description is a better word for items, but not a person

siman · 2019-03-13T10:35:47Z

+
+// Start at 1001? instead to reserve first 1000 ids ?
+const FIRST_MEMBER_ID: u64 = 1;
+const INITIAL_PAID_TERMS_ID: u64 = 1;


Inconsistency in naming: FIRST_ vs INITIAL_

siman · 2019-03-13T10:37:18Z

+ NextMemberId get(next_member_id) : T::MemberId = T::MemberId::sa(FIRST_MEMBER_ID);
+
+ /// Mapping of member ids to their corresponding accountid
+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;


Suggested change
MembersById get(members_by_id): map T::MemberId => T::AccountId;
AccountIdByMemberId get(account_id_by_member_id): map T::MemberId => T::AccountId;

siman · 2019-03-13T10:37:39Z

+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;
+
+ /// Mapping of members' accountid to their member id
+ MemberByAccount get(members_by_account) : map T::AccountId => T::MemberId;


Suggested change
MemberByAccount get(members_by_account): map T::AccountId => T::MemberId;
MemberIdByAccountId get(member_id_by_account_id): map T::AccountId => T::MemberId;

siman · 2019-03-13T10:39:47Z

+ ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec<T::PaidTermId> = vec![T::PaidTermId::sa(INITIAL_PAID_TERMS_ID)];
+
+ /// Is the platform is accepting new members or not
+ PlatformAcceptingNewMemberships get(platform_accepting_new_memberships) : bool = true;


Suggested change
PlatformAcceptingNewMemberships get(platform_accepting_new_memberships):bool = true;
NewMembershipsAllowed get(new_memberships_allowed):bool = true;

siman · 2019-03-13T10:40:40Z

+ pub enum Event<T> where
+ <T as system::Trait>::AccountId,
+ <T as Trait>::MemberId {
+ MemberAdded(MemberId, AccountId),


Maybe rename to MemberRegistered? Registered sounds better when we think about users and social networks

siman · 2019-03-13T10:43:41Z

+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,
+ added: T::BlockNumber,


Suggested change
added:T::BlockNumber,
registeredAt:T::BlockNumber,

siman · 2019-03-13T10:44:21Z

+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,


Option for a case if a user has not set their avatar.
Suggested change
avatarUri:Vec<u8>,
avatarUri:Option<Vec<u8>>,

decided on default empty vector == empty string to represent same as not setting avatar or about text. Also makes the batch updating logic simpler.. how to differentiate between user wanting to pass None to 'unset' avatar, or None as in they don't want to update the avatar

Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None.

siman · 2019-03-13T10:45:51Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


Option for a case if a user has not set their about - this is not required, yes?
Suggested change
description:Vec<u8>,
about:Option<Vec<u8>>,

siman · 2019-03-14T20:55:14Z

+
+ fn validate_avatar(uri: &Vec<u8>) -> Vec<u8> {
+ let mut uri = uri.clone();
+ uri.truncate(Self::max_avatar_uri_length() as usize);


Not sure that truncate is a good approach for avatar: if you truncate an URL then it became invalid.

We have to limit the length somehow. We can accommodate a longer url upto 2000 ? https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers

The same as with a handle:
ensure!(uri.len() <= Self::max_avatar_uri_length() as usize, "avatar uri is too long");

mnaamani · 2019-03-14T22:22:02Z

Yeah interesting. Does parity-coded know how to encode Option<Option<T>> and could it be implemented in polkadot-js-apps as custom type I wonder.

…

On Fri, Mar 15, 2019 at 12:00 AM Alex Siman ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/membership.rs <#3 (comment)> : > +pub trait Trait: system::Trait + GovernanceCurrency { + type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>; + + type MemberId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u64> + MaybeSerializeDebug; + + type PaidTermId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; + + type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; +} + +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +#[derive(Encode, Decode)] +pub struct Profile<T: Trait> { + id: T::MemberId, // is it necessary to have the id in the struct? + handle: u32, + avatarUri: Vec<u8>, Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABi8FPdlzM4G-tUs5FUwrqhFWvGXJ2OVks5vWsZhgaJpZM4bspRq> .

siman · 2019-03-15T19:19:16Z

+ <Handles<T>>::remove(&profile.handle);
+ <Handles<T>>::insert(handle.clone(), profile.id);
+ profile.handle = handle;
+ <MemberProfile<T>>::insert(profile.id, profile);


Missing an event MemberUpdatedHandle

siman · 2019-03-15T20:14:08Z

+ Self::_change_member_handle(&who, handle)?;
+ }
+
+ fn batch_change_member_profile(origin, user_info: UserInfo) {


maybe just update_profile?
Suggested change
fnbatch_change_member_profile(origin,user_info:UserInfo){
fnupdate_profile(origin,user_info:UserInfo){

Change ‘hiring’ module dependency version

Test pack #3 for Hiring Module

Add storage module+events

Update substrate

Upgrade libp2p

Rome

Introduce live milestons + various tweaks

Updated fork 14.02.2020

Husky config

* NotFound translatable * Connecting overlay * Update rx-react (proper state management) * Use i18n-next everywhere * @flow updates

Joystream update membership roles

Content directory second try

File input to uploadVideo - suggested approach

small fixes

Fix "Trying to access property id of undefined" (candidate.member.id)

mnaamani · 2019-03-13T08:24:31Z

Adding membership module.

siman · 2019-03-13T10:06:44Z

+#[derive(Encode, Decode)]
+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,


Suggested change
handle:u32,
handle:Vec<u8>,

siman · 2019-03-13T10:09:08Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


I would suggest naming this field as about such as description is a better word for items, but not a person

siman · 2019-03-13T10:35:47Z

+
+// Start at 1001? instead to reserve first 1000 ids ?
+const FIRST_MEMBER_ID: u64 = 1;
+const INITIAL_PAID_TERMS_ID: u64 = 1;


Inconsistency in naming: FIRST_ vs INITIAL_

siman · 2019-03-13T10:37:18Z

+ NextMemberId get(next_member_id) : T::MemberId = T::MemberId::sa(FIRST_MEMBER_ID);
+
+ /// Mapping of member ids to their corresponding accountid
+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;


Suggested change
MembersById get(members_by_id): map T::MemberId => T::AccountId;
AccountIdByMemberId get(account_id_by_member_id): map T::MemberId => T::AccountId;

siman · 2019-03-13T10:37:39Z

+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;
+
+ /// Mapping of members' accountid to their member id
+ MemberByAccount get(members_by_account) : map T::AccountId => T::MemberId;


Suggested change
MemberByAccount get(members_by_account): map T::AccountId => T::MemberId;
MemberIdByAccountId get(member_id_by_account_id): map T::AccountId => T::MemberId;

siman · 2019-03-13T10:39:47Z

+ ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec<T::PaidTermId> = vec![T::PaidTermId::sa(INITIAL_PAID_TERMS_ID)];
+
+ /// Is the platform is accepting new members or not
+ PlatformAcceptingNewMemberships get(platform_accepting_new_memberships) : bool = true;


Suggested change
PlatformAcceptingNewMemberships get(platform_accepting_new_memberships):bool = true;
NewMembershipsAllowed get(new_memberships_allowed):bool = true;

siman · 2019-03-13T10:40:40Z

+ pub enum Event<T> where
+ <T as system::Trait>::AccountId,
+ <T as Trait>::MemberId {
+ MemberAdded(MemberId, AccountId),


Maybe rename to MemberRegistered? Registered sounds better when we think about users and social networks

siman · 2019-03-13T10:43:41Z

+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,
+ added: T::BlockNumber,


Suggested change
added:T::BlockNumber,
registeredAt:T::BlockNumber,

siman · 2019-03-13T10:44:21Z

+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,


Option for a case if a user has not set their avatar.
Suggested change
avatarUri:Vec<u8>,
avatarUri:Option<Vec<u8>>,

decided on default empty vector == empty string to represent same as not setting avatar or about text. Also makes the batch updating logic simpler.. how to differentiate between user wanting to pass None to 'unset' avatar, or None as in they don't want to update the avatar

Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None.

siman · 2019-03-13T10:45:51Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


Option for a case if a user has not set their about - this is not required, yes?
Suggested change
description:Vec<u8>,
about:Option<Vec<u8>>,

siman · 2019-03-14T20:55:14Z

+
+ fn validate_avatar(uri: &Vec<u8>) -> Vec<u8> {
+ let mut uri = uri.clone();
+ uri.truncate(Self::max_avatar_uri_length() as usize);


Not sure that truncate is a good approach for avatar: if you truncate an URL then it became invalid.

We have to limit the length somehow. We can accommodate a longer url upto 2000 ? https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers

The same as with a handle:
ensure!(uri.len() <= Self::max_avatar_uri_length() as usize, "avatar uri is too long");

mnaamani · 2019-03-14T22:22:02Z

Yeah interesting. Does parity-coded know how to encode Option<Option<T>> and could it be implemented in polkadot-js-apps as custom type I wonder.

…

On Fri, Mar 15, 2019 at 12:00 AM Alex Siman ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/membership.rs <#3 (comment)> : > +pub trait Trait: system::Trait + GovernanceCurrency { + type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>; + + type MemberId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u64> + MaybeSerializeDebug; + + type PaidTermId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; + + type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; +} + +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +#[derive(Encode, Decode)] +pub struct Profile<T: Trait> { + id: T::MemberId, // is it necessary to have the id in the struct? + handle: u32, + avatarUri: Vec<u8>, Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABi8FPdlzM4G-tUs5FUwrqhFWvGXJ2OVks5vWsZhgaJpZM4bspRq> .

siman · 2019-03-15T19:19:16Z

+ <Handles<T>>::remove(&profile.handle);
+ <Handles<T>>::insert(handle.clone(), profile.id);
+ profile.handle = handle;
+ <MemberProfile<T>>::insert(profile.id, profile);


Missing an event MemberUpdatedHandle

siman · 2019-03-15T20:14:08Z

+ Self::_change_member_handle(&who, handle)?;
+ }
+
+ fn batch_change_member_profile(origin, user_info: UserInfo) {


maybe just update_profile?
Suggested change
fnbatch_change_member_profile(origin,user_info:UserInfo){
fnupdate_profile(origin,user_info:UserInfo){

Change ‘hiring’ module dependency version

Test pack #3 for Hiring Module

Add storage module+events

Update substrate

Upgrade libp2p

Rome

Introduce live milestons + various tweaks

Updated fork 14.02.2020

Husky config

* NotFound translatable * Connecting overlay * Update rx-react (proper state management) * Use i18n-next everywhere * @flow updates

Joystream update membership roles

Content directory second try

File input to uploadVideo - suggested approach

small fixes

Fix "Trying to access property id of undefined" (candidate.member.id)

mnaamani · 2019-03-13T08:24:31Z

Adding membership module.

siman · 2019-03-13T10:06:44Z

+#[derive(Encode, Decode)]
+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,


Suggested change
handle:u32,
handle:Vec<u8>,

siman · 2019-03-13T10:09:08Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


I would suggest naming this field as about such as description is a better word for items, but not a person

siman · 2019-03-13T10:35:47Z

+
+// Start at 1001? instead to reserve first 1000 ids ?
+const FIRST_MEMBER_ID: u64 = 1;
+const INITIAL_PAID_TERMS_ID: u64 = 1;


Inconsistency in naming: FIRST_ vs INITIAL_

siman · 2019-03-13T10:37:18Z

+ NextMemberId get(next_member_id) : T::MemberId = T::MemberId::sa(FIRST_MEMBER_ID);
+
+ /// Mapping of member ids to their corresponding accountid
+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;


Suggested change
MembersById get(members_by_id): map T::MemberId => T::AccountId;
AccountIdByMemberId get(account_id_by_member_id): map T::MemberId => T::AccountId;

siman · 2019-03-13T10:37:39Z

+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;
+
+ /// Mapping of members' accountid to their member id
+ MemberByAccount get(members_by_account) : map T::AccountId => T::MemberId;


Suggested change
MemberByAccount get(members_by_account): map T::AccountId => T::MemberId;
MemberIdByAccountId get(member_id_by_account_id): map T::AccountId => T::MemberId;

siman · 2019-03-13T10:39:47Z

+ ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec<T::PaidTermId> = vec![T::PaidTermId::sa(INITIAL_PAID_TERMS_ID)];
+
+ /// Is the platform is accepting new members or not
+ PlatformAcceptingNewMemberships get(platform_accepting_new_memberships) : bool = true;


Suggested change
PlatformAcceptingNewMemberships get(platform_accepting_new_memberships):bool = true;
NewMembershipsAllowed get(new_memberships_allowed):bool = true;

siman · 2019-03-13T10:40:40Z

+ pub enum Event<T> where
+ <T as system::Trait>::AccountId,
+ <T as Trait>::MemberId {
+ MemberAdded(MemberId, AccountId),


Maybe rename to MemberRegistered? Registered sounds better when we think about users and social networks

siman · 2019-03-13T10:43:41Z

+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,
+ added: T::BlockNumber,


Suggested change
added:T::BlockNumber,
registeredAt:T::BlockNumber,

siman · 2019-03-13T10:44:21Z

+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,


Option for a case if a user has not set their avatar.
Suggested change
avatarUri:Vec<u8>,
avatarUri:Option<Vec<u8>>,

decided on default empty vector == empty string to represent same as not setting avatar or about text. Also makes the batch updating logic simpler.. how to differentiate between user wanting to pass None to 'unset' avatar, or None as in they don't want to update the avatar

Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None.

siman · 2019-03-13T10:45:51Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


Option for a case if a user has not set their about - this is not required, yes?
Suggested change
description:Vec<u8>,
about:Option<Vec<u8>>,

siman · 2019-03-14T20:55:14Z

+
+ fn validate_avatar(uri: &Vec<u8>) -> Vec<u8> {
+ let mut uri = uri.clone();
+ uri.truncate(Self::max_avatar_uri_length() as usize);


Not sure that truncate is a good approach for avatar: if you truncate an URL then it became invalid.

We have to limit the length somehow. We can accommodate a longer url upto 2000 ? https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers

The same as with a handle:
ensure!(uri.len() <= Self::max_avatar_uri_length() as usize, "avatar uri is too long");

mnaamani · 2019-03-14T22:22:02Z

Yeah interesting. Does parity-coded know how to encode Option<Option<T>> and could it be implemented in polkadot-js-apps as custom type I wonder.

…

On Fri, Mar 15, 2019 at 12:00 AM Alex Siman ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/membership.rs <#3 (comment)> : > +pub trait Trait: system::Trait + GovernanceCurrency { + type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>; + + type MemberId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u64> + MaybeSerializeDebug; + + type PaidTermId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; + + type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; +} + +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +#[derive(Encode, Decode)] +pub struct Profile<T: Trait> { + id: T::MemberId, // is it necessary to have the id in the struct? + handle: u32, + avatarUri: Vec<u8>, Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABi8FPdlzM4G-tUs5FUwrqhFWvGXJ2OVks5vWsZhgaJpZM4bspRq> .

siman · 2019-03-15T19:19:16Z

+ <Handles<T>>::remove(&profile.handle);
+ <Handles<T>>::insert(handle.clone(), profile.id);
+ profile.handle = handle;
+ <MemberProfile<T>>::insert(profile.id, profile);


Missing an event MemberUpdatedHandle

siman · 2019-03-15T20:14:08Z

+ Self::_change_member_handle(&who, handle)?;
+ }
+
+ fn batch_change_member_profile(origin, user_info: UserInfo) {


maybe just update_profile?
Suggested change
fnbatch_change_member_profile(origin,user_info:UserInfo){
fnupdate_profile(origin,user_info:UserInfo){

Change ‘hiring’ module dependency version

Test pack #3 for Hiring Module

Add storage module+events

Update substrate

Upgrade libp2p

Rome

Introduce live milestons + various tweaks

Updated fork 14.02.2020

Husky config

* NotFound translatable * Connecting overlay * Update rx-react (proper state management) * Use i18n-next everywhere * @flow updates

Joystream update membership roles

Content directory second try

File input to uploadVideo - suggested approach

small fixes

Fix "Trying to access property id of undefined" (candidate.member.id)

mnaamani · 2019-03-13T08:24:31Z

Adding membership module.

siman · 2019-03-13T10:06:44Z

+#[derive(Encode, Decode)]
+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,


Suggested change
handle:u32,
handle:Vec<u8>,

siman · 2019-03-13T10:09:08Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


I would suggest naming this field as about such as description is a better word for items, but not a person

siman · 2019-03-13T10:35:47Z

+
+// Start at 1001? instead to reserve first 1000 ids ?
+const FIRST_MEMBER_ID: u64 = 1;
+const INITIAL_PAID_TERMS_ID: u64 = 1;


Inconsistency in naming: FIRST_ vs INITIAL_

siman · 2019-03-13T10:37:18Z

+ NextMemberId get(next_member_id) : T::MemberId = T::MemberId::sa(FIRST_MEMBER_ID);
+
+ /// Mapping of member ids to their corresponding accountid
+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;


Suggested change
MembersById get(members_by_id): map T::MemberId => T::AccountId;
AccountIdByMemberId get(account_id_by_member_id): map T::MemberId => T::AccountId;

siman · 2019-03-13T10:37:39Z

+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;
+
+ /// Mapping of members' accountid to their member id
+ MemberByAccount get(members_by_account) : map T::AccountId => T::MemberId;


Suggested change
MemberByAccount get(members_by_account): map T::AccountId => T::MemberId;
MemberIdByAccountId get(member_id_by_account_id): map T::AccountId => T::MemberId;

siman · 2019-03-13T10:39:47Z

+ ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec<T::PaidTermId> = vec![T::PaidTermId::sa(INITIAL_PAID_TERMS_ID)];
+
+ /// Is the platform is accepting new members or not
+ PlatformAcceptingNewMemberships get(platform_accepting_new_memberships) : bool = true;


Suggested change
PlatformAcceptingNewMemberships get(platform_accepting_new_memberships):bool = true;
NewMembershipsAllowed get(new_memberships_allowed):bool = true;

siman · 2019-03-13T10:40:40Z

+ pub enum Event<T> where
+ <T as system::Trait>::AccountId,
+ <T as Trait>::MemberId {
+ MemberAdded(MemberId, AccountId),


Maybe rename to MemberRegistered? Registered sounds better when we think about users and social networks

siman · 2019-03-13T10:43:41Z

+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,
+ added: T::BlockNumber,


Suggested change
added:T::BlockNumber,
registeredAt:T::BlockNumber,

siman · 2019-03-13T10:44:21Z

+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,


Option for a case if a user has not set their avatar.
Suggested change
avatarUri:Vec<u8>,
avatarUri:Option<Vec<u8>>,

decided on default empty vector == empty string to represent same as not setting avatar or about text. Also makes the batch updating logic simpler.. how to differentiate between user wanting to pass None to 'unset' avatar, or None as in they don't want to update the avatar

Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None.

siman · 2019-03-13T10:45:51Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


Option for a case if a user has not set their about - this is not required, yes?
Suggested change
description:Vec<u8>,
about:Option<Vec<u8>>,

siman · 2019-03-14T20:55:14Z

+
+ fn validate_avatar(uri: &Vec<u8>) -> Vec<u8> {
+ let mut uri = uri.clone();
+ uri.truncate(Self::max_avatar_uri_length() as usize);


Not sure that truncate is a good approach for avatar: if you truncate an URL then it became invalid.

We have to limit the length somehow. We can accommodate a longer url upto 2000 ? https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers

The same as with a handle:
ensure!(uri.len() <= Self::max_avatar_uri_length() as usize, "avatar uri is too long");

mnaamani · 2019-03-14T22:22:02Z

Yeah interesting. Does parity-coded know how to encode Option<Option<T>> and could it be implemented in polkadot-js-apps as custom type I wonder.

…

On Fri, Mar 15, 2019 at 12:00 AM Alex Siman ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/membership.rs <#3 (comment)> : > +pub trait Trait: system::Trait + GovernanceCurrency { + type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>; + + type MemberId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u64> + MaybeSerializeDebug; + + type PaidTermId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; + + type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; +} + +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +#[derive(Encode, Decode)] +pub struct Profile<T: Trait> { + id: T::MemberId, // is it necessary to have the id in the struct? + handle: u32, + avatarUri: Vec<u8>, Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABi8FPdlzM4G-tUs5FUwrqhFWvGXJ2OVks5vWsZhgaJpZM4bspRq> .

siman · 2019-03-15T19:19:16Z

+ <Handles<T>>::remove(&profile.handle);
+ <Handles<T>>::insert(handle.clone(), profile.id);
+ profile.handle = handle;
+ <MemberProfile<T>>::insert(profile.id, profile);


Missing an event MemberUpdatedHandle

siman · 2019-03-15T20:14:08Z

+ Self::_change_member_handle(&who, handle)?;
+ }
+
+ fn batch_change_member_profile(origin, user_info: UserInfo) {


maybe just update_profile?
Suggested change
fnbatch_change_member_profile(origin,user_info:UserInfo){
fnupdate_profile(origin,user_info:UserInfo){

Change ‘hiring’ module dependency version

Test pack #3 for Hiring Module

Add storage module+events

Update substrate

Upgrade libp2p

Rome

Introduce live milestons + various tweaks

Updated fork 14.02.2020

Husky config

* NotFound translatable * Connecting overlay * Update rx-react (proper state management) * Use i18n-next everywhere * @flow updates

Joystream update membership roles

Content directory second try

File input to uploadVideo - suggested approach

small fixes

Fix "Trying to access property id of undefined" (candidate.member.id)

mnaamani · 2019-03-13T08:24:31Z

Adding membership module.

siman · 2019-03-13T10:06:44Z

+#[derive(Encode, Decode)]
+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,


Suggested change
handle:u32,
handle:Vec<u8>,

siman · 2019-03-13T10:09:08Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


I would suggest naming this field as about such as description is a better word for items, but not a person

siman · 2019-03-13T10:35:47Z

+
+// Start at 1001? instead to reserve first 1000 ids ?
+const FIRST_MEMBER_ID: u64 = 1;
+const INITIAL_PAID_TERMS_ID: u64 = 1;


Inconsistency in naming: FIRST_ vs INITIAL_

siman · 2019-03-13T10:37:18Z

+ NextMemberId get(next_member_id) : T::MemberId = T::MemberId::sa(FIRST_MEMBER_ID);
+
+ /// Mapping of member ids to their corresponding accountid
+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;


Suggested change
MembersById get(members_by_id): map T::MemberId => T::AccountId;
AccountIdByMemberId get(account_id_by_member_id): map T::MemberId => T::AccountId;

siman · 2019-03-13T10:37:39Z

+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;
+
+ /// Mapping of members' accountid to their member id
+ MemberByAccount get(members_by_account) : map T::AccountId => T::MemberId;


Suggested change
MemberByAccount get(members_by_account): map T::AccountId => T::MemberId;
MemberIdByAccountId get(member_id_by_account_id): map T::AccountId => T::MemberId;

siman · 2019-03-13T10:39:47Z

+ ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec<T::PaidTermId> = vec![T::PaidTermId::sa(INITIAL_PAID_TERMS_ID)];
+
+ /// Is the platform is accepting new members or not
+ PlatformAcceptingNewMemberships get(platform_accepting_new_memberships) : bool = true;


Suggested change
PlatformAcceptingNewMemberships get(platform_accepting_new_memberships):bool = true;
NewMembershipsAllowed get(new_memberships_allowed):bool = true;

siman · 2019-03-13T10:40:40Z

+ pub enum Event<T> where
+ <T as system::Trait>::AccountId,
+ <T as Trait>::MemberId {
+ MemberAdded(MemberId, AccountId),


Maybe rename to MemberRegistered? Registered sounds better when we think about users and social networks

siman · 2019-03-13T10:43:41Z

+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,
+ added: T::BlockNumber,


Suggested change
added:T::BlockNumber,
registeredAt:T::BlockNumber,

siman · 2019-03-13T10:44:21Z

+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,


Option for a case if a user has not set their avatar.
Suggested change
avatarUri:Vec<u8>,
avatarUri:Option<Vec<u8>>,

decided on default empty vector == empty string to represent same as not setting avatar or about text. Also makes the batch updating logic simpler.. how to differentiate between user wanting to pass None to 'unset' avatar, or None as in they don't want to update the avatar

Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None.

siman · 2019-03-13T10:45:51Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


Option for a case if a user has not set their about - this is not required, yes?
Suggested change
description:Vec<u8>,
about:Option<Vec<u8>>,

siman · 2019-03-14T20:55:14Z

+
+ fn validate_avatar(uri: &Vec<u8>) -> Vec<u8> {
+ let mut uri = uri.clone();
+ uri.truncate(Self::max_avatar_uri_length() as usize);


Not sure that truncate is a good approach for avatar: if you truncate an URL then it became invalid.

We have to limit the length somehow. We can accommodate a longer url upto 2000 ? https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers

The same as with a handle:
ensure!(uri.len() <= Self::max_avatar_uri_length() as usize, "avatar uri is too long");

mnaamani · 2019-03-14T22:22:02Z

Yeah interesting. Does parity-coded know how to encode Option<Option<T>> and could it be implemented in polkadot-js-apps as custom type I wonder.

…

On Fri, Mar 15, 2019 at 12:00 AM Alex Siman ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/membership.rs <#3 (comment)> : > +pub trait Trait: system::Trait + GovernanceCurrency { + type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>; + + type MemberId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u64> + MaybeSerializeDebug; + + type PaidTermId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; + + type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; +} + +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +#[derive(Encode, Decode)] +pub struct Profile<T: Trait> { + id: T::MemberId, // is it necessary to have the id in the struct? + handle: u32, + avatarUri: Vec<u8>, Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABi8FPdlzM4G-tUs5FUwrqhFWvGXJ2OVks5vWsZhgaJpZM4bspRq> .

siman · 2019-03-15T19:19:16Z

+ <Handles<T>>::remove(&profile.handle);
+ <Handles<T>>::insert(handle.clone(), profile.id);
+ profile.handle = handle;
+ <MemberProfile<T>>::insert(profile.id, profile);


Missing an event MemberUpdatedHandle

siman · 2019-03-15T20:14:08Z

+ Self::_change_member_handle(&who, handle)?;
+ }
+
+ fn batch_change_member_profile(origin, user_info: UserInfo) {


maybe just update_profile?
Suggested change
fnbatch_change_member_profile(origin,user_info:UserInfo){
fnupdate_profile(origin,user_info:UserInfo){

Change ‘hiring’ module dependency version

Test pack #3 for Hiring Module

Add storage module+events

Update substrate

Upgrade libp2p

Rome

Introduce live milestons + various tweaks

Updated fork 14.02.2020

Husky config

* NotFound translatable * Connecting overlay * Update rx-react (proper state management) * Use i18n-next everywhere * @flow updates

Joystream update membership roles

Content directory second try

File input to uploadVideo - suggested approach

small fixes

Fix "Trying to access property id of undefined" (candidate.member.id)

mnaamani · 2019-03-13T08:24:31Z

Adding membership module.

siman · 2019-03-13T10:06:44Z

+#[derive(Encode, Decode)]
+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,


Suggested change
handle:u32,
handle:Vec<u8>,

siman · 2019-03-13T10:09:08Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


I would suggest naming this field as about such as description is a better word for items, but not a person

siman · 2019-03-13T10:35:47Z

+
+// Start at 1001? instead to reserve first 1000 ids ?
+const FIRST_MEMBER_ID: u64 = 1;
+const INITIAL_PAID_TERMS_ID: u64 = 1;


Inconsistency in naming: FIRST_ vs INITIAL_

siman · 2019-03-13T10:37:18Z

+ NextMemberId get(next_member_id) : T::MemberId = T::MemberId::sa(FIRST_MEMBER_ID);
+
+ /// Mapping of member ids to their corresponding accountid
+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;


Suggested change
MembersById get(members_by_id): map T::MemberId => T::AccountId;
AccountIdByMemberId get(account_id_by_member_id): map T::MemberId => T::AccountId;

siman · 2019-03-13T10:37:39Z

+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;
+
+ /// Mapping of members' accountid to their member id
+ MemberByAccount get(members_by_account) : map T::AccountId => T::MemberId;


Suggested change
MemberByAccount get(members_by_account): map T::AccountId => T::MemberId;
MemberIdByAccountId get(member_id_by_account_id): map T::AccountId => T::MemberId;

siman · 2019-03-13T10:39:47Z

+ ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec<T::PaidTermId> = vec![T::PaidTermId::sa(INITIAL_PAID_TERMS_ID)];
+
+ /// Is the platform is accepting new members or not
+ PlatformAcceptingNewMemberships get(platform_accepting_new_memberships) : bool = true;


Suggested change
PlatformAcceptingNewMemberships get(platform_accepting_new_memberships):bool = true;
NewMembershipsAllowed get(new_memberships_allowed):bool = true;

siman · 2019-03-13T10:40:40Z

+ pub enum Event<T> where
+ <T as system::Trait>::AccountId,
+ <T as Trait>::MemberId {
+ MemberAdded(MemberId, AccountId),


Maybe rename to MemberRegistered? Registered sounds better when we think about users and social networks

siman · 2019-03-13T10:43:41Z

+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,
+ added: T::BlockNumber,


Suggested change
added:T::BlockNumber,
registeredAt:T::BlockNumber,

siman · 2019-03-13T10:44:21Z

+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,


Option for a case if a user has not set their avatar.
Suggested change
avatarUri:Vec<u8>,
avatarUri:Option<Vec<u8>>,

decided on default empty vector == empty string to represent same as not setting avatar or about text. Also makes the batch updating logic simpler.. how to differentiate between user wanting to pass None to 'unset' avatar, or None as in they don't want to update the avatar

Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None.

siman · 2019-03-13T10:45:51Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


Option for a case if a user has not set their about - this is not required, yes?
Suggested change
description:Vec<u8>,
about:Option<Vec<u8>>,

siman · 2019-03-14T20:55:14Z

+
+ fn validate_avatar(uri: &Vec<u8>) -> Vec<u8> {
+ let mut uri = uri.clone();
+ uri.truncate(Self::max_avatar_uri_length() as usize);


Not sure that truncate is a good approach for avatar: if you truncate an URL then it became invalid.

We have to limit the length somehow. We can accommodate a longer url upto 2000 ? https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers

The same as with a handle:
ensure!(uri.len() <= Self::max_avatar_uri_length() as usize, "avatar uri is too long");

mnaamani · 2019-03-14T22:22:02Z

Yeah interesting. Does parity-coded know how to encode Option<Option<T>> and could it be implemented in polkadot-js-apps as custom type I wonder.

…

On Fri, Mar 15, 2019 at 12:00 AM Alex Siman ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/membership.rs <#3 (comment)> : > +pub trait Trait: system::Trait + GovernanceCurrency { + type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>; + + type MemberId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u64> + MaybeSerializeDebug; + + type PaidTermId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; + + type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; +} + +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +#[derive(Encode, Decode)] +pub struct Profile<T: Trait> { + id: T::MemberId, // is it necessary to have the id in the struct? + handle: u32, + avatarUri: Vec<u8>, Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABi8FPdlzM4G-tUs5FUwrqhFWvGXJ2OVks5vWsZhgaJpZM4bspRq> .

siman · 2019-03-15T19:19:16Z

+ <Handles<T>>::remove(&profile.handle);
+ <Handles<T>>::insert(handle.clone(), profile.id);
+ profile.handle = handle;
+ <MemberProfile<T>>::insert(profile.id, profile);


Missing an event MemberUpdatedHandle

siman · 2019-03-15T20:14:08Z

+ Self::_change_member_handle(&who, handle)?;
+ }
+
+ fn batch_change_member_profile(origin, user_info: UserInfo) {


maybe just update_profile?
Suggested change
fnbatch_change_member_profile(origin,user_info:UserInfo){
fnupdate_profile(origin,user_info:UserInfo){

Change ‘hiring’ module dependency version

Test pack #3 for Hiring Module

Add storage module+events

Update substrate

Upgrade libp2p

Rome

Introduce live milestons + various tweaks

Updated fork 14.02.2020

Husky config

* NotFound translatable * Connecting overlay * Update rx-react (proper state management) * Use i18n-next everywhere * @flow updates

Joystream update membership roles

Content directory second try

File input to uploadVideo - suggested approach

small fixes

Fix "Trying to access property id of undefined" (candidate.member.id)

mnaamani · 2019-03-13T08:24:31Z

Adding membership module.

siman · 2019-03-13T10:06:44Z

+#[derive(Encode, Decode)]
+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,


Suggested change
handle:u32,
handle:Vec<u8>,

siman · 2019-03-13T10:09:08Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


I would suggest naming this field as about such as description is a better word for items, but not a person

siman · 2019-03-13T10:35:47Z

+
+// Start at 1001? instead to reserve first 1000 ids ?
+const FIRST_MEMBER_ID: u64 = 1;
+const INITIAL_PAID_TERMS_ID: u64 = 1;


Inconsistency in naming: FIRST_ vs INITIAL_

siman · 2019-03-13T10:37:18Z

+ NextMemberId get(next_member_id) : T::MemberId = T::MemberId::sa(FIRST_MEMBER_ID);
+
+ /// Mapping of member ids to their corresponding accountid
+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;


Suggested change
MembersById get(members_by_id): map T::MemberId => T::AccountId;
AccountIdByMemberId get(account_id_by_member_id): map T::MemberId => T::AccountId;

siman · 2019-03-13T10:37:39Z

+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;
+
+ /// Mapping of members' accountid to their member id
+ MemberByAccount get(members_by_account) : map T::AccountId => T::MemberId;


Suggested change
MemberByAccount get(members_by_account): map T::AccountId => T::MemberId;
MemberIdByAccountId get(member_id_by_account_id): map T::AccountId => T::MemberId;

siman · 2019-03-13T10:39:47Z

+ ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec<T::PaidTermId> = vec![T::PaidTermId::sa(INITIAL_PAID_TERMS_ID)];
+
+ /// Is the platform is accepting new members or not
+ PlatformAcceptingNewMemberships get(platform_accepting_new_memberships) : bool = true;


Suggested change
PlatformAcceptingNewMemberships get(platform_accepting_new_memberships):bool = true;
NewMembershipsAllowed get(new_memberships_allowed):bool = true;

siman · 2019-03-13T10:40:40Z

+ pub enum Event<T> where
+ <T as system::Trait>::AccountId,
+ <T as Trait>::MemberId {
+ MemberAdded(MemberId, AccountId),


Maybe rename to MemberRegistered? Registered sounds better when we think about users and social networks

siman · 2019-03-13T10:43:41Z

+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,
+ added: T::BlockNumber,


Suggested change
added:T::BlockNumber,
registeredAt:T::BlockNumber,

siman · 2019-03-13T10:44:21Z

+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,


Option for a case if a user has not set their avatar.
Suggested change
avatarUri:Vec<u8>,
avatarUri:Option<Vec<u8>>,

decided on default empty vector == empty string to represent same as not setting avatar or about text. Also makes the batch updating logic simpler.. how to differentiate between user wanting to pass None to 'unset' avatar, or None as in they don't want to update the avatar

Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None.

siman · 2019-03-13T10:45:51Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


Option for a case if a user has not set their about - this is not required, yes?
Suggested change
description:Vec<u8>,
about:Option<Vec<u8>>,

siman · 2019-03-14T20:55:14Z

+
+ fn validate_avatar(uri: &Vec<u8>) -> Vec<u8> {
+ let mut uri = uri.clone();
+ uri.truncate(Self::max_avatar_uri_length() as usize);


Not sure that truncate is a good approach for avatar: if you truncate an URL then it became invalid.

We have to limit the length somehow. We can accommodate a longer url upto 2000 ? https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers

The same as with a handle:
ensure!(uri.len() <= Self::max_avatar_uri_length() as usize, "avatar uri is too long");

mnaamani · 2019-03-14T22:22:02Z

Yeah interesting. Does parity-coded know how to encode Option<Option<T>> and could it be implemented in polkadot-js-apps as custom type I wonder.

…

On Fri, Mar 15, 2019 at 12:00 AM Alex Siman ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/membership.rs <#3 (comment)> : > +pub trait Trait: system::Trait + GovernanceCurrency { + type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>; + + type MemberId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u64> + MaybeSerializeDebug; + + type PaidTermId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; + + type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; +} + +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +#[derive(Encode, Decode)] +pub struct Profile<T: Trait> { + id: T::MemberId, // is it necessary to have the id in the struct? + handle: u32, + avatarUri: Vec<u8>, Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABi8FPdlzM4G-tUs5FUwrqhFWvGXJ2OVks5vWsZhgaJpZM4bspRq> .

siman · 2019-03-15T19:19:16Z

+ <Handles<T>>::remove(&profile.handle);
+ <Handles<T>>::insert(handle.clone(), profile.id);
+ profile.handle = handle;
+ <MemberProfile<T>>::insert(profile.id, profile);


Missing an event MemberUpdatedHandle

siman · 2019-03-15T20:14:08Z

+ Self::_change_member_handle(&who, handle)?;
+ }
+
+ fn batch_change_member_profile(origin, user_info: UserInfo) {


maybe just update_profile?
Suggested change
fnbatch_change_member_profile(origin,user_info:UserInfo){
fnupdate_profile(origin,user_info:UserInfo){

Change ‘hiring’ module dependency version

Test pack #3 for Hiring Module

Add storage module+events

Update substrate

Upgrade libp2p

Rome

Introduce live milestons + various tweaks

Updated fork 14.02.2020

Husky config

* NotFound translatable * Connecting overlay * Update rx-react (proper state management) * Use i18n-next everywhere * @flow updates

Joystream update membership roles

Content directory second try

File input to uploadVideo - suggested approach

small fixes

Fix "Trying to access property id of undefined" (candidate.member.id)

mnaamani · 2019-03-13T08:24:31Z

Adding membership module.

siman · 2019-03-13T10:06:44Z

+#[derive(Encode, Decode)]
+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,


Suggested change
handle:u32,
handle:Vec<u8>,

siman · 2019-03-13T10:09:08Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


I would suggest naming this field as about such as description is a better word for items, but not a person

siman · 2019-03-13T10:35:47Z

+
+// Start at 1001? instead to reserve first 1000 ids ?
+const FIRST_MEMBER_ID: u64 = 1;
+const INITIAL_PAID_TERMS_ID: u64 = 1;


Inconsistency in naming: FIRST_ vs INITIAL_

siman · 2019-03-13T10:37:18Z

+ NextMemberId get(next_member_id) : T::MemberId = T::MemberId::sa(FIRST_MEMBER_ID);
+
+ /// Mapping of member ids to their corresponding accountid
+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;


Suggested change
MembersById get(members_by_id): map T::MemberId => T::AccountId;
AccountIdByMemberId get(account_id_by_member_id): map T::MemberId => T::AccountId;

siman · 2019-03-13T10:37:39Z

+ MembersById get(members_by_id) : map T::MemberId => T::AccountId;
+
+ /// Mapping of members' accountid to their member id
+ MemberByAccount get(members_by_account) : map T::AccountId => T::MemberId;


Suggested change
MemberByAccount get(members_by_account): map T::AccountId => T::MemberId;
MemberIdByAccountId get(member_id_by_account_id): map T::AccountId => T::MemberId;

siman · 2019-03-13T10:39:47Z

+ ActivePaidMembershipTerms get(active_paid_membership_terms) : Vec<T::PaidTermId> = vec![T::PaidTermId::sa(INITIAL_PAID_TERMS_ID)];
+
+ /// Is the platform is accepting new members or not
+ PlatformAcceptingNewMemberships get(platform_accepting_new_memberships) : bool = true;


Suggested change
PlatformAcceptingNewMemberships get(platform_accepting_new_memberships):bool = true;
NewMembershipsAllowed get(new_memberships_allowed):bool = true;

siman · 2019-03-13T10:40:40Z

+ pub enum Event<T> where
+ <T as system::Trait>::AccountId,
+ <T as Trait>::MemberId {
+ MemberAdded(MemberId, AccountId),


Maybe rename to MemberRegistered? Registered sounds better when we think about users and social networks

siman · 2019-03-13T10:43:41Z

+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,
+ added: T::BlockNumber,


Suggested change
added:T::BlockNumber,
registeredAt:T::BlockNumber,

siman · 2019-03-13T10:44:21Z

+pub struct Profile<T: Trait> {
+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,


Option for a case if a user has not set their avatar.
Suggested change
avatarUri:Vec<u8>,
avatarUri:Option<Vec<u8>>,

decided on default empty vector == empty string to represent same as not setting avatar or about text. Also makes the batch updating logic simpler.. how to differentiate between user wanting to pass None to 'unset' avatar, or None as in they don't want to update the avatar

Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None.

siman · 2019-03-13T10:45:51Z

+ id: T::MemberId, // is it necessary to have the id in the struct?
+ handle: u32,
+ avatarUri: Vec<u8>,
+ description: Vec<u8>,


Option for a case if a user has not set their about - this is not required, yes?
Suggested change
description:Vec<u8>,
about:Option<Vec<u8>>,

siman · 2019-03-14T20:55:14Z

+
+ fn validate_avatar(uri: &Vec<u8>) -> Vec<u8> {
+ let mut uri = uri.clone();
+ uri.truncate(Self::max_avatar_uri_length() as usize);


Not sure that truncate is a good approach for avatar: if you truncate an URL then it became invalid.

We have to limit the length somehow. We can accommodate a longer url upto 2000 ? https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of-a-url-in-different-browsers

The same as with a handle:
ensure!(uri.len() <= Self::max_avatar_uri_length() as usize, "avatar uri is too long");

mnaamani · 2019-03-14T22:22:02Z

Yeah interesting. Does parity-coded know how to encode Option<Option<T>> and could it be implemented in polkadot-js-apps as custom type I wonder.

…

On Fri, Mar 15, 2019 at 12:00 AM Alex Siman ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/membership.rs <#3 (comment)> : > +pub trait Trait: system::Trait + GovernanceCurrency { + type Event: From<Event<Self>> + Into<<Self as system::Trait>::Event>; + + type MemberId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u64> + MaybeSerializeDebug; + + type PaidTermId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; + + type SubscriptionId: Parameter + Member + SimpleArithmetic + Codec + Default + Copy + As<usize> + As<u32> + MaybeSerializeDebug; +} + +#[cfg_attr(feature = "std", derive(Serialize, Deserialize, Debug))] +#[derive(Encode, Decode)] +pub struct Profile<T: Trait> { + id: T::MemberId, // is it necessary to have the id in the struct? + handle: u32, + avatarUri: Vec<u8>, Good question. In such case, it should be Option<Option<Vec<u8>>> and then if you want to set avatar_uri to None you will pass it with UserInfo as Some(None). And if you don't want to update it, then just None. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABi8FPdlzM4G-tUs5FUwrqhFWvGXJ2OVks5vWsZhgaJpZM4bspRq> .

siman · 2019-03-15T19:19:16Z

+ <Handles<T>>::remove(&profile.handle);
+ <Handles<T>>::insert(handle.clone(), profile.id);
+ profile.handle = handle;
+ <MemberProfile<T>>::insert(profile.id, profile);


Missing an event MemberUpdatedHandle

siman · 2019-03-15T20:14:08Z

+ Self::_change_member_handle(&who, handle)?;
+ }
+
+ fn batch_change_member_profile(origin, user_info: UserInfo) {


maybe just update_profile?
Suggested change
fnbatch_change_member_profile(origin,user_info:UserInfo){
fnupdate_profile(origin,user_info:UserInfo){

Change ‘hiring’ module dependency version

Test pack #3 for Hiring Module

Add storage module+events

Update substrate

Upgrade libp2p

Rome

Introduce live milestons + various tweaks

Updated fork 14.02.2020

Husky config

* NotFound translatable * Connecting overlay * Update rx-react (proper state management) * Use i18n-next everywhere * @flow updates

Joystream update membership roles

Content directory second try

File input to uploadVideo - suggested approach

small fixes

Fix "Trying to access property id of undefined" (candidate.member.id)

adding membership module - storage
43376ab

siman reviewed Mar 13, 2019
View reviewed changes

mnaamani added 11 commits March 13, 2019 13:47

use default implicit value for paid terms instead of initializing map
216e694

add migration
a7d4aac

renaming, adding handles map
6923186

membership: implement buy_membership
1237df8

make avatar_uri an Option instead of empty string to represent no uri
918868e

add simple buy membership method
cb1a3a7

burn fee when buying membership
6e202d9

keep extrinsic validation outside insert_new_paid_member
54738e7

update methods for profile
e99cb53

add method to change multiple profile user infos in one transaction
6f46ad1

check allowed new members before accepting new members
ac0c928

siman reviewed Mar 14, 2019
View reviewed changes

mnaamani added 2 commits March 14, 2019 23:52

handle runtime uptime by using spec_version
f095b67

fix batch profile update
1d63f67

mnaamani marked this pull request as ready for review March 15, 2019 08:58

siman reviewed Mar 15, 2019
View reviewed changes

mnaamani added 2 commits March 16, 2019 08:03

rename update profile method, and don't truncate avatar uri
7ae24d0

member sub accounts
2d3329a

mnaamani changed the title ~~adding membership module - storage~~membership moduleMar 16, 2019

mnaamani added 7 commits March 16, 2019 10:56

add some notes on sub-accounts
da0a072

default min length of handle 5 chars
31fd1de

adjust avatar, handle and about text default max lengths
95e4cd3

reorg membership into a module directory
478fbe0

add tests for membership
a227fa3

membership tests
27b7841

membership: more tests
5ba5f86

mnaamani added 3 commits March 20, 2019 16:04

additional comments on membership methods
53e54b6

membership sub accounts - revert (move to staked roles)
3fab73d

add member through screener
9f07751

mnaamani mentioned this pull request Mar 23, 2019
Staked Roles #6
Merged
3 tasks

bwhm added this to the Finalize runtime for internal Athens Runtime Testnet milestone Mar 23, 2019

refactor membership: renamed registry module to members
3e43d9e

siman reviewed Mar 25, 2019
View reviewed changes

Comment threadsrc/membership/members.rs Outdated

members: refactor insert members method
75d835d

siman merged commit b592639 into Joystream:developmentMar 25, 2019

siman mentioned this pull request Mar 25, 2019
For #7, implement simple Data Object Type registry #13
Merged

bedeho pushed a commit that referenced this pull request Feb 24, 2020
Merge pull request#3from shamil-gadelshin/hiring-dependency-upgrade
cd83dcf
Change ‘hiring’ module dependency version

shamil-gadelshin referenced this pull request in shamil-gadelshin/joystream-network Mar 9, 2020
Merge pull requestJoystream#20from shamil-gadelshin/test_pack3
b400e30
Test pack #3 for Hiring Module

shamil-gadelshin referenced this pull request in shamil-gadelshin/joystream-network Mar 9, 2020
Merge pull request#3from bedeho/add-storage
3e65016
Add storage module+events

shamil-gadelshin referenced this pull request in shamil-gadelshin/joystream-network Mar 9, 2020
Merge pull request#3from mnaamani/update-substrate
dfdadb2
Update substrate

shamil-gadelshin referenced this pull request in shamil-gadelshin/joystream-network Mar 9, 2020
Merge pull request#3from mnaamani/upgrade-libp2p
86f4e7b
Upgrade libp2p

shamil-gadelshin referenced this pull request in shamil-gadelshin/joystream-network Mar 9, 2020
Merge pull request#3from Joystream/rome
e8241cf
Rome

bedeho pushed a commit that referenced this pull request Apr 16, 2020
Merge pull request#3from bedeho/add-live-milestones
819ab2b
Introduce live milestons + various tweaks

bedeho pushed a commit that referenced this pull request Apr 16, 2020
Merge pull request#3from Joystream/master
956db2c
Updated fork 14.02.2020

semeano added a commit to semeano/joystream that referenced this pull request May 14, 2020
Merge pull requestJoystream#3from semeano/husky-config
7c124c2
Husky config

Lezek123 referenced this pull request in Lezek123/substrate-runtime-joystream May 21, 2020
Connecting overlay (#3)
6631261
* NotFound translatable * Connecting overlay * Update rx-react (proper state management) * Use i18n-next everywhere * @flow updates

mnaamani deleted the membership branch May 22, 2020 20:32

bedeho pushed a commit that referenced this pull request May 29, 2020
Merge pull request#3from mnaamani/joystream-update-membership-roles
3b034ad
Joystream update membership roles

bedeho mentioned this pull request Jun 6, 2020
Application staking identifiers: bloated role & mutable #591
Closed

gleb-urvanov referenced this pull request in gleb-urvanov/substrate-runtime-joystream Aug 21, 2020
Vesting appear logic (try#3) (Joystream#3087)
3d4a903

DzhideX pushed a commit to DzhideX/joystream that referenced this pull request Sep 10, 2020
Docker from CI... tryJoystream#3
6a2a3a1

shamil-gadelshin pushed a commit that referenced this pull request Sep 16, 2020
Merge pull request#3from Joystream/content_directory_second_try
c74df4f
Content directory second try

bwhm referenced this pull request in bwhm/joystream Dec 2, 2020
Merge pull request#3from Lezek123/cli-upload-video-input
bb301a9
File input to uploadVideo - suggested approach

shamil-gadelshin pushed a commit that referenced this pull request Apr 4, 2021
Merge pull request#3from mnaamani/antioch-params-small-fixes
1ec9d2f
small fixes

mnaamani pushed a commit that referenced this pull request Aug 11, 2021
Add raspberry pi binary (#3)
4fba814

Lezek123 pushed a commit that referenced this pull request Dec 16, 2021
Merge pull request#3from Lezek123/olympia-mappings-fix
7efbb8b
Fix "Trying to access property id of undefined" (candidate.member.id)