Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add new API appservice specific public room list #1676

Merged
merged 7 commits into from Dec 12, 2016
Merged

Conversation

erikjohnston
Copy link
Member

@erikjohnston erikjohnston commented Dec 6, 2016

This adds the following:

  • A instance_id field to each instance in the GET /thirdparty/protocols API
  • An API that allows ASes to publish rooms to their own dedicated lists, PUT /directory/list/appservice/$network_id/$room_id
  • Two extra params to POST /publicRooms, third_party_instance_id which returns rooms published to AS instance specific lists, and include_all_networks which if true returns all rooms that have been published to any list. By default it will only return rooms on the main, default list.

https://docs.google.com/document/d/12mVuOT7Qoa49L_PQAPjavoK9c2nalYEFOHxJOmH5j-w/edit?usp=sharing

def edit_published_appservice_room_list(self, appservice_id, network_id,
room_id, visibility):
"""Edit the appservice/network specific public room list.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we have some docstring for the parameters here?

@defer.inlineCallbacks
def set_room_is_public_appservice(self, room_id, appservice_id, network_id,
is_public):
"""Edit the appservice/network specific public room list.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we have docstring for what the params are and what this does?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

room_id TEXT NOT NULL
);

CREATE UNIQUE INDEX appservice_room_list_idx ON appservice_room_list(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add comment on what this table is for?

appservice and network id to use an appservice specific one.
Setting to None returns all public rooms across all lists.
"""
if search_filter or network_tuple is not (None, None):
# We explicitly don't bother caching searches.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the comment need updating if we aren't caching the appservice lists as well?

@NegativeMjark
Copy link
Contributor

What is an appservice_id look like ooi?

@erikjohnston
Copy link
Member Author

What is an appservice_id look like ooi?

e.g. irc-freenode or 9666621b268e14d7b18b110073f07c6e955c60a8c8a5792fd810e9978d1931d7

@erikjohnston
Copy link
Member Author

Now with added sytests matrix-org/sytest#328!

@lukebarnard1
Copy link
Contributor

Is it really instance_id? Because I can't see any sign of that in the code...

@erikjohnston
Copy link
Member Author

@erikjohnston
Copy link
Member Author

Will probably change it to network_id in the AS layer, and instance_id in the client layer, if that sounds good to you @lukebarnard1 ?

lukebarnard1 pushed a commit to matrix-org/matrix-js-sdk that referenced this pull request Dec 12, 2016
This is for setting the publicity of a room that is bridged to a 3rd party network. This change reflects the second bullet point of matrix-org/synapse#1676 (comment).
Copy link
Contributor

@NegativeMjark NegativeMjark left a comment

Choose a reason for hiding this comment

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

LGTM

@erikjohnston
Copy link
Member Author

@matrixbot retest this please

@erikjohnston erikjohnston merged commit 1574b83 into develop Dec 12, 2016
dbkr added a commit to element-hq/element-web that referenced this pull request Dec 14, 2016
As per matrix-org/synapse#1676

The existing local config system still exists and is used for
remote home server directories (since /thirdparty/protocols
doesn't support listing remote home servers, and also because
people are using it).
Half-Shot pushed a commit to Half-Shot/riot-web that referenced this pull request Feb 9, 2017
As per matrix-org/synapse#1676

The existing local config system still exists and is used for
remote home server directories (since /thirdparty/protocols
doesn't support listing remote home servers, and also because
people are using it).
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants