Compare commits
13 Commits
c71a28fd98
..
main
| Author | SHA1 | Date | |
|---|---|---|---|
| aae537b312 | |||
| 19238edf57 | |||
| 0f8ad4a36a | |||
| 9fac4d0149 | |||
| 0de1cdfb8d | |||
| 6d86630b4a | |||
| 89c86a2d32 | |||
| 15829e1b19 | |||
| 97e11a4de7 | |||
| 070638b2f1 | |||
| f58af022dc | |||
| 4475879330 | |||
| 7492680457 |
@@ -3,7 +3,9 @@ name: Commit
|
||||
on:
|
||||
push:
|
||||
# only trigger on branches, not on tags
|
||||
branches: '**'
|
||||
branches:
|
||||
- 'main'
|
||||
- 'dev'
|
||||
|
||||
jobs:
|
||||
# Job 1: Lint and Test (Type Check)
|
||||
|
||||
@@ -17,8 +17,10 @@ jobs:
|
||||
http = true
|
||||
insecure = true
|
||||
|
||||
- name: login
|
||||
- name: login gitea registry
|
||||
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,8 +18,10 @@ jobs:
|
||||
http = true
|
||||
insecure = true
|
||||
|
||||
- name: login
|
||||
- name: login gitea registry
|
||||
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,2 +1,3 @@
|
||||
pnpm run format
|
||||
pnpm run lint
|
||||
pnpm run test
|
||||
|
||||
@@ -1,65 +0,0 @@
|
||||
# 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.
|
||||
@@ -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() {
|
||||
export async function getAktisWithAvgRating(limit = 20, offset = 0) {
|
||||
return await db
|
||||
.select({
|
||||
id: aktis.id,
|
||||
@@ -12,5 +12,7 @@ export async function getAktisWithAvgRating() {
|
||||
})
|
||||
.from(aktis)
|
||||
.leftJoin(ratings, eq(aktis.id, ratings.aktiId))
|
||||
.groupBy(aktis.id, aktis.title, aktis.summary);
|
||||
.groupBy(aktis.id, aktis.title, aktis.summary)
|
||||
.limit(limit)
|
||||
.offset(offset);
|
||||
}
|
||||
|
||||
@@ -1,8 +1,11 @@
|
||||
import { getAktisWithAvgRating } from '$lib/server/db/queries';
|
||||
import type { PageServerLoad } from './$types';
|
||||
|
||||
export const load: PageServerLoad = async () => {
|
||||
const a = await getAktisWithAvgRating();
|
||||
export const load: PageServerLoad = async ({ url }) => {
|
||||
const offset = Number(url.searchParams.get('offset')) || 0;
|
||||
const limit = 20;
|
||||
|
||||
const a = await getAktisWithAvgRating(limit, offset);
|
||||
|
||||
return {
|
||||
aktis: a.map((a) => ({ ...a, rating: a.rating ? parseFloat(a.rating) : undefined }))
|
||||
|
||||
+44
-1
@@ -1,12 +1,55 @@
|
||||
<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 data.aktis as akti (akti.id)}
|
||||
{#each 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, redirect, type Actions } from '@sveltejs/kit';
|
||||
import { error, fail, 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 error(400);
|
||||
if (!changeRequest) return fail(400, { message: 'Invalid data' });
|
||||
|
||||
changeRequest.body = sanitizeHtml(changeRequest.body);
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import type { PageServerLoad } from './$types';
|
||||
import { ensureAuth } from '$lib/server/session';
|
||||
import { error, redirect, type Actions } from '@sveltejs/kit';
|
||||
import { error, fail, 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 error(400);
|
||||
if (!rating) return fail(400, { message: 'Invalid data' });
|
||||
|
||||
await db.insert(ratings).values({
|
||||
...rating,
|
||||
|
||||
@@ -0,0 +1,11 @@
|
||||
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 })));
|
||||
};
|
||||
Reference in New Issue
Block a user