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

Implement API for setting RM #419

Merged
merged 9 commits into from
Apr 20, 2017
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/base-apis.js
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,33 @@ MatrixBaseApis.prototype.roomInitialSync = function(roomId, limit, callback) {
);
};

/**
* Set a marker to represent which event the user was last reading in a room. This can
Copy link
Member

Choose a reason for hiding this comment

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

it's not the event the user was last reading though, is it?

* be retrieved from room account data and displayed as a horizontal line in the client
* that is visually distinct to the position of the user's own read receipt.
* @param {string} roomId ID of the room that has been read
* @param {string} rmEventId ID of the event that has been read
* @param {string} rrEventId ID of the event tracked by the read receipt. This is here
* for convenience because the RR and the RM are commonly updated at the same time as
* each other. Optional.
* @return {module:client.Promise} Resolves: (@see module:http-api.authedRequest)
* @return {module:http-api.MatrixError} Rejects: with an error response.
Copy link
Member

Choose a reason for hiding this comment

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

it never returns a MatrixError; it always returns a Promise.

Sorry, I should have been clearer about what to put here. There are a lot of historical functions (including authedRequest) which are basically full of shit. Note also that httpApi.opts.onlyData is true as far as base-apis is concerned, so authedRequest always just returns the result of the api call, so linking to that isn't particularly helpful.

Basically the best thing to do here is just have one @return line, along the lines of:

@return {module:client.Promise} resolves to the result object for the API call.

If you have the time and energy to document the shame of that object, so much the better, but generally we expect people to go and read the spec.

*/
MatrixBaseApis.prototype.setRoomReadMarkers = function(roomId, rmEventId, rrEventId) {
const path = utils.encodeUri("/rooms/$roomId/read_markers", {
$roomId: roomId,
});

const content = {
"m.fully_read": rmEventId,
"m.read": rrEventId,
};

return this._http.authedRequest(
undefined, "POST", path, undefined, content,
);
};


// Room Directory operations
// =========================
Expand Down
27 changes: 27 additions & 0 deletions src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -1201,6 +1201,33 @@ MatrixClient.prototype.sendReadReceipt = function(event, callback) {
return this.sendReceipt(event, "m.read", callback);
};

/**
Copy link
Member

Choose a reason for hiding this comment

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

jsdocs need tweaks as above

* Set a marker to represent which event the user was last reading in a room. This can
* be retrieved from room account data and displayed as a horizontal line in the client
* that is visually distinct to the position of the user's own read receipt.
* @param {string} roomId ID of the room that has been read
* @param {string} eventId ID of the event that has been read
* @param {string} rrEvent the event tracked by the read receipt. This is here for
* convenience because the RR and the RM are commonly updated at the same time as each
* other. Optional.
* @return {module:client.Promise} Resolves: (@see module:http-api.authedRequest)
* @return {module:http-api.MatrixError} Rejects: with an error response.
*/
MatrixClient.prototype.setRoomReadMarker = function(roomId, eventId, rrEvent) {
Copy link
Member

Choose a reason for hiding this comment

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

let's not distinguish these two by the presence or absence of 's'.

How about calling the other setRoomReadMarkersHttpRequest or something (we have a _sendEventHttpRequest for distinction from _sendEvent)

const rmEventId = eventId;
let rrEventId;

// Add the optional RR update, do local echo like `sendReceipt`
if (rrEvent) {
rrEventId = rrEvent.getId();
const room = this.getRoom(roomId);
if (room) {
room._addLocalEchoReceipt(this.credentials.userId, rrEvent, "m.read");
}
}

return this.setRoomReadMarkers(roomId, rmEventId, rrEventId);
};

/**
* Get a preview of the given URL as of (roughly) the given point in time,
Expand Down