Skip to content
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

Fix undefined array indices while updating comments via API #4208

Merged
merged 1 commit into from Jun 24, 2016

Conversation

gravityrail
Copy link
Contributor

Fixes #3687

Fixes a bug checking for comment privileges, and ensures an array index exists when checking if an authenticated user can read a comment.

@@ -81,6 +81,8 @@ function get_comment( $comment_id, $context ) {
&&
isset( $this->api->token_details['user'] )
&&
isset( $this->api->token_details['user']['user_email'] )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seeing this, shouldn't we check for isset( $this->api->token_details['user']['display_name'] ) too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, but that wasn't the error. Maybe that value is always set?

@lezama lezama added the [Status] Ready to Merge Go ahead, you can push that green button! label Jun 24, 2016
@lezama
Copy link
Contributor

lezama commented Jun 24, 2016

Thanks! :shipit: once tests pass.

@gravityrail gravityrail merged commit 8033b42 into master Jun 24, 2016
@gravityrail gravityrail deleted the fix/undefined-indices-updating-comment branch June 24, 2016 22:02
@matticbot matticbot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Jun 24, 2016
@jeherve jeherve added this to the 4.2 milestone Jun 26, 2016
jeherve added a commit that referenced this pull request Jul 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants