Skip to content

Conversation

weilai-irl
Copy link
Collaborator

No description provided.

@weilai-irl weilai-irl requested a review from Copilot June 30, 2025 10:37
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures that get_description() in the action_failed event always returns a string by JSON-encoding its structured payload.

  • Changed the return value of get_description() from raw data to a JSON-encoded string.
  • Updated the implementation in auth/oidc/classes/event/action_failed.php.
Comments suppressed due to low confidence (2)

auth/oidc/classes/event/action_failed.php:44

  • Update the docblock to note that the returned string is a JSON-encoded representation of the other data, so future readers know the format.
     * @return string

auth/oidc/classes/event/action_failed.php:47

  • Add or update unit tests to verify that get_description() correctly JSON-encodes various other payload structures and handles encoding errors as expected.
        return json_encode($this->data['other']);

*/
public function get_description() {
return $this->data['other'];
return json_encode($this->data['other']);
Copy link

Copilot AI Jun 30, 2025

Choose a reason for hiding this comment

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

Consider adding JSON_THROW_ON_ERROR (e.g., json_encode(..., JSON_THROW_ON_ERROR)) to catch encoding failures explicitly instead of returning false on error.

Suggested change
return json_encode($this->data['other']);
try {
return json_encode($this->data['other'], JSON_THROW_ON_ERROR);
} catch (\JsonException $e) {
// Log the error or handle it appropriately.
error_log('JSON encoding error: ' . $e->getMessage());
return '{}'; // Return an empty JSON object as a fallback.
}

Copilot uses AI. Check for mistakes.

@weilai-irl weilai-irl added this to the 2025-02 milestone Jun 30, 2025
@weilai-irl weilai-irl self-assigned this Jul 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant