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(OpenAPI): Add missing reactionsSelf to ChatMessage #12386

Merged
merged 1 commit into from
May 22, 2024

Conversation

provokateurin
Copy link
Member

Psalm probably didn't catch the missing field because the logic for setting it is too complicated and it can't figure out what is happening.

// Inject the reactions self into the $messages array
foreach ($reactionsById as $messageId => $reactions) {
if (isset($commentIdToIndex[$messageId]) && isset($messages[$commentIdToIndex[$messageId]])) {
$messages[$commentIdToIndex[$messageId]]['reactionsSelf'] = $reactions;
}
// Add the self part also to potential parent elements
if (isset($parentMap[$messageId])) {
foreach ($parentMap[$messageId] as $mid) {
if (isset($messages[$commentIdToIndex[$mid]])) {
$messages[$commentIdToIndex[$mid]]['parent']['reactionsSelf'] = $reactions;
}
}
}
}

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not possible
  • 📘 API documentation in docs/ has been updated or is not required
  • 🔖 Capability is added or not needed

Signed-off-by: provokateurin <kate@provokateurin.de>
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Makes sense, looking at TS output

@provokateurin
Copy link
Member Author

@Antreesy do you know why typescript wasn't complaining that a non-existent property of an object was used?

@provokateurin provokateurin merged commit aea691b into main May 22, 2024
64 checks passed
@provokateurin provokateurin deleted the fix/openapi/chat-message-reactions-self branch May 22, 2024 05:34
@nickvergessen
Copy link
Member

/backport to stable29

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.

3 participants