When adding a new domain, pass in parent domain id

as opposed to local skill id

contracts/Colony.sol

@@ -119,19 +119,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);

contracts/ColonyStorage.sol

@@ -170,12 +170,6 @@ contract ColonyStorage is DSAuth {
     _;
   }
 
-  modifier localSkill(uint256 _skillId) {
-    IColonyNetwork colonyNetworkContract = IColonyNetwork(colonyNetworkAddress);
-    require(!colonyNetworkContract.isGlobalSkill(_skillId));
-    _;
-  }
-
   modifier skillExists(uint256 _skillId) {
     IColonyNetwork colonyNetworkContract = IColonyNetwork(colonyNetworkAddress);
     require(_skillId <= colonyNetworkContract.getSkillCount());

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

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");
     });
 

test/meta-colony.js

@@ -262,7 +262,7 @@ 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(), 3);
@@ -283,8 +283,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(3));
+      await metaColony.addDomain(1);
+      await checkErrorRevert(metaColony.addDomain(2));
 
       const skillCount = await colonyNetwork.getSkillCount.call();
       assert.equal(skillCount.toNumber(), 3);
@@ -305,13 +305,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(3);
-      await colony.addDomain(3);
-      await colony.addDomain(3);
+      await colony.addDomain(1);
+      await colony.addDomain(1);
+      await colony.addDomain(1);
 
       const skillCount = await colonyNetwork.getSkillCount.call();
       assert.equal(skillCount.toNumber(), 6);
@@ -343,25 +343,6 @@ contract("Meta Colony", accounts => {
       assert.equal(rootSkillChild3.toNumber(), 6);
     });
 
-    it("should NOT be able to add a child local skill more than one level from the root local skill", async () => {
-      await colony.addDomain(3);
-      await checkErrorRevert(colony.addDomain(4));
-
-      const skillCount = await colonyNetwork.getSkillCount.call();
-      assert.equal(skillCount.toNumber(), 4);
-      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(), 3);
-      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));
 
@@ -388,7 +369,7 @@ contract("Meta Colony", accounts => {
     });
 
     it("should be able to set domain on task", async () => {
-      await colony.addDomain(3);
+      await colony.addDomain(1);
       const taskId = await makeTask({ colony });
 
       await colony.setTaskDomain(taskId, 2);
@@ -398,7 +379,7 @@ contract("Meta Colony", accounts => {
     });
 
     it("should NOT allow a non-manager to set domain on task", async () => {
-      await colony.addDomain(3);
+      await colony.addDomain(1);
       await makeTask({ colony });
       await checkErrorRevert(colony.setTaskDomain(1, 2, { from: OTHER_ACCOUNT }));
       const task = await colony.getTask.call(1);