-
Notifications
You must be signed in to change notification settings - Fork 506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
API should not support adding child categories with different permission than parent #2481
Conversation
@@ -162,8 +160,30 @@ public function delete(Entity $entity) | |||
public function deleteTag($id) | |||
{ | |||
// Remove tag from attribute options | |||
$this->removeTagFromAttributeOptions($entity->id); | |||
|
|||
$this->removeTagFromAttributeOptions($id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really not sure why this was using $entity->id since there was not entity to use. I could revert it .. it's probably not being used . :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm yeah this must not have been used anymore
@willdoran can you take a look at this PR? I'm not sure TBH, I think isRoleValid could move to a trait and have a better name, (besides the implementation itself) ?? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
@@ -162,8 +160,30 @@ public function delete(Entity $entity) | |||
public function deleteTag($id) | |||
{ | |||
// Remove tag from attribute options | |||
$this->removeTagFromAttributeOptions($entity->id); | |||
|
|||
$this->removeTagFromAttributeOptions($id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm yeah this must not have been used anymore
This pull request makes the following changes:
Test checklist:
[ ]
I certify that I ran my checklist
Fixes ushahidi/platform# .
Ping @ushahidi/platform