Skip to content

Conversation

kabir276
Copy link
Contributor

@kabir276 kabir276 commented Jun 3, 2024

Fixes Issue #61

Description
I've added an option to create users on the dashboard's "Users" section. I've created a pull request for your review. Please let me know if this is what you envisioned. If not, I'm happy to make further adjustments.

Changes Made
Updated users>page.client.tsx , lib>stack-app.ts , Interface>clientInterface.ts to include the button to create users.
Additional Notes
No new dependencies were added.

@vercel
Copy link

vercel bot commented Jun 3, 2024

@kabir276 is attempting to deploy a commit to the Stackframe Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@N2D4 N2D4 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

type CreateDialogProps = {
open: boolean;
onOpenChange: (open: boolean) => void;
opennotification: Function
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of passing up a function like this, can you put the notifyPassword dialog into ?

Copy link
Contributor Author

@kabir276 kabir276 Jun 4, 2024

Choose a reason for hiding this comment

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

How about this
setShowNotifyPasswordDialog: React.Dispatch<React.SetStateAction>;
instead of
onOpenChange: (open: boolean) => void;
Will this work?

const allUsers = stackAdminApp.useServerUsers();

const [addUserOpen, setAddUserOpen] = React.useState(false);
const [notifyPassword, setNotifyPassword] = React.useState(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const [notifyPassword, setNotifyPassword] = React.useState(false);
const [showNotifyPasswordDialog, setShowNotifyPasswordDialog] = React.useState(false);

});
}

async createUser(
Copy link
Contributor

Choose a reason for hiding this comment

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

There is some code duplication between this and signUpWithCredential now. Can we make the latter call this function?

return errorCode;
}

async createUser(options: {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is some code duplication between this and signUpWithCredential now. Can we make the latter call this function?

Comment on lines 790 to 792
if (!errorCode) {
console.log("done")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!errorCode) {
console.log("done")
}


const [addUserOpen, setAddUserOpen] = React.useState(false);
const [notifyPassword, setNotifyPassword] = React.useState(false);
console.log(notifyPassword)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.log(notifyPassword)


signInWithOAuth(provider: string): Promise<void>,
signInWithCredential(options: { email: string, password: string }): Promise<KnownErrors["EmailPasswordMismatch"] | undefined>,
createUser(options: { email: string }): Promise<KnownErrors["UserEmailAlreadyExists"] | KnownErrors["PasswordRequirementsNotMet"] | undefined>,
Copy link
Contributor

Choose a reason for hiding this comment

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

This function should return the generated password.

Do you have any thoughts on whether createUserWithCredential is a better name for this? Since theoretically a user could be a different type as well (OAuth, magic link, etc). Maybe @fomalhautb you have an opinion on this?

@kabir276 kabir276 requested a review from N2D4 June 7, 2024 16:26
@kabir276 kabir276 closed this Jun 11, 2024
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