From 191d06e685836b0cb226431c7f2d6882219ef128 Mon Sep 17 00:00:00 2001 From: "lu.kevin@berkeley.edu" Date: Mon, 16 Jul 2018 18:20:12 -0700 Subject: [PATCH] Return a isGlobalSkill boolean flag in ColonyNetwork.getSkill --- contracts/ColonyNetwork.sol | 5 +++-- contracts/ColonyNetworkStorage.sol | 2 +- contracts/IColonyNetwork.sol | 3 ++- contracts/ReputationMiningCycle.sol | 2 +- test/meta-colony.js | 12 ++++++++++++ 5 files changed, 19 insertions(+), 5 deletions(-) diff --git a/contracts/ColonyNetwork.sol b/contracts/ColonyNetwork.sol index c8c741d3a1..4ab1f6fe46 100644 --- a/contracts/ColonyNetwork.sol +++ b/contracts/ColonyNetwork.sol @@ -82,8 +82,9 @@ contract ColonyNetwork is ColonyNetworkStorage { return colonyVersionResolver[_version]; } - function getSkill(uint256 _skillId) public view returns (uint256, uint256) { - return (skills[_skillId].nParents, skills[_skillId].nChildren); + function getSkill(uint256 _skillId) public view returns (uint256, uint256, bool) { + Skill storage skill = skills[_skillId]; + return (skill.nParents, skill.nChildren, skill.globalSkill); } function isGlobalSkill(uint256 _skillId) public view returns (bool) { diff --git a/contracts/ColonyNetworkStorage.sol b/contracts/ColonyNetworkStorage.sol index 2e1f8524fb..0729a06213 100644 --- a/contracts/ColonyNetworkStorage.sol +++ b/contracts/ColonyNetworkStorage.sol @@ -53,7 +53,7 @@ contract ColonyNetworkStorage is DSAuth, DSMath { // `true` for a global skill reused across colonies or `false` for a skill mapped to a single colony's domain bool globalSkill; } - // Contains all global and local skills in the network, mapping skillId to Skill. Where skillId is 1-based unique identofier + // Contains all global and local skills in the network, mapping skillId to Skill. Where skillId is 1-based unique identifier mapping (uint256 => Skill) skills; // Number of skills in the network, including both global and local skills uint256 skillCount; diff --git a/contracts/IColonyNetwork.sol b/contracts/IColonyNetwork.sol index a556802e24..e8bd6eb752 100644 --- a/contracts/IColonyNetwork.sol +++ b/contracts/IColonyNetwork.sol @@ -68,7 +68,8 @@ contract IColonyNetwork { /// @param _skillId Id of the skill /// @return nParents uint256 `skill.nParents` i.e. the number of parent skills of skill with id `_skillId` /// @return nChildren uint256 `skill.nChildren` i.e. the number of child skills of skill with id `_skillId` - function getSkill(uint256 _skillId) public view returns (uint256 nParents, uint256 nChildren); + /// @return isGlobalSkill true if specified skill is a global skill, otherwise false + function getSkill(uint256 _skillId) public view returns (uint256 nParents, uint256 nChildren, bool isGlobalSkill); /// @notice Checks if skill with id `_skillId` is a global skill /// @param _skillId Id of the skill diff --git a/contracts/ReputationMiningCycle.sol b/contracts/ReputationMiningCycle.sol index f92115623d..f40149c8b3 100644 --- a/contracts/ReputationMiningCycle.sol +++ b/contracts/ReputationMiningCycle.sol @@ -593,7 +593,7 @@ contract ReputationMiningCycle is PatriciaTreeProofs, DSMath { // We update skills in the order children, then parents, then the skill listed in the log itself. // If the amount in the log is positive, then no children are being updated. uint nParents; - (nParents, ) = IColonyNetwork(colonyNetworkAddress).getSkill(logEntry.skillId); + (nParents, , ) = IColonyNetwork(colonyNetworkAddress).getSkill(logEntry.skillId); uint nChildUpdates; if (logEntry.amount >= 0) { // solium-disable-line no-empty-blocks, whitespace // Then we have no child updates to consider diff --git a/test/meta-colony.js b/test/meta-colony.js index 2cfaf1809a..f3ef5c2c1f 100644 --- a/test/meta-colony.js +++ b/test/meta-colony.js @@ -463,4 +463,16 @@ contract("Meta Colony", accounts => { await checkErrorRevert(colony.setTaskSkill(1, 3)); }); }); + + describe("when getting a skill", () => { + it("should return a true flag if the skill is global", async () => { + const globalSkill = await colonyNetwork.getSkill.call(1); + assert.isTrue(globalSkill[2]); + }); + + it("should return a false flag if the skill is local", async () => { + const localSkill = await colonyNetwork.getSkill.call(2); + assert.isFalse(localSkill[2]); + }); + }); });