Compare commits
2 Commits
main
..
9b2ed1c0b6
| Author | SHA1 | Date | |
|---|---|---|---|
| 9b2ed1c0b6 | |||
| b2d2d46db7 |
@@ -3,9 +3,7 @@ name: Commit
|
||||
on:
|
||||
push:
|
||||
# only trigger on branches, not on tags
|
||||
branches:
|
||||
- 'main'
|
||||
- 'dev'
|
||||
branches: '**'
|
||||
|
||||
jobs:
|
||||
# Job 1: Lint and Test (Type Check)
|
||||
@@ -36,6 +34,3 @@ jobs:
|
||||
- name: Type Check (Svelte Check)
|
||||
# Based on your package.json "check" script
|
||||
run: pnpm check
|
||||
|
||||
- name: Run Tests (Vitest)
|
||||
run: pnpm run test
|
||||
|
||||
@@ -17,10 +17,8 @@ jobs:
|
||||
http = true
|
||||
insecure = true
|
||||
|
||||
- name: login gitea registry
|
||||
- name: login
|
||||
run: docker login -u schreifuchs -p ${{ secrets.REGISTRY_TOKEN }} git.schreifuchs.ch
|
||||
- name: login dockerhub
|
||||
run: docker login -u aktitiel -p ${{ secrets.DOCKER_HUB_TOKEN}}
|
||||
- name: Build and push Docker image
|
||||
uses: https://github.com/docker/build-push-action@v5
|
||||
with:
|
||||
|
||||
@@ -18,10 +18,8 @@ jobs:
|
||||
http = true
|
||||
insecure = true
|
||||
|
||||
- name: login gitea registry
|
||||
- name: login
|
||||
run: docker login -u schreifuchs -p ${{ secrets.REGISTRY_TOKEN }} git.schreifuchs.ch
|
||||
- name: login dockerhub
|
||||
run: docker login -u aktitiel -p ${{ secrets.DOCKER_HUB_TOKEN}}
|
||||
- name: Build and push Docker image
|
||||
uses: https://github.com/docker/build-push-action@v5
|
||||
with:
|
||||
|
||||
@@ -1,3 +1,2 @@
|
||||
pnpm run format
|
||||
pnpm run lint
|
||||
pnpm run test
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
# Project Review & Refactoring TODOs
|
||||
|
||||
This document contains the prioritized list of refactoring tasks, architectural improvements, and testing strategies for the Aktiteil project.
|
||||
|
||||
## 🚨 Must do (Security & Critical Best Practices)
|
||||
|
||||
- [ ] **Fix Critical XSS Vulnerability (`{@html}` without sanitization)**
|
||||
- **Where:** `src/routes/akti/[aktiId]/+page.svelte`
|
||||
- **Why:** Rendering user input via `{@html data.akti.body}` without sanitization allows malicious scripts to be injected.
|
||||
- **Fix:** Use the already installed `sanitize-html` library on the server to sanitize `changeRequest.body` before updating/inserting into the database.
|
||||
|
||||
- [ ] **Move Server-Only Code to `$lib/server`**
|
||||
- **Where:** `src/lib/auth.ts`
|
||||
- **Why:** It imports from `./server/db`. Keeping server-side dependencies in the general `$lib` folder risks accidental imports by client components, breaking the Vite build and potentially leaking server logic.
|
||||
- **Fix:** Move and rename it to `src/lib/server/session.ts` (or `authUtils.ts`) and update imports in `.server.ts` files.
|
||||
|
||||
- [ ] **Fix Action Validation Error Handling**
|
||||
- **Where:** `src/routes/akti/[aktiId]/+page.server.ts` and `src/routes/akti/[aktiId]/comment/+page.server.ts`
|
||||
- **Why:** Currently returning `error(400)` on validation failure, which wipes form data and shows a generic error page.
|
||||
- **Fix:** Use SvelteKit's `fail(400, { message: 'Invalid data' })` to keep the user on the page and preserve their input.
|
||||
|
||||
- [ ] **Fix Hacky Fallback in Auth Query**
|
||||
- **Where:** `src/lib/auth.ts` -> `getSession()`
|
||||
- **Why:** Querying the DB with a fallback UUID (`eaf930...`) when email is missing is an anti-pattern.
|
||||
- **Fix:** Implement an early return (`if (!session?.user?.email) return null;`) before hitting the database.
|
||||
|
||||
## 🛠️ Should do (Performance & Architecture)
|
||||
|
||||
- [ ] **Parallelize Database Queries**
|
||||
- **Where:** `src/routes/akti/[aktiId]/+page.server.ts` (load function)
|
||||
- **Why:** Queries are running sequentially.
|
||||
- **Fix:** Use `Promise.all([ db.query.aktis.findFirst(...), db.query.ratings.findMany(...) ])` to run concurrently.
|
||||
|
||||
- [ ] **Implement Pagination / Limit for the Dashboard**
|
||||
- **Where:** `src/routes/+page.server.ts`
|
||||
- **Why:** Querying all records joined with ratings will scale poorly.
|
||||
- **Fix:** Add a `.limit()` clause and consider basic pagination or infinite scrolling.
|
||||
|
||||
- [ ] **Extend Auth.js Types Globally**
|
||||
- **Where:** `src/app.d.ts`
|
||||
- **Why:** TypeScript doesn't inherently know `session.user.id` exists, leading to hacky workarounds.
|
||||
- **Fix:** Override `@auth/sveltekit` Session types in `app.d.ts` to include `id` and `email` strictly.
|
||||
|
||||
- [ ] **Consider Adopting a Form Library**
|
||||
- **Where:** `src/lib/extractFormData.ts`
|
||||
- **Why:** Custom form extractors lack instant client-side validation and seamless server-side error mapping.
|
||||
- **Fix:** Consider switching to `sveltekit-superforms` which integrates well with Valibot.
|
||||
|
||||
## ✨ Nice to have (UX & Polish)
|
||||
|
||||
- [ ] **Clarify File Naming (`auth.ts` vs `auth.ts`)**
|
||||
- Rename `src/lib/auth.ts` to `session.ts` or similar to distinguish from `src/auth.ts` (Auth.js setup).
|
||||
|
||||
- [ ] **Abstract Heavy Database Queries**
|
||||
- Move complex aggregations (like computing averages in `src/routes/+page.server.ts`) into a dedicated `src/lib/server/db/queries.ts` file to keep routes clean.
|
||||
|
||||
- [ ] **Clean up Redundant Imports**
|
||||
- In `src/routes/+layout.server.ts`, change `import { getSession as getSession }` to `import { getSession }`.
|
||||
|
||||
## 🧪 Testing Plan
|
||||
|
||||
- [ ] **Add Playwright (End-to-End Testing)**
|
||||
- Install Playwright to test SvelteKit server actions, DB integration, and Flowbite forms holistically.
|
||||
- [ ] **Add Vitest + Svelte Testing Library (Unit/Component Testing)**
|
||||
- Set up Vitest to test UI components (`AktiCard`, `AktiEditor`) and utility functions (`extractFormData`) in isolation.
|
||||
+1
-7
@@ -10,8 +10,6 @@
|
||||
"prepare": "husky",
|
||||
"check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json",
|
||||
"check:watch": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json --watch",
|
||||
"test": "vitest run",
|
||||
"test:watch": "vitest",
|
||||
"format": "prettier --write .",
|
||||
"lint": "prettier --check . && eslint .",
|
||||
"db:start": "docker compose up",
|
||||
@@ -30,8 +28,6 @@
|
||||
"@sveltejs/kit": "^2.49.0",
|
||||
"@sveltejs/vite-plugin-svelte": "^6.2.1",
|
||||
"@tailwindcss/vite": "^4.1.17",
|
||||
"@testing-library/jest-dom": "^6.9.1",
|
||||
"@testing-library/svelte": "^5.3.1",
|
||||
"@tiptap/core": "3.7.2",
|
||||
"@types/node": "^20.19.25",
|
||||
"@types/sanitize-html": "^2.16.1",
|
||||
@@ -44,7 +40,6 @@
|
||||
"flowbite-svelte-icons": "^3.0.0",
|
||||
"globals": "^16.5.0",
|
||||
"husky": "^9.1.7",
|
||||
"jsdom": "^29.0.1",
|
||||
"lowlight": "^3.3.0",
|
||||
"prettier": "^3.6.2",
|
||||
"prettier-plugin-svelte": "^3.4.0",
|
||||
@@ -53,8 +48,7 @@
|
||||
"tailwindcss": "^4.1.17",
|
||||
"typescript": "^5.9.3",
|
||||
"typescript-eslint": "^8.47.0",
|
||||
"vite": "^7.2.4",
|
||||
"vitest": "^4.1.2"
|
||||
"vite": "^7.2.4"
|
||||
},
|
||||
"dependencies": {
|
||||
"@auth/drizzle-adapter": "^1.11.1",
|
||||
|
||||
Generated
-742
File diff suppressed because it is too large
Load Diff
Vendored
-11
@@ -1,5 +1,3 @@
|
||||
import { DefaultSession } from '@auth/sveltekit';
|
||||
|
||||
// See https://svelte.dev/docs/kit/types#app.d.ts
|
||||
// for information about these interfaces
|
||||
declare global {
|
||||
@@ -12,13 +10,4 @@ declare global {
|
||||
}
|
||||
}
|
||||
|
||||
declare module '@auth/sveltekit' {
|
||||
interface Session {
|
||||
user: {
|
||||
id: string;
|
||||
email: string;
|
||||
} & DefaultSession['user'];
|
||||
}
|
||||
}
|
||||
|
||||
export {};
|
||||
|
||||
@@ -1,46 +0,0 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { extractFormData } from './extractFormData';
|
||||
import * as v from 'valibot';
|
||||
|
||||
describe('extractFormData', () => {
|
||||
it('should successfully extract and validate correct form data', async () => {
|
||||
const formData = new FormData();
|
||||
formData.append('name', 'John Doe');
|
||||
formData.append('age', '30');
|
||||
|
||||
const request = new Request('http://localhost', {
|
||||
method: 'POST',
|
||||
body: formData
|
||||
});
|
||||
|
||||
const schema = v.object({
|
||||
name: v.string(),
|
||||
age: v.string()
|
||||
});
|
||||
|
||||
const result = await extractFormData(request, schema);
|
||||
|
||||
expect(result.error).toBeNull();
|
||||
expect(result.data).toEqual({ name: 'John Doe', age: '30' });
|
||||
});
|
||||
|
||||
it('should fail validation with missing required fields', async () => {
|
||||
const formData = new FormData();
|
||||
formData.append('age', '30');
|
||||
|
||||
const request = new Request('http://localhost', {
|
||||
method: 'POST',
|
||||
body: formData
|
||||
});
|
||||
|
||||
const schema = v.object({
|
||||
name: v.string(),
|
||||
age: v.string()
|
||||
});
|
||||
|
||||
const result = await extractFormData(request, schema);
|
||||
|
||||
expect(result.data).toBeUndefined();
|
||||
expect(result.error).toBeTypeOf('string');
|
||||
});
|
||||
});
|
||||
@@ -2,7 +2,7 @@ import { db } from '$lib/server/db';
|
||||
import { aktis, ratings } from '$lib/server/db/schema';
|
||||
import { avg, eq } from 'drizzle-orm';
|
||||
|
||||
export async function getAktisWithAvgRating(limit = 20, offset = 0) {
|
||||
export async function getAktisWithAvgRating() {
|
||||
return await db
|
||||
.select({
|
||||
id: aktis.id,
|
||||
@@ -12,7 +12,5 @@ export async function getAktisWithAvgRating(limit = 20, offset = 0) {
|
||||
})
|
||||
.from(aktis)
|
||||
.leftJoin(ratings, eq(aktis.id, ratings.aktiId))
|
||||
.groupBy(aktis.id, aktis.title, aktis.summary)
|
||||
.limit(limit)
|
||||
.offset(offset);
|
||||
.groupBy(aktis.id, aktis.title, aktis.summary);
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import type { Session } from '@auth/sveltekit';
|
||||
import type { Session, User } from '@auth/sveltekit';
|
||||
import { error } from '@sveltejs/kit';
|
||||
import { db } from './db';
|
||||
import { users } from './db/schema';
|
||||
@@ -8,29 +8,32 @@ interface Event {
|
||||
auth(): Promise<Session | null>;
|
||||
};
|
||||
}
|
||||
interface UserWithId extends User {
|
||||
id: string;
|
||||
email: string;
|
||||
}
|
||||
|
||||
export async function ensureAuth(event: Event): Promise<Session['user']> {
|
||||
export async function ensureAuth(event: Event): Promise<UserWithId> {
|
||||
const session = await getSession(event);
|
||||
if (!session) error(401, { message: 'Du muesch di zersch iiloge' });
|
||||
|
||||
const user = session.user;
|
||||
const user = session?.user;
|
||||
if (!user || !user.email || !user.id) {
|
||||
error(401, { message: 'Du muesch di zersch iiloge' });
|
||||
}
|
||||
|
||||
return user;
|
||||
return { ...user, id: user.id, email: user.email }; // weird thingamajig so that ts compiler is happy
|
||||
}
|
||||
export async function getSession(event: Event) {
|
||||
const session = await event.locals.auth();
|
||||
if (!session) return null;
|
||||
if (!session.user) error(403, { message: 'Di gits garnid. Vilich nomau usloge u iiloge?' });
|
||||
if (!session?.user?.email) return null;
|
||||
|
||||
const res = await db
|
||||
.select({ id: users.id })
|
||||
.from(users)
|
||||
.limit(1)
|
||||
.where(eq(users.email, session.user.email));
|
||||
.where(eq(users.email, session.user.email ?? 'eaf9302d-9525-4f3e-8147-9620d2076f67')); //uuid as default to find nothing
|
||||
|
||||
if (!res[0]?.id) {
|
||||
error(403, { message: 'Di gits garnid. Vilich nomau usloge u iiloge?' });
|
||||
|
||||
@@ -1,11 +1,8 @@
|
||||
import { getAktisWithAvgRating } from '$lib/server/db/queries';
|
||||
import type { PageServerLoad } from './$types';
|
||||
|
||||
export const load: PageServerLoad = async ({ url }) => {
|
||||
const offset = Number(url.searchParams.get('offset')) || 0;
|
||||
const limit = 20;
|
||||
|
||||
const a = await getAktisWithAvgRating(limit, offset);
|
||||
export const load: PageServerLoad = async () => {
|
||||
const a = await getAktisWithAvgRating();
|
||||
|
||||
return {
|
||||
aktis: a.map((a) => ({ ...a, rating: a.rating ? parseFloat(a.rating) : undefined }))
|
||||
|
||||
+1
-44
@@ -1,55 +1,12 @@
|
||||
<script lang="ts">
|
||||
import AktiCard from '$lib/components/akti/AktiCard.svelte';
|
||||
import type { PageProps } from './$types';
|
||||
import { Spinner } from 'flowbite-svelte';
|
||||
|
||||
let { data }: PageProps = $props();
|
||||
|
||||
let aktis = $state(data.aktis);
|
||||
let offset = $state(data.aktis.length);
|
||||
let loading = $state(false);
|
||||
let hasMore = $state(data.aktis.length >= 20);
|
||||
|
||||
async function loadMore() {
|
||||
if (loading || !hasMore) return;
|
||||
loading = true;
|
||||
const res = await fetch(`/api/aktis?offset=${offset}`);
|
||||
const newAktis = await res.json();
|
||||
|
||||
if (newAktis.length < 20) {
|
||||
hasMore = false;
|
||||
}
|
||||
|
||||
aktis = [...aktis, ...newAktis];
|
||||
offset += newAktis.length;
|
||||
loading = false;
|
||||
}
|
||||
|
||||
function infiniteScroll(node: HTMLElement) {
|
||||
const observer = new IntersectionObserver((entries) => {
|
||||
if (entries[0].isIntersecting) {
|
||||
loadMore();
|
||||
}
|
||||
});
|
||||
observer.observe(node);
|
||||
return {
|
||||
destroy() {
|
||||
observer.disconnect();
|
||||
}
|
||||
};
|
||||
}
|
||||
</script>
|
||||
|
||||
<div class="grid gap-5 grid-cols-1 sm:grid-cols-2 lg:grid-cols-3 3xl:grid-cols-5">
|
||||
{#each aktis as akti (akti.id)}
|
||||
{#each data.aktis as akti (akti.id)}
|
||||
<AktiCard {akti}></AktiCard>
|
||||
{/each}
|
||||
</div>
|
||||
|
||||
<div class="mt-10 flex justify-center h-20">
|
||||
{#if loading}
|
||||
<Spinner />
|
||||
{:else if hasMore}
|
||||
<div use:infiniteScroll></div>
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { db } from '$lib/server/db';
|
||||
import { aktis, ratings } from '$lib/server/db/schema';
|
||||
import { error, fail, redirect, type Actions } from '@sveltejs/kit';
|
||||
import { error, redirect, type Actions } from '@sveltejs/kit';
|
||||
import { and, eq } from 'drizzle-orm';
|
||||
import type { PageServerLoad } from './$types';
|
||||
import { ensureAuth } from '$lib/server/session';
|
||||
@@ -56,7 +56,7 @@ export const actions = {
|
||||
)
|
||||
).data;
|
||||
|
||||
if (!changeRequest) return fail(400, { message: 'Invalid data' });
|
||||
if (!changeRequest) return error(400);
|
||||
|
||||
changeRequest.body = sanitizeHtml(changeRequest.body);
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import type { PageServerLoad } from './$types';
|
||||
import { ensureAuth } from '$lib/server/session';
|
||||
import { error, fail, redirect, type Actions } from '@sveltejs/kit';
|
||||
import { error, redirect, type Actions } from '@sveltejs/kit';
|
||||
import { extractFormData } from '$lib/extractFormData';
|
||||
import { aktis, ratings } from '$lib/server/db/schema';
|
||||
import { eq } from 'drizzle-orm';
|
||||
@@ -54,7 +54,7 @@ export const actions = {
|
||||
)
|
||||
).data;
|
||||
|
||||
if (!rating) return fail(400, { message: 'Invalid data' });
|
||||
if (!rating) return error(400);
|
||||
|
||||
await db.insert(ratings).values({
|
||||
...rating,
|
||||
|
||||
@@ -1,11 +0,0 @@
|
||||
import { getAktisWithAvgRating } from '$lib/server/db/queries';
|
||||
import { json, type RequestHandler } from '@sveltejs/kit';
|
||||
|
||||
export const GET: RequestHandler = async ({ url }) => {
|
||||
const offset = Number(url.searchParams.get('offset')) || 0;
|
||||
const limit = 20;
|
||||
|
||||
const a = await getAktisWithAvgRating(limit, offset);
|
||||
|
||||
return json(a.map((a) => ({ ...a, rating: a.rating ? parseFloat(a.rating) : undefined })));
|
||||
};
|
||||
@@ -1 +0,0 @@
|
||||
import '@testing-library/jest-dom/vitest';
|
||||
@@ -1,12 +0,0 @@
|
||||
import { defineConfig } from 'vitest/config';
|
||||
import { sveltekit } from '@sveltejs/kit/vite';
|
||||
import { svelteTesting } from '@testing-library/svelte/vite';
|
||||
|
||||
export default defineConfig({
|
||||
plugins: [sveltekit(), svelteTesting()],
|
||||
test: {
|
||||
include: ['src/**/*.{test,spec}.{js,ts}'],
|
||||
environment: 'jsdom',
|
||||
setupFiles: ['./vitest-setup.ts']
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user