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

feat: add social service docs #233

Merged
merged 1 commit into from
Aug 30, 2023
Merged

feat: add social service docs #233

merged 1 commit into from
Aug 30, 2023

Conversation

agusaldasoro
Copy link
Contributor

@agusaldasoro agusaldasoro commented Aug 28, 2023

Add documentation to build a Social Service Client

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Aug 28, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: ff873a1
Status: ✅  Deploy successful!
Preview URL: https://41d0b673.new-docs-6m4.pages.dev
Branch Preview URL: https://feat-add-social-client.new-docs-6m4.pages.dev

View logs

@agusaldasoro agusaldasoro marked this pull request as ready for review August 29, 2023 12:50
Copy link
Contributor

@moliva moliva left a comment

Choose a reason for hiding this comment

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

Great work! Leaving some comments to clarify and improve experience.


### URLs

#### .zone Domain
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add a comment about these being dev servers

- [REST API](https://social.decentraland.zone)
- [WebSocket API](https://rpc-social.decentraland.zone)

#### .org Domain
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above but clarifying these are for production env


## Description

The authentication process for the Social Service involves interacting with the Matrix server. Users must log in to Matrix using their address, signing a fetch operation to obtain an authentication token. This token is then used to authorize interactions with the Social Service.
Copy link
Contributor

Choose a reason for hiding this comment

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

Clarify Matrix is the underlying chat platform.

Suggested change
The authentication process for the Social Service involves interacting with the Matrix server. Users must log in to Matrix using their address, signing a fetch operation to obtain an authentication token. This token is then used to authorize interactions with the Social Service.
The authentication process for the Social Service involves interacting with the [Matrix server](https://matrix.org/), used under the hood for the private chat subsystem. Users must log in to Matrix using their address, signing a fetch operation to obtain an authentication token. This token is then used to authorize interactions with the Social Service.

}
```

The authentication token, present in the access_token field, is required for subsequent interactions with the Social Service.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:

Suggested change
The authentication token, present in the access_token field, is required for subsequent interactions with the Social Service.
The authentication token, present in the `access_token` field, is required for subsequent interactions with the Social Service.


## HTTP Request

To acquire the authentication token, a POST request must be made to the following URL: `https://social.decentraland.org/_matrix/client/r0/login`
Copy link
Contributor

Choose a reason for hiding this comment

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

We can add here that for this we need to create and sign a message with AuthChain and link to the docs section.

Also comment that a more complete example will be shown below to understand how it's used.

import fetch from 'cross-fetch'
import { AuthIdentity, Authenticator } from '@dcl/crypto'

export async function getSynapseToken(synapseUrl: string, address: string, identity: AuthIdentity): Promise<string> {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could mention somewhere what Synapse is or rename these to Matrix to avoid confusion.

@@ -0,0 +1,89 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why but I see this section twice in the preview.

---
title: Login
url: /contributor/social-service/login
weight: 2
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's change these weights to have a proper order in the section. WDYT?

@@ -0,0 +1,92 @@
---
title: Javascript Client
url: /contributor/social-service/login
Copy link
Member

Choose a reason for hiding this comment

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

this URL is duplicated with another doc, each one needs to be unique or it will be impossible to reach one of the two docs

@@ -0,0 +1,69 @@
---
title: List friendships, requests and mutual friends
Copy link
Member

Choose a reason for hiding this comment

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

I would make this title shorter, remember that this title displays on the right-hand margin, and will look kind of bad if it won't fit in one line

url: /contributor/social-service/ws
weight: 1
---
# WS Implementation API for Social Service
Copy link
Member

Choose a reason for hiding this comment

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

This top level title is not necessary,

The text from the field "title" is shown as a top level title automatically, so you will end up with two large titles

---
# WS Implementation API for Social Service

## Description
Copy link
Member

Choose a reason for hiding this comment

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

For consistency with other pages, I wouldn't add a "description" title. It's already implicit that the first paragraph at the start of the doc is a high level description of what the rest of the doc is about

weight: 4
---

# Social Service Get Friendships API Documentation
Copy link
Member

Choose a reason for hiding this comment

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

Please remove all page header titles, because they end up redundant. The text from the "title" field goes automatically both to the index and to the page header

Copy link
Contributor

@moliva moliva left a comment

Choose a reason for hiding this comment

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

LGTM!

adapt docs

add link to client doc

improve docs

fix style

fix title

improve login docs

fix style

remove redundant titles
@agusaldasoro agusaldasoro merged commit a0bda97 into main Aug 30, 2023
2 checks passed
@agusaldasoro agusaldasoro deleted the feat/add-social-client branch August 30, 2023 13:30
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