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.

Sign up for GitHub

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

Jump to bottom

Support standalone signaling server for chat #890

Merged
merged 3 commits into from
Jun 15, 2018
Merged

Support standalone signaling server for chat #890

merged 3 commits into from
Jun 15, 2018

Conversation

fancycode
Copy link
Member

@fancycode fancycode commented May 9, 2018 •
edited
Loading

Implements #624. Based on #887 which should be merged first.

@fancycode fancycode added feature: chat 💬 Chat and system messages feature: signaling 📶 Internal and external signaling backends labels May 9, 2018
@fancycode fancycode changed the title Support standalone signaling server for chat (#624) Support standalone signaling server for chat May 9, 2018
@fancycode
Copy link
Member Author

This is a first improvement in that the polling gets removed, the messages are still loaded from PHP.

In a future step, this could be changed further so that the messages are sent through the WebSocket and no more PHP calls are necessary (besides loading the initial list of past messages).

@nickvergessen nickvergessen added this to the 4.0 (Nextcloud 14) milestone May 11, 2018
fancycode added 3 commits June 12, 2018 12:05
@fancycode
Use standalone signaling server to notify about new chat messages.
a5b899c 
This removes the long-polling against the PHP backend and instead sends an
event to all users in a room through the standalone signaling server to
notify that new chat messages exist.

Signed-off-by: Joachim Bauch <bauch@struktur.de>
@fancycode
Load potentially missed chat messages after reconnect.
890e42e 
Signed-off-by: Joachim Bauch <bauch@struktur.de>
@fancycode
Attempt to fix broken tests.
191075b 
Signed-off-by: Joachim Bauch <bauch@struktur.de>
@fancycode fancycode changed the base branch from chat-signaling to master June 12, 2018 10:05
@fancycode fancycode force-pushed the chat-standalone-signaling branch from 27d55af to 191075b Compare June 12, 2018 10:13
@Ivansss Ivansss merged commit 22e8ca7 into master Jun 15, 2018
@Ivansss Ivansss deleted the chat-standalone-signaling branch June 15, 2018 07:46
@nickvergessen
Copy link
Member

You guys forgot the backport label :(

@fancycode
Copy link
Member Author

This will also need a backport of #887 then.

@nickvergessen
Copy link
Member

And #398 :(

@fancycode
Copy link
Member Author

Btw, what are the guidelines for backporting issues? From what I understand from the past, only bug fixes should be backported but no new features.

@nickvergessen
Copy link
Member

In this app we backport everything which is not server-version dependent.
So that all features are always available in the latest supported server version (atm 13).

The diff between master + stable13 (or in the future stable14) should only be code that is necessary for server master, e.g. a lot of CSS changes are necessary and personal settings module was changed.

@fancycode
Copy link
Member Author

Ok, thanks for clarification. I'll add the backport-request label accordingly in the future.

@fancycode fancycode mentioned this pull request Jul 12, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ivansss Ivansss Ivansss approved these changes

Assignees
No one assigned
Labels
3. to review feature: chat 💬 Chat and system messages feature: signaling 📶 Internal and external signaling backends
Projects
None yet
Milestone
4.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@fancycode @nickvergessen @Ivansss