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

Url share #16

Merged
merged 11 commits into from
Dec 11, 2023
Merged

Url share #16

merged 11 commits into from
Dec 11, 2023

Conversation

tfloxolodeiro
Copy link
Contributor

Copy link
Contributor

@asanzo asanzo left a comment

Choose a reason for hiding this comment

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

Van un par de comments!! Está mergeable, charlemos sobre mis comments

res.json(await createUserChallenge(body, user))
}))

router.get('/userChallenge/:sharedId', (async (req: AuthenticatedRequest, res) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Me gusta userChallenge.

Copy link
Contributor

Choose a reason for hiding this comment

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

Al principio no me convencía sharedId, pero sospecho que es mejor que se llame así porque es algo que "ve" el usuario. Y que por dentro los objetos estos tengan el _id automático que sea distinto a ese.

request().put(authenticated(`/userChallenge/1234`))
.send(userChallenge)
.expect(200)
.then(hasBodyProperty('sharedId'))
Copy link
Contributor

Choose a reason for hiding this comment

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

¿No deberían chequear que el user sea el nuevo, y el sharedId sea el del nuevo user y no el del viejo?

Copy link
Contributor

Choose a reason for hiding this comment

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

Eso es si esta lógica está en el back ¿dónde está esta lógica? Si es en el front, los tests van en el front.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

La logica esta en el backend:
image
Esa función es lo que hace el PUT. Si no se encuentra un desafío con el id y usuario que viene en la request se crea un nuevo desafio. Entonces este test está testeando esa logica haciendo que el id no exista.

import { prop, getModelForClass, Severity, modelOptions, Ref } from '@typegoose/typegoose'
import { User } from './user'

//This is duplicated in pilas react, remember to unificar when making the monorepo :)
Copy link
Contributor

Choose a reason for hiding this comment

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

unificar 😆

@tfloxolodeiro tfloxolodeiro merged commit 190473e into main Dec 11, 2023
6 checks passed
@tfloxolodeiro tfloxolodeiro deleted the url-share branch December 11, 2023 17:56
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