-
Notifications
You must be signed in to change notification settings - Fork 9
feat: cool dev ui #23
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
base: main
Are you sure you want to change the base?
Conversation
@NeonGamerBot-QK is attempting to deploy a commit to the Hack Club Team on Vercel. A member of the Team first needs to authorize it. |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Also, you should be using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds development tools to improve the developer experience, including debug UI features and magic link testing capabilities. The changes focus on providing developers with convenient utilities for testing authentication flows and user signup processes.
Key changes:
- Adds development-only UI components for testing user signup with random data and accessing magic links
- Implements a "letter box" endpoint and page for viewing generated magic links during development
- Includes minor code formatting improvements and redirects for better UX
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
frontend/src/routes/login/+page.svelte | Adds dev-only buttons for random user generation and letter box access |
frontend/src/routes/letter_box/+page.svelte | Creates new page to display magic links for development testing |
frontend/src/lib/misc.ts | Implements generateUser function with random test data generation |
frontend/src/lib/components/CreateEvent.svelte | Adds redirect to events page after event creation |
frontend/src/lib/components/Admin/Button.svelte | Creates reusable admin button component with orange styling |
frontend/.env.development | Adds development API URL configuration |
backend/poetry.dev.sh | Adds development server startup script |
backend/podium/routers/auth.py | Implements letter_box endpoint and magic link collection for development |
backend/docker-compose.yaml | Enables port mapping and formatting cleanup |
Comments suppressed due to low confidence (1)
backend/podium/routers/auth.py:26
- [nitpick] Variable name 'magic_urls' should follow Python naming conventions. Consider 'magic_links' to match the context where it stores magic links.
magic_urls= []
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Frankly, I don't think a magic link inbox is needed. It's just one more thing that could lead to a security vulnerability in the future and by default, when debugging, the localhost login link is already printed to the console.
magic_link += f"&redirect={redirect}" | ||
|
||
if environment == "development": | ||
magic_urls.append(MagicLink(email=email, magic_link=magic_link)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MagicLink isn't defined
ports: | ||
- "8000:8000" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This messes with Coolify
ports: | |
- "8000:8000" | |
# ports: | |
# - "8000:8000" |
|
||
def get_mail(): | ||
""" kinda like /letter_box but in python or smt.""" | ||
return magic_urls | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def get_mail(): | |
""" kinda like /letter_box but in python or smt.""" | |
return magic_urls |
""" | ||
This is a temporary endpoint to get the magic links that have been sent. | ||
""" | ||
return get_mail() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it's possible, but an even better approach would be to wrap the entire function in a conditional so it's not even defined in prod
return get_mail() | |
return magic_urls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convert to a utility class in app.css
, and also, use border-warning bg-warning/10
instead of border-orange-500 bg-orange-500/10
so it responds to theme changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be using the generated SDK, not fetch
.
{#if showSignupFields} | ||
<button | ||
onclick={() => { | ||
// todo call info func |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// todo call info func |
This is implemented on the line below, correct?
No description provided.