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

Auth page #53

Open
wants to merge 91 commits into
base: dev
Choose a base branch
from
Open

Auth page #53

wants to merge 91 commits into from

Conversation

michaelbrusegard
Copy link
Member

No description provided.

@ZeroWave022 ZeroWave022 linked an issue Sep 26, 2024 that may be closed by this pull request
@michaelbrusegard michaelbrusegard marked this pull request as ready for review January 16, 2025 10:48
@michaelbrusegard
Copy link
Member Author

lighthouse workflow will fail since we do not have a way of it connecting to a database when it runs the build yet. But that should be a separate issue

@michaelbrusegard
Copy link
Member Author

Also profile and account settings page will be its own issue. And forgot password requests will be its own issue

@michaelbrusegard
Copy link
Member Author

Now that this is done the backend is fully working and up and running. And authentication with and without Feide should work. If you want to test you can login with the Frank Sinatra account which is currently being seeded to the database.
Username: fransina
Password: Password1!

@michaelbrusegard michaelbrusegard requested review from HaakonWiland and removed request for ZeroWave022 January 16, 2025 16:09
Copy link
Contributor

@HaakonWiland HaakonWiland left a comment

Choose a reason for hiding this comment

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

Looks good, lets merge yolo

Copy link
Member

@seandreassen seandreassen left a comment

Choose a reason for hiding this comment

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

Didn't really look through thoroughly, and couldn't run the website so I don't know if the CSS looks good. I'm sure most of it works but it should probably be reviewed by someone else as well.


import { EyeIcon, EyeOffIcon } from 'lucide-react';
import { useTranslations } from 'next-intl';
import * as React from 'react';
Copy link
Member

Choose a reason for hiding this comment

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

Should import only useState instead of react

@@ -0,0 +1,11 @@
const skillIdentifiers = [
'printing',
'souldering',
Copy link
Member

Choose a reason for hiding this comment

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

soldering

},
'/auth/success': {
en: '/auth/success',
no: '/autentisering/success',
Copy link
Member

Choose a reason for hiding this comment

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

skal den her være på engelsk?

import { type InsertUser, users } from '@/server/db/tables';

async function getUserFromUsername(username: string) {
return await db.query.users.findFirst({
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be findUnique? I don't really know much about this so maybe this is correct.

Copy link
Member

Choose a reason for hiding this comment

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

findUnique is interchangable with findFirst when assuming all usernames are truly unique. Perhaps we should throw an error if there are multiple people with the same username? Or, we can just make sure to have proper username creation routines which will not result in conflicts.

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.

feat: auth page
4 participants