diff --git a/contracts/Colony.sol b/contracts/Colony.sol index de38c17303..1b6169479f 100755 --- a/contracts/Colony.sol +++ b/contracts/Colony.sol @@ -117,19 +117,17 @@ contract Colony is ColonyStorage, PatriciaTreeProofs { return colonyNetwork.addSkill(_parentSkillId, true); } - function addDomain(uint256 _parentSkillId) public + function addDomain(uint256 _parentDomainId) public auth - localSkill(_parentSkillId) { - // Note: remove that when we start allowing more domain hierarchy levels - // Instead check that the parent skill id belongs to this colony own domain - // Get the local skill id of the root domain - uint256 rootDomainSkillId = domains[1].skillId; - require(_parentSkillId == rootDomainSkillId, "colony-parent-skill-not-root"); + // Note: Remove when we want to allow more domain hierarchy levels + require(_parentDomainId == 1, "colony-parent-skill-not-root"); + + uint256 parentSkillId = domains[_parentDomainId].skillId; // Setup new local skill IColonyNetwork colonyNetwork = IColonyNetwork(colonyNetworkAddress); - uint256 newLocalSkill = colonyNetwork.addSkill(_parentSkillId, false); + uint256 newLocalSkill = colonyNetwork.addSkill(parentSkillId, false); // Add domain to local mapping initialiseDomain(newLocalSkill); diff --git a/contracts/ColonyStorage.sol b/contracts/ColonyStorage.sol index 56e74a3126..9faa28c548 100755 --- a/contracts/ColonyStorage.sol +++ b/contracts/ColonyStorage.sol @@ -164,12 +164,6 @@ contract ColonyStorage is DSAuth, DSMath { _; } - modifier localSkill(uint256 _skillId) { - IColonyNetwork colonyNetworkContract = IColonyNetwork(colonyNetworkAddress); - require(!colonyNetworkContract.isGlobalSkill(_skillId)); - _; - } - modifier skillExists(uint256 _skillId) { IColonyNetwork colonyNetworkContract = IColonyNetwork(colonyNetworkAddress); require(_skillId <= colonyNetworkContract.getSkillCount()); diff --git a/contracts/IColony.sol b/contracts/IColony.sol index 3ee519854a..a8f5bf6bae 100644 --- a/contracts/IColony.sol +++ b/contracts/IColony.sol @@ -167,8 +167,9 @@ contract IColony { /// @notice Add a colony domain, and its respective local skill under skill with id `_parentSkillId` /// New funding pot is created and associated with the domain here - /// @param _parentSkillId Id of the local skill under which the new skill will be added - function addDomain(uint256 _parentSkillId) public; + /// @param _parentDomainId Id of the domain under which the new one will be added + /// @dev Adding new domains is currently retricted to one level only, i.e. `_parentDomainId` has to be the root domain id: 1 + function addDomain(uint256 _parentDomainId) public; /// @notice Get the domain's local skill and funding pot id /// @param _id Id of the domain which details to get diff --git a/test/colony.js b/test/colony.js index 8e1665a5f2..8c8acf9189 100755 --- a/test/colony.js +++ b/test/colony.js @@ -261,8 +261,7 @@ contract("Colony", addresses => { describe("when adding domains", () => { it("should log DomainAdded and PotAdded events", async () => { - const skillCount = await colonyNetwork.getSkillCount.call(); - await expectAllEvents(colony.addDomain(skillCount.toNumber()), ["DomainAdded", "PotAdded"]); + await expectAllEvents(colony.addDomain(1), ["DomainAdded", "PotAdded"]); }); }); @@ -304,8 +303,7 @@ contract("Colony", addresses => { }); it("should set the task domain correctly", async () => { - const skillCount = await colonyNetwork.getSkillCount.call(); - await colony.addDomain(skillCount.toNumber()); + await colony.addDomain(1); await makeTask({ colony, domainId: 2 }); const task = await colony.getTask.call(1); assert.equal(task[8].toNumber(), 2); @@ -1247,11 +1245,7 @@ contract("Colony", addresses => { it("should log a TaskDomainChanged event, if the task domain gets changed", async () => { const taskId = await makeTask({ colony }); - - // Create a domain, change task's domain - const skillCount = await colonyNetwork.getSkillCount.call(); - await colony.addDomain(skillCount.toNumber()); - + await colony.addDomain(1); await expectEvent(colony.setTaskDomain(taskId, 2), "TaskDomainChanged"); }); diff --git a/test/meta-colony.js b/test/meta-colony.js index 3cfe8f8211..af17756367 100644 --- a/test/meta-colony.js +++ b/test/meta-colony.js @@ -262,7 +262,8 @@ contract("Meta Colony", accounts => { describe("when adding domains in the meta colony", () => { it("should be able to add new domains as children to the root domain", async () => { - await metaColony.addDomain(2); + await metaColony.addDomain(1); + const skillCount = await colonyNetwork.getSkillCount.call(); assert.equal(skillCount.toNumber(), 4); const domainCount = await metaColony.getDomainCount.call(); @@ -283,8 +284,8 @@ contract("Meta Colony", accounts => { }); it("should NOT be able to add a child local skill more than one level from the root local skill", async () => { - await metaColony.addDomain(2); - await checkErrorRevert(metaColony.addDomain(4)); + await metaColony.addDomain(1); + await checkErrorRevert(metaColony.addDomain(2)); const skillCount = await colonyNetwork.getSkillCount.call(); assert.equal(skillCount.toNumber(), 4); @@ -305,13 +306,13 @@ contract("Meta Colony", accounts => { }); it("someone who does not have owner role should not be able to add domains", async () => { - await checkErrorRevert(colony.addDomain(3, { from: OTHER_ACCOUNT })); + await checkErrorRevert(colony.addDomain(1, { from: OTHER_ACCOUNT })); }); it("should be able to add new domains as children to the root domain", async () => { - await colony.addDomain(4); - await colony.addDomain(4); - await colony.addDomain(4); + await colony.addDomain(1); + await colony.addDomain(1); + await colony.addDomain(1); const skillCount = await colonyNetwork.getSkillCount.call(); assert.equal(skillCount.toNumber(), 7); @@ -342,25 +343,6 @@ contract("Meta Colony", accounts => { assert.equal(rootSkillChild3.toNumber(), 7); }); - it("should NOT be able to add a child local skill more than one level from the root local skill", async () => { - await colony.addDomain(4); - await checkErrorRevert(colony.addDomain(5)); - - const skillCount = await colonyNetwork.getSkillCount.call(); - assert.equal(skillCount.toNumber(), 5); - const domainCount = await colony.getDomainCount.call(); - assert.equal(domainCount.toNumber(), 2); - }); - - it("should NOT be able to add a child local skill to a global skill parent", async () => { - await checkErrorRevert(colony.addDomain(1)); - - const skillCount = await colonyNetwork.getSkillCount.call(); - assert.equal(skillCount.toNumber(), 4); - const domainCount = await colony.getDomainCount.call(); - assert.equal(domainCount.toNumber(), 1); - }); - it("should NOT be able to add a new local skill by anyone but a Colony", async () => { await checkErrorRevert(colonyNetwork.addSkill(2, false)); @@ -387,8 +369,7 @@ contract("Meta Colony", accounts => { }); it("should be able to set domain on task", async () => { - const colonyRootDomain = await colony.getDomain(1); - await colony.addDomain(colonyRootDomain[0].toString()); + await colony.addDomain(1); const taskId = await makeTask({ colony }); await colony.setTaskDomain(taskId, 2); @@ -398,9 +379,7 @@ contract("Meta Colony", accounts => { }); it("should NOT allow a non-manager to set domain on task", async () => { - const colonyRootDomain = await colony.getDomain(1); - await colony.addDomain(colonyRootDomain[0].toString()); - + await colony.addDomain(1); await makeTask({ colony }); await checkErrorRevert(colony.setTaskDomain(1, 2, { from: OTHER_ACCOUNT })); const task = await colony.getTask.call(1);