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

Update and retrieve display name #193

Merged
merged 15 commits into from
Mar 20, 2024

Conversation

aaditkamat
Copy link
Collaborator

Resolves #183

To avoid confusion between functions involving multiple users and those for the current user, I've renamed the getConnectedUsers.ts file to getMultipleConnectedUsers.ts and created a seperate file for a function to get the display name for the author of the message.

@aaditkamat aaditkamat requested a review from h1divp March 2, 2024 21:31
server/src/index.ts Outdated Show resolved Hide resolved
server/src/index.ts Outdated Show resolved Hide resolved
server/src/actions/getSingleConnectedUser.ts Outdated Show resolved Hide resolved

const success = await getConnectedUserDisplayName(userId);
if (success) {
res.json(success);
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you rename this to something other than success. the variable name is not descriptive enough for people to know that it is a displayName: "string". You could rename it displayName.

@@ -1,6 +1,19 @@
import { distanceBetween, geohashForLocation, geohashQueryBounds } from 'geofire-common'
import { connectedUsersCollection } from '../utilities/firebaseInit'

export const getConnectedUserDisplayName = async (socketID: string) => {
try {
const snap = await connectedUsersCollection.doc(socketID).get();
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you change this to user instead of snap?

@h1divp
Copy link
Collaborator

h1divp commented Mar 20, 2024

The API endpoint used for returning the displayName is /users?userid. I think this might be a little misleading since it only returns a ConnectedUser's displayName, when I feel like one would expect the whole user document should be returned.

So I'm going to update the GET route to just return the document in json. This will also prevent us from trying to build more routes to just return one parameter in a user doc.

h1divp added 2 commits March 19, 2024 21:06
…rn all ConnectedUser document information. Updated /users?userId GET endpoint to match this change.
@h1divp
Copy link
Collaborator

h1divp commented Mar 20, 2024

Everything has been tested and works 👍

@h1divp h1divp merged commit 93b848b into ufosc:main Mar 20, 2024
2 checks passed
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.

Create socket API endpoints for author displayName update and retrival
3 participants