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

Add conversation participants support #241

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thewheat
Copy link
Contributor

Adding support for adding and removing of participants from group conversations

  • Created a Participant class as we're using Customer in the response for conversations ratings Add conversation ratings support #235 and also in the return response for the added methods
  • Open to feedback on a better approach for this

image

Related Docs:

@@ -178,7 +183,7 @@ private HttpURLConnection initializeConnection(URI uri, String method) throws IO
}

private boolean shouldSkipResponseEntity(JavaType javaType, HttpURLConnection conn, int responseCode) {
return responseCode == 204 || Void.class.equals(javaType.getRawClass()) || "DELETE".equals(conn.getRequestMethod());
return responseCode == 204 || Void.class.equals(javaType.getRawClass());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed to parse response data. But in further testing this breaks Tag.delete

@thewheat
Copy link
Contributor Author

thewheat commented Oct 28, 2018

Should not be merged as it breaks Tag.delete due to this change https://github.com/intercom/intercom-java/pull/241/files#r228734640
Will need to rethink this a bit more but also open for discussion regarding Customer and Participant calsses

@choran
Copy link
Member

choran commented Nov 2, 2018

@thewheat ok .. will leave this for now and focus on other open PRs

@thewheat
Copy link
Contributor Author

thewheat commented Nov 5, 2018

For reference PR to parse DELETE requests handled here #248

Will need to merge that first before merging this

@thewheat thewheat force-pushed the timlim/add-participant-functionality branch from d1a24e8 to 7b4f618 Compare November 13, 2018 13:44
@thewheat thewheat force-pushed the timlim/add-participant-functionality branch from 7b4f618 to 4b63f9e Compare November 13, 2018 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants