feat: Implement new protocol definition #32
Conversation
Pull Request Test Coverage Report for Build 12831901632Details
💛 - Coveralls |
| if (!lastFriendshipAction) { | ||
| return { | ||
| response: { | ||
| $case: 'internalServerError', |
There was a problem hiding this comment.
Is it ok to handle this case as an InternalServerError?
There was a problem hiding this comment.
It came from the previous definition. We can rethink the error responses.
| return async function ( | ||
| request: GetFriendshipRequestsPayload, | ||
| context: RpcServerContext | ||
| ): Promise<PaginatedFriendshipRequestsResponse> { |
There was a problem hiding this comment.
NIT: I'm thinking that maybe we could wrap-up pagination functionality in something like:
Paginated<FriendshipRequestResponse>
And so on for all the things we do need to paginate, what do you think?
There was a problem hiding this comment.
In terms of the functionality, I agree, but the types come from the protocol, which doesn't allowtheo create generics
| @@ -2,7 +2,7 @@ import { Action, FriendshipStatus, RpcServerContext, RPCServiceContext } from '. | |||
| import { | |||
| UpsertFriendshipPayload, | |||
| UpsertFriendshipResponse | |||
| } from '@dcl/protocol/out-ts/decentraland/social_service_v2/social_service.gen' | |||
| } from '@dcl/protocol/out-ts/decentraland/social_service/v3/social_service_v3.gen' | |||
There was a problem hiding this comment.
Doubt, is it okay to also have the v3 at the file name level and not only at route level?
There was a problem hiding this comment.
Ideally, there should be no issue, but since the previous definition filename is "social_service," I attempted to use /v3/social_service.proto. However, it resulted in a compilation error for C#.
src/adapters/db.ts
Outdated
| async getFriends(userAddress, { onlyActive, pagination } = { onlyActive: true }) { | ||
| const { limit, offset } = getPaginationOrDefaults(pagination) |
There was a problem hiding this comment.
What about placing the default directly on this getFriends function's params instead at getPaginationOrDefaults as we do with onlyActive:true
There was a problem hiding this comment.
Also, could be possible that limit is set but not the offset, or vice versa?
No description provided.