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 email to users #64

Merged
merged 3 commits into from
Jan 31, 2025
Merged

Conversation

EnzoDOROSARIO
Copy link
Contributor

Description

Prepare #49

  • Add email field for users in database
  • Update existing endpoints
  • Update tests

Checklist

migrations/006.do.user_email.sql Outdated Show resolved Hide resolved
test/helper.ts Outdated Show resolved Hide resolved
@jean-michelet
Copy link
Contributor

You can proceed to update the auth too if you feel motivated.

@EnzoDOROSARIO EnzoDOROSARIO force-pushed the feat/user-emails branch 2 times, most recently from b1bfded to 45a9e6b Compare December 14, 2024 08:41
test/routes/api/auth/auth.test.ts Outdated Show resolved Hide resolved
src/routes/api/auth/index.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@jean-michelet jean-michelet left a comment

Choose a reason for hiding this comment

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

I recommend fixing my feedback before merging.

@@ -4,7 +4,7 @@ import { build } from '../../../helper.js'
import { FastifyInstance } from 'fastify'
import { scryptHash } from '../../../../src/plugins/custom/scrypt.js'

async function createUser (app: FastifyInstance, userData: Partial<{ username: string; password: string }>) {
async function createUser (app: FastifyInstance, userData: Partial<{ username: string; email: string; password: string }>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same, email can be created dynamically.

const res = await this.inject({
method: 'POST',
url: '/api/auth/login',
payload: {
username,
email,
Copy link
Contributor

Choose a reason for hiding this comment

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

You can automate the creation of the email from the username by doing '${username}@example.com'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did that first, but if the authentication should be based on email, wouldn't it be better if the tests reflected this?

@jean-michelet
Copy link
Contributor

If you pull the last changes, sonarcloud should not fail anymore.

@jean-michelet jean-michelet merged commit ac53126 into fastify:main Jan 31, 2025
5 checks passed
@jean-michelet
Copy link
Contributor

Thanks @EnzoDOROSARIO!

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.

2 participants