Skip to content

Conversation

lowbits
Copy link
Owner

@lowbits lowbits commented Nov 21, 2024

No description provided.

Copy link

cloudflare-workers-and-pages bot commented Nov 21, 2024

Deploying nuxt-hub-landing with  Cloudflare Pages  Cloudflare Pages

Latest commit: d35e096
Status: ✅  Deploy successful!
Preview URL: https://baeab745.nuxt-hub-landing.pages.dev
Branch Preview URL: https://feat-add-double-opt-in.nuxt-hub-landing.pages.dev

View logs

Copy link

what-the-diff bot commented Nov 22, 2024

PR Summary

  • Environment Variables Template Created: Added an example file for environment variables the application might need. This will help new developers set up faster.

  • README Document Update: Enhanced and detailed instructions are now available for setting up email verification with all necessary configuration details.

  • Application Template Cleanup: Removed deprecated waitlist code from our main application template and improved the footer layout for better readability.

  • Email Verification Component: Created a new component for sending verification emails to users. This ensures email standardization across the application.

  • Email Verification Module Addition: Included a Nuxt module that handles email verification settings, enhancing security, and user authentication.

  • User Sign-Up Flow Enhancement: Integrated a user sign-up handler for collecting user details and sending verification emails instantly.

  • Email Verification Endpoint: Established an endpoint to validate user email addresses using tokens, improving user data accuracy and security.

  • Updated Configuration: Updated our main application configuration to properly interact with the new email verification module.

  • New Dependencies Added: Integrated new libraries for email rendering in Vue and secrets generation.

  • Rate Limit Update: Increased the limit on joining the waitlist to help in handling more incoming traffic.

  • Waitlist Landing Page Addition: Developed a new landing page with an input form, which validates and processes emails for the waitlist effectively.

  • Email Verification Page Addition: Created a new page for processing email verification, giving a clear message on successful or failed verification.

  • Secret Key Generation Script: Implemented a script to automatically generate the required secret key if it's absent, improving convenience and security.

  • Database Update: A new column verified_at is added to the waitlist table, noting when a user email is verified.

  • Schema Update: Updated our database schema to include the new verified_at field, reflecting database changes in code.

  • Migration Metadata Update: Modified JSON metadata to record changes in database migration.

  • Project Configuration Update: Expanded the TypeScript configuration for including custom type definitions.

  • New Type Definition: Added custom types for extending runtime configurations, ensuring code maintains type safety.

  • Utility Functions Addition: Implemented additional helper functions for secure token creation, comparison, and verification URL generation, enhancing application security and code reusability.

const config = useRuntimeConfig()


if (config.landing.verifyEmail) {
Copy link

Choose a reason for hiding this comment

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

config.landing is possibly undefined. Would consider to use optional chaining

subject: `Confirm your email on ${appName}`,
html: await render(VerifyTemplate, {
email: entry.email,
appName,
Copy link

Choose a reason for hiding this comment

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

appName could be undefined, but expected type is string

Copy link

@Eddy401 Eddy401 left a comment

Choose a reason for hiding this comment

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

I have finished my first iteration of review. Due to time constraints I have to split the effort. I will continue providing as soon as possible. So far I could join the waitlist, but I did not receive a email from resend (probably configuration issue).

One thing I noticed when starting the app is that the database migration is run every time and I get error about already existing tables. Here is the stackstrace:

` ERROR Database migrations failed D1_ERROR: table waitlist already exists at offset 13: SQLITE_ERROR

at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async SQLiteD1Session.batch (node_modules/src/d1/session.ts:82:24)
at async migrate (node_modules/src/d1/migrator.ts:47:3)
at async (server/plugins/migrations.ts:9:1)
at async Promise.all (index 0)
at async (node_modules/@nuxthub/core/dist/runtime/database/server/plugins/migrations.dev.js:15:5)
at async Promise.all (index 0)
at async (node_modules/@nuxthub/core/dist/runtime/ready.dev.js:5:3)`



function compareToken(email, token) {
return generateSecureToken(email, token) === token
Copy link

Choose a reason for hiding this comment

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

Suggested change
return generateSecureToken(email, token) === token
return generateSecureToken(email) === token


export {
compareToken,
generateSecureToken,
Copy link

Choose a reason for hiding this comment

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

Note: No need to export this function since its unused outside of this script.

Suggested change
generateSecureToken,


export default defineEventHandler(async event => {
consola.info("User trying to signup for waitlist...")
const {emails} = useResend();
Copy link

Choose a reason for hiding this comment

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

Question: Is there no import required?


consola.info("User joining waitlist...")

let entry = await useDrizzle().insert(tables.waitlist).values({
Copy link

Choose a reason for hiding this comment

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

Thought: tables is not explicitly imported. This reduces readability.

Comment on lines +16 to +17
name: 'nuxtHubLanding',
configKey: "landing",
Copy link

Choose a reason for hiding this comment

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

Thought: I think if this is supposed to be published as module a more descriptive name is required. How about evoize-waitlist?

@@ -0,0 +1,76 @@
import {addServerHandler, createResolver, defineNuxtModule, logger, extendPages} from 'nuxt/kit'
Copy link

Choose a reason for hiding this comment

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

Suggestion: Add a prettifier to format this import syntax, disallow semicolons and disallow implicit imports.

Comment on lines +6 to +9
appName?: string
verifyEmail?: boolean
email?: string,
resendApiKey?: string,
Copy link

Choose a reason for hiding this comment

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

Thought: All of those properties should be required.

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