From 73117f2f5afab4c8afb2d9cf4c63a460e76f0fa3 Mon Sep 17 00:00:00 2001 From: Evgenii Neumerzhitckii Date: Wed, 31 Jul 2024 15:26:07 +1000 Subject: [PATCH 1/3] Add ability to unsubscribe from emails (issue 107) --- .vscode/settings.json | 12 +- README.md | 11 +- database_migration.sql | 5 +- src/DatabaseDefinitions.ts | 3 + src/lib/emails/welcome_email_html.svelte | 16 +++ src/lib/emails/welcome_email_text.svelte | 4 + src/lib/mailer.test.ts | 121 ++++++++++++++++++ src/lib/mailer.ts | 20 ++- .../account/(menu)/settings/+page.svelte | 13 ++ .../change_email_subscription/+page.svelte | 26 ++++ .../(admin)/account/api/+page.server.ts | 28 ++++ .../(admin)/account/api/page.server.test.ts | 120 +++++++++++++++++ .../migrations/20240730010101_initial.sql | 72 +++++++++++ ...731051052_add_unsubscribed_to_profiles.sql | 2 + tsconfig.json | 5 +- vite.config.ts | 1 + 16 files changed, 449 insertions(+), 10 deletions(-) create mode 100644 src/lib/mailer.test.ts create mode 100644 src/routes/(admin)/account/(menu)/settings/change_email_subscription/+page.svelte create mode 100644 src/routes/(admin)/account/api/page.server.test.ts create mode 100644 supabase/migrations/20240730010101_initial.sql create mode 100644 supabase/migrations/20240731051052_add_unsubscribed_to_profiles.sql diff --git a/.vscode/settings.json b/.vscode/settings.json index 0a39a7a2..78afa4dd 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,5 +1,13 @@ { "editor.formatOnSave": true, - "editor.defaultFormatter": "svelte.svelte-vscode", - "eslint.validate": ["javascript", "javascriptreact", "svelte"] + "[svelte]": { + "editor.defaultFormatter": "svelte.svelte-vscode" + }, + "[typescript]": { + "editor.defaultFormatter": "esbenp.prettier-vscode" + }, + "[javascript]": { + "editor.defaultFormatter": "esbenp.prettier-vscode" + }, + "eslint.validate": ["javascript", "javascriptreact", "typescript", "svelte"] } diff --git a/README.md b/README.md index 4a5561b5..2e08599e 100644 --- a/README.md +++ b/README.md @@ -193,9 +193,14 @@ Finally: if you find build, formatting or linting rules too tedious, you can dis - Create a Supabase account - Create a new Supabase project in the console - Wait for the database to launch -- Create your user management tables in the database - - Go to the [SQL Editor](https://supabase.com/dashboard/project/_/sql) page in the Dashboard. - - Paste the SQL from `database_migration.sql` in this repo to create your user/profiles table and click run. +- Set up your database schema: + - For new Supabase projects: + - Go to the [SQL Editor](https://supabase.com/dashboard/project/_/sql) page in the Dashboard. + - Run the SQL from `database_migration.sql` to create the initial schema. + - For existing projects: + - Apply migrations from the `supabase/migrations` directory: + 1. Go to the Supabase dashboard's SQL Editor. + 2. Identify the last migration you applied, then run the SQL content of each subsequent file in chronological order. - Enable user signups in the [Supabase console](https://app.supabase.com/project/_/settings/auth): sometimes new signups are disabled by default in Supabase projects - Go to the [API Settings](https://supabase.com/dashboard/project/_/settings/api) page in the Dashboard. Find your Project-URL (PUBLIC_SUPABASE_URL), anon (PUBLIC_SUPABASE_ANON_KEY) and service_role (PRIVATE_SUPABASE_SERVICE_ROLE). - For local development: create a `.env.local` file: diff --git a/database_migration.sql b/database_migration.sql index d78d6f15..d1b212e8 100644 --- a/database_migration.sql +++ b/database_migration.sql @@ -5,7 +5,8 @@ create table profiles ( full_name text, company_name text, avatar_url text, - website text + website text, + unsubscribed boolean NOT NULL DEFAULT false ); -- Set up Row Level Security (RLS) -- See https://supabase.com/docs/guides/auth/row-level-security for more details. @@ -69,4 +70,4 @@ create policy "Avatar images are publicly accessible." on storage.objects for select using (bucket_id = 'avatars'); create policy "Anyone can upload an avatar." on storage.objects - for insert with check (bucket_id = 'avatars'); \ No newline at end of file + for insert with check (bucket_id = 'avatars'); diff --git a/src/DatabaseDefinitions.ts b/src/DatabaseDefinitions.ts index 91bbc6d1..3d9122a1 100644 --- a/src/DatabaseDefinitions.ts +++ b/src/DatabaseDefinitions.ts @@ -50,6 +50,7 @@ export interface Database { updated_at: string | null company_name: string | null website: string | null + unsubscribed: boolean } Insert: { avatar_url?: string | null @@ -58,6 +59,7 @@ export interface Database { updated_at?: Date | null company_name?: string | null website?: string | null + unsubscribed: boolean } Update: { avatar_url?: string | null @@ -66,6 +68,7 @@ export interface Database { updated_at?: string | null company_name?: string | null website?: string | null + unsubscribed: boolean } Relationships: [ { diff --git a/src/lib/emails/welcome_email_html.svelte b/src/lib/emails/welcome_email_html.svelte index cdac05cc..72622ec2 100644 --- a/src/lib/emails/welcome_email_html.svelte +++ b/src/lib/emails/welcome_email_html.svelte @@ -1,4 +1,6 @@ + + + Change Email Subscription + + +

+ {unsubscribed ? "Re-subscribe to Emails" : "Unsubscribe from Emails"} +

+ + diff --git a/src/routes/(admin)/account/api/+page.server.ts b/src/routes/(admin)/account/api/+page.server.ts index 9be2ba3a..5b1e78fd 100644 --- a/src/routes/(admin)/account/api/+page.server.ts +++ b/src/routes/(admin)/account/api/+page.server.ts @@ -2,6 +2,34 @@ import { fail, redirect } from "@sveltejs/kit" import { sendAdminEmail, sendUserEmail } from "$lib/mailer" export const actions = { + toggleEmailSubscription: async ({ locals: { supabase, safeGetSession } }) => { + const { session } = await safeGetSession() + + if (!session) { + redirect(303, "/login") + } + + const { data: currentProfile } = await supabase + .from("profiles") + .select("unsubscribed") + .eq("id", session.user.id) + .single() + + const newUnsubscribedStatus = !currentProfile?.unsubscribed + + const { error } = await supabase + .from("profiles") + .update({ unsubscribed: newUnsubscribedStatus }) + .eq("id", session.user.id) + + if (error) { + return fail(500, { message: "Failed to update subscription status" }) + } + + return { + unsubscribed: newUnsubscribedStatus, + } + }, updateEmail: async ({ request, locals: { supabase, safeGetSession } }) => { const { session } = await safeGetSession() if (!session) { diff --git a/src/routes/(admin)/account/api/page.server.test.ts b/src/routes/(admin)/account/api/page.server.test.ts new file mode 100644 index 00000000..28724e66 --- /dev/null +++ b/src/routes/(admin)/account/api/page.server.test.ts @@ -0,0 +1,120 @@ +import { describe, it, expect, vi, beforeEach } from "vitest" +import { actions } from "./+page.server" +import { fail, redirect } from "@sveltejs/kit" + +vi.mock("@sveltejs/kit", async () => { + const actual = await vi.importActual("@sveltejs/kit") + return { + ...actual, + fail: vi.fn(), + redirect: vi.fn().mockImplementation(() => { + throw new Error("Redirect error") + }), + } +}) + +describe("toggleEmailSubscription", () => { + const mockSupabase = { + from: vi.fn().mockReturnThis(), + select: vi.fn().mockReturnThis(), + eq: vi.fn().mockReturnThis(), + single: vi.fn().mockResolvedValue({ data: null }), + update: vi.fn().mockReturnThis(), + } + + const mockSafeGetSession = vi.fn() + + beforeEach(() => { + vi.clearAllMocks() + }) + + it("should redirect if no session", async () => { + mockSafeGetSession.mockResolvedValue({ session: null }) + + await expect( + actions.toggleEmailSubscription({ + locals: { supabase: mockSupabase, safeGetSession: mockSafeGetSession }, + }), + ).rejects.toThrow("Redirect") + + expect(redirect).toHaveBeenCalledWith(303, "/login") + }) + + it("should toggle subscription status from false to true", async () => { + const mockSession = { user: { id: "user123" } } + mockSafeGetSession.mockResolvedValue({ session: mockSession }) + + // Mock the first query to get the current status + mockSupabase.single.mockResolvedValueOnce({ data: { unsubscribed: false } }) + + // Mock the update query + const mockUpdateChain = { + eq: vi.fn().mockResolvedValue({ error: null }), + } + + mockSupabase.update.mockReturnValue(mockUpdateChain) + + const result = await actions.toggleEmailSubscription({ + locals: { supabase: mockSupabase, safeGetSession: mockSafeGetSession }, + }) + + expect(mockSupabase.from).toHaveBeenCalledWith("profiles") + expect(mockSupabase.select).toHaveBeenCalledWith("unsubscribed") + expect(mockSupabase.eq).toHaveBeenCalledWith("id", "user123") + expect(mockSupabase.single).toHaveBeenCalled() + expect(mockSupabase.update).toHaveBeenCalledWith({ unsubscribed: true }) + expect(mockUpdateChain.eq).toHaveBeenCalledWith("id", "user123") + expect(result).toEqual({ unsubscribed: true }) + }) + + it("should toggle subscription status from true to false", async () => { + const mockSession = { user: { id: "user123" } } + mockSafeGetSession.mockResolvedValue({ session: mockSession }) + + // Mock the first query to get the current status + mockSupabase.single.mockResolvedValueOnce({ data: { unsubscribed: true } }) + + // Mock the update query + const mockUpdateChain = { + eq: vi.fn().mockResolvedValue({ error: null }), + } + + mockSupabase.update.mockReturnValue(mockUpdateChain) + + const result = await actions.toggleEmailSubscription({ + locals: { supabase: mockSupabase, safeGetSession: mockSafeGetSession }, + }) + + expect(mockSupabase.from).toHaveBeenCalledWith("profiles") + expect(mockSupabase.select).toHaveBeenCalledWith("unsubscribed") + expect(mockSupabase.eq).toHaveBeenCalledWith("id", "user123") + expect(mockSupabase.single).toHaveBeenCalled() + expect(mockSupabase.update).toHaveBeenCalledWith({ unsubscribed: false }) + expect(mockUpdateChain.eq).toHaveBeenCalledWith("id", "user123") + expect(result).toEqual({ unsubscribed: false }) + }) + + it("should return fail response if update operation fails", async () => { + const mockSession = { user: { id: "user123" } } + mockSafeGetSession.mockResolvedValue({ session: mockSession }) + + // Mock the first query to get the current status + mockSupabase.single.mockResolvedValueOnce({ data: { unsubscribed: false } }) + + // Mock the update query to return an error + const mockUpdateChain = { + eq: vi.fn().mockResolvedValue({ error: new Error("Update failed") }), + } + + mockSupabase.update.mockReturnValue(mockUpdateChain) + + await actions.toggleEmailSubscription({ + locals: { supabase: mockSupabase, safeGetSession: mockSafeGetSession }, + }) + + // Check if fail was called with the correct arguments + expect(fail).toHaveBeenCalledWith(500, { + message: "Failed to update subscription status", + }) + }) +}) diff --git a/supabase/migrations/20240730010101_initial.sql b/supabase/migrations/20240730010101_initial.sql new file mode 100644 index 00000000..076ae11d --- /dev/null +++ b/supabase/migrations/20240730010101_initial.sql @@ -0,0 +1,72 @@ +-- Create a table for public profiles +create table profiles ( + id uuid references auth.users on delete cascade not null primary key, + updated_at timestamp with time zone, + full_name text, + company_name text, + avatar_url text, + website text +); +-- Set up Row Level Security (RLS) +-- See https://supabase.com/docs/guides/auth/row-level-security for more details. +alter table profiles + enable row level security; + +create policy "Profiles are viewable by self." on profiles + for select using (auth.uid() = id); + +create policy "Users can insert their own profile." on profiles + for insert with check (auth.uid() = id); + +create policy "Users can update own profile." on profiles + for update using (auth.uid() = id); + +-- Create Stripe Customer Table +-- One stripe customer per user (PK enforced) +-- Limit RLS policies -- mostly only server side access +create table stripe_customers ( + user_id uuid references auth.users on delete cascade not null primary key, + updated_at timestamp with time zone, + stripe_customer_id text unique +); +alter table stripe_customers enable row level security; + +-- Create a table for "Contact Us" form submissions +-- Limit RLS policies -- only server side access +create table contact_requests ( + id uuid primary key default gen_random_uuid(), + updated_at timestamp with time zone, + first_name text, + last_name text, + email text, + phone text, + company_name text, + message_body text +); +alter table contact_requests enable row level security; + +-- This trigger automatically creates a profile entry when a new user signs up via Supabase Auth. +-- See https://supabase.com/docs/guides/auth/managing-user-data#using-triggers for more details. +create function public.handle_new_user() +returns trigger as $$ +begin + insert into public.profiles (id, full_name, avatar_url) + values (new.id, new.raw_user_meta_data->>'full_name', new.raw_user_meta_data->>'avatar_url'); + return new; +end; +$$ language plpgsql security definer; +create trigger on_auth_user_created + after insert on auth.users + for each row execute procedure public.handle_new_user(); + +-- Set up Storage! +insert into storage.buckets (id, name) + values ('avatars', 'avatars'); + +-- Set up access controls for storage. +-- See https://supabase.com/docs/guides/storage#policy-examples for more details. +create policy "Avatar images are publicly accessible." on storage.objects + for select using (bucket_id = 'avatars'); + +create policy "Anyone can upload an avatar." on storage.objects + for insert with check (bucket_id = 'avatars'); diff --git a/supabase/migrations/20240731051052_add_unsubscribed_to_profiles.sql b/supabase/migrations/20240731051052_add_unsubscribed_to_profiles.sql new file mode 100644 index 00000000..1042d6d1 --- /dev/null +++ b/supabase/migrations/20240731051052_add_unsubscribed_to_profiles.sql @@ -0,0 +1,2 @@ +ALTER TABLE profiles +ADD COLUMN IF NOT EXISTS unsubscribed boolean NOT NULL DEFAULT false; diff --git a/tsconfig.json b/tsconfig.json index f1da068b..1c4f9069 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -8,8 +8,9 @@ "resolveJsonModule": true, "skipLibCheck": true, "sourceMap": true, - "strict": true - } + "strict": true, + "types": ["vitest/globals"] // allows to skip import of test functions like `describe`, `it`, `expect`, etc. + }, // Path aliases are handled by https://kit.svelte.dev/docs/configuration#alias and https://kit.svelte.dev/docs/configuration#files // // If you want to overwrite includes/excludes, make sure to copy over the relevant includes/excludes diff --git a/vite.config.ts b/vite.config.ts index d1d54a1d..40e82872 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -5,5 +5,6 @@ export default defineConfig({ plugins: [sveltekit()], test: { include: ["src/**/*.{test,spec}.{js,ts}"], + globals: true /// allows to skip import of test functions like `describe`, `it`, `expect`, etc. }, }) From 02c9f37bf78b31cd9d3968bc51aeadb03a9dc755 Mon Sep 17 00:00:00 2001 From: Evgenii Neumerzhitckii Date: Thu, 1 Aug 2024 22:05:45 +1000 Subject: [PATCH 2/3] Fix linting errors --- .eslintrc.cjs | 7 +++++++ src/lib/emails/welcome_email_html.svelte | 1 - src/routes/(admin)/account/api/+page.server.ts | 1 + src/routes/(admin)/account/api/page.server.test.ts | 13 ++++++++----- tsconfig.json | 2 +- vite.config.ts | 2 +- 6 files changed, 18 insertions(+), 8 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 861b0bdd..658e1df8 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -26,6 +26,13 @@ module.exports = { }, }, }, + { + // Apply to all test files. Proper type checking in tests with mocks can be tedious and counterproductive. + files: ['**/*.test.ts', '**/*.spec.ts'], + rules: { + '@typescript-eslint/no-explicit-any': 'off' + } + } ], env: { browser: true, diff --git a/src/lib/emails/welcome_email_html.svelte b/src/lib/emails/welcome_email_html.svelte index 72622ec2..2dbfb0c2 100644 --- a/src/lib/emails/welcome_email_html.svelte +++ b/src/lib/emails/welcome_email_html.svelte @@ -193,7 +193,6 @@ style="font-family: Helvetica, sans-serif; font-size: 16px; vertical-align: top; border-radius: 4px; text-align: center; background-color: #0867ec;" valign="top" align="center" - bgcolor="#0867ec" > { await expect( actions.toggleEmailSubscription({ - locals: { supabase: mockSupabase, safeGetSession: mockSafeGetSession }, - }), + locals: { + supabase: mockSupabase, + safeGetSession: mockSafeGetSession, + }, + } as any), ).rejects.toThrow("Redirect") expect(redirect).toHaveBeenCalledWith(303, "/login") @@ -56,7 +59,7 @@ describe("toggleEmailSubscription", () => { const result = await actions.toggleEmailSubscription({ locals: { supabase: mockSupabase, safeGetSession: mockSafeGetSession }, - }) + } as any) expect(mockSupabase.from).toHaveBeenCalledWith("profiles") expect(mockSupabase.select).toHaveBeenCalledWith("unsubscribed") @@ -83,7 +86,7 @@ describe("toggleEmailSubscription", () => { const result = await actions.toggleEmailSubscription({ locals: { supabase: mockSupabase, safeGetSession: mockSafeGetSession }, - }) + } as any) expect(mockSupabase.from).toHaveBeenCalledWith("profiles") expect(mockSupabase.select).toHaveBeenCalledWith("unsubscribed") @@ -110,7 +113,7 @@ describe("toggleEmailSubscription", () => { await actions.toggleEmailSubscription({ locals: { supabase: mockSupabase, safeGetSession: mockSafeGetSession }, - }) + } as any) // Check if fail was called with the correct arguments expect(fail).toHaveBeenCalledWith(500, { diff --git a/tsconfig.json b/tsconfig.json index 1c4f9069..59aa71ce 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -10,7 +10,7 @@ "sourceMap": true, "strict": true, "types": ["vitest/globals"] // allows to skip import of test functions like `describe`, `it`, `expect`, etc. - }, + } // Path aliases are handled by https://kit.svelte.dev/docs/configuration#alias and https://kit.svelte.dev/docs/configuration#files // // If you want to overwrite includes/excludes, make sure to copy over the relevant includes/excludes diff --git a/vite.config.ts b/vite.config.ts index 40e82872..0eebd751 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -5,6 +5,6 @@ export default defineConfig({ plugins: [sveltekit()], test: { include: ["src/**/*.{test,spec}.{js,ts}"], - globals: true /// allows to skip import of test functions like `describe`, `it`, `expect`, etc. + globals: true, /// allows to skip import of test functions like `describe`, `it`, `expect`, etc. }, }) From e4ecdd4b739214eb00883ed0b0deef64b7b568e4 Mon Sep 17 00:00:00 2001 From: Evgenii Neumerzhitckii Date: Thu, 1 Aug 2024 22:41:36 +1000 Subject: [PATCH 3/3] Fix lint errors --- .eslintrc.cjs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 658e1df8..1a5c8ae3 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -28,11 +28,11 @@ module.exports = { }, { // Apply to all test files. Proper type checking in tests with mocks can be tedious and counterproductive. - files: ['**/*.test.ts', '**/*.spec.ts'], + files: ["**/*.test.ts", "**/*.spec.ts"], rules: { - '@typescript-eslint/no-explicit-any': 'off' - } - } + "@typescript-eslint/no-explicit-any": "off", + }, + }, ], env: { browser: true,