Mage_Api - IDE improvements #677
Conversation
- added/fixed missing DOCs - changed @return to $this for methods - use @inheritdoc for methods that return parent call - added class check (instanceof) for those methods (instead adding @var SomeClass) - added class check (instanceof) for mysql related methods in setup scripts - changed magic methods on Varien_Objects to setData()/getData() - flipped @var annotations
|
sorry for the extra work this causes, but I think the changes inside the code (like in the method where I did comment in) should be better done in a separate PR. |
|
@Flyingmana no problem to split this. Are you okay with revert |
- _updateRoleUsersAcl() requires Mage_Api_Model_Roles, not Mage_Core_Model_Abstract
Instead of replacing magic methods on Varien_Object just for IDE, maybe it would be better to add some pseudo classes with @method annotations.
| @@ -0,0 +1,10 @@ | |||
| <?php | |||
There was a problem hiding this comment.
IDE unrelated change (new code?)
There was a problem hiding this comment.
Proposal. See #687. Should not break anything.
| @@ -0,0 +1,14 @@ | |||
| <?php | |||
There was a problem hiding this comment.
IDE unrelated change (new code)
There was a problem hiding this comment.
Proposal. See #687. Should not break anything.
| @@ -49,6 +49,9 @@ public function assert(Mage_Api_Model_Acl $acl, Mage_Api_Model_Acl_Role $role = | |||
| return $this->_isCleanTime(time()); | |||
| } | |||
|
|
|||
| /** | |||
| * @param $time | |||
There was a problem hiding this comment.
no benefit without type for IDE
There was a problem hiding this comment.
Method has no content. Just supress PHPStorm warning ;)
| @@ -354,7 +354,7 @@ public function add(Mage_Core_Model_Abstract $user) | |||
| if ($user->getId() > 0) { | |||
| $role = Mage::getModel('api/role')->load($user->getRoleId()); | |||
| } else { | |||
| $role = new Varien_Object(array('tree_level' => 0)); | |||
| $role = new Mage_Api_Helper_Object_Role(array('tree_level' => 0)); | |||
| @@ -51,7 +54,7 @@ public function __construct($sourceData=null) | |||
| unset($queryParams['wsdl']); | |||
|
|
|||
| // set up default WSDL template variables | |||
| $this->_wsdlVariables = new Varien_Object( | |||
| $this->_wsdlVariables = new Mage_Api_Helper_Object_Wsdl( | |||
There was a problem hiding this comment.
Proposal. Please see #687
added class check (instanceof) for those methods (instead adding @var SomeClass)added class check (instanceof) for mysql related methods in setup scriptschanged typehint to concrete class instead of Mage_Core_Model_Abstract