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

Support hydrating embedded threads in incoming webhooks #289

Merged
merged 2 commits into from
Dec 18, 2023

Conversation

matt-h
Copy link
Contributor

@matt-h matt-h commented Mar 31, 2022

This pulls the code from #208

It keeps just the hydrating embedded threads part of that pull request while removing the part that added withEmbed to conversation filters.

I'm pulling this back up because when getting webhooks with the IncomingWebhook class the threads are all embedded in the request, but it is impossible to access them since they are never hydrated.

The only thing that might be missing here is getting the correct value for ->getThreadCount().

Currently the webhooks I'm getting are setting threads to 1 which in this code:

if (is_numeric($data['threads'])) {
     $this->setThreadCount($data['threads']);
}

Sets it to that. I'm not sure if embedded threads should do anything to change that without knowing how the underlying system works so I'll leave that up to you if that needs to be modified more. This change doesn't change that at all from how it currently works anyway. And this change allows me to use ->getThreads() which is the goal of what I'm trying to do with this.

cesutherland and others added 2 commits March 30, 2022 22:08
This pulls the code from helpscout#208

It keeps just the hydrating embedded threads while removing the part
that added withEmbed to conversation filters.

Co-authored-by: Ammeded by Matt Harrison <matt@hallme.com>
@miguelrs miguelrs changed the title Support hydrating embedded threads. Support hydrating embedded threads in incoming webhooks Dec 18, 2023
@miguelrs miguelrs merged commit 3fbe4d4 into helpscout:master Dec 18, 2023
1 check passed
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.

3 participants