Skip to content

Conversation

@iorveth
Copy link
Contributor

@iorveth iorveth commented Sep 15, 2020

This PR aims to

  1. Refactor proposals_codex pallet according to Content directory integration work, part 2: Update proposals-codex pallet #1295
  2. Fix check in ensure_entities_creation_limits_are_valid function.

@iorveth iorveth marked this pull request as ready for review September 16, 2020 15:11
@iorveth iorveth changed the base branch from content_directory_second_try to content_dir_2 September 16, 2020 15:57
@iorveth iorveth changed the base branch from content_dir_2 to content_directory_second_try September 16, 2020 15:57
Copy link
Contributor

@shamil-gadelshin shamil-gadelshin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@bedeho bedeho left a comment

Choose a reason for hiding this comment

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

  1. Haven't you forgot to update this so that the proposals actually work for the new working group? At least you would need to change enum common::WorkingGroup, no?

  2. Also, some tests should have failed if I am correct, so there may be missing tests that actually attempt to submit proposals to this new working group

@iorveth iorveth requested a review from bedeho September 21, 2020 16:45
@iorveth
Copy link
Contributor Author

iorveth commented Sep 21, 2020

Thanks for pointing an issue!

Copy link
Member

@bedeho bedeho left a comment

Choose a reason for hiding this comment

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

  • Good fix on <= bug.
  • Good fix on adding Content variant.
  • You seem to have just replaced away all tests for Storage? dont we need tests to cover all active groups ? So we need tests for both storage and content, and whenever a new group is added, we need tests for that also? Can't you just in each test case basically iterate over all variants and call some core test logic for each case? So for example something like
#[test]
fn create_set_working_group_leader_reward_proposal_common_checks_succeed() {
    // This uses strum crate for enum iteration
    for group in WorkingGroup::iter() {
        run_create_set_working_group_leader_reward_proposal_common_checks_succeed(group);
    }
}

fn run_create_set_working_group_leader_reward_proposal_common_checks_succeed(group: WorkingGroup) {
initial_test_ext().execute_with(|| {
        let proposal_fixture = ProposalTestFixture {
            insufficient_rights_call: || {
                ProposalCodex::create_set_working_group_leader_reward_proposal(
                    RawOrigin::None.into(),
                    1,
                    b"title".to_vec(),
                    b"body".to_vec(),
                    None,
                    0,
                    10,
                    group,
                )
            },
            empty_stake_call: || {
                ProposalCodex::create_set_working_group_leader_reward_proposal(
                    RawOrigin::Signed(1).into(),
                    1,
                    b"title".to_vec(),
                    b"body".to_vec(),
                    None,
                    0,
                    10,
                    group,
                )
            },
            invalid_stake_call: || {
                ProposalCodex::create_set_working_group_leader_reward_proposal(
                    RawOrigin::Signed(1).into(),
                    1,
                    b"title".to_vec(),
                    b"body".to_vec(),
                    Some(<BalanceOf<Test>>::from(5000u32)),
                    0,
                    10,
                    group,
                )
            },
            successful_call: || {
                ProposalCodex::create_set_working_group_leader_reward_proposal(
                    RawOrigin::Signed(1).into(),
                    1,
                    b"title".to_vec(),
                    b"body".to_vec(),
                    Some(<BalanceOf<Test>>::from(50000u32)),
                    10,
                    10,
                    group,
                )
            },
            proposal_parameters:
                crate::proposal_types::parameters::set_working_group_leader_reward_proposal::<Test>(
                ),
            proposal_details: ProposalDetails::SetWorkingGroupLeaderReward(
                10,
                10,
                    group,
            ),
        };
        proposal_fixture.check_all();
    });
}
``` 

@iorveth iorveth requested a review from bedeho September 28, 2020 15:28
Copy link
Member

@bedeho bedeho left a comment

Choose a reason for hiding this comment

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

Nice work.

@bedeho bedeho merged commit d8bba20 into Joystream:content_directory_second_try Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants