Skip to content

Commit d2069fd

Browse files
committed
guest check on event creation bug
1 parent a81ffb6 commit d2069fd

4 files changed

Lines changed: 118 additions & 163 deletions

File tree

.claude/skills/general-practices/backend-endpoints/SKILL.md

Lines changed: 53 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,15 @@ If a service throws an exception, it bubbles up through the controller's `next(e
6363

6464
## File Locations
6565

66-
| Layer | Path | Naming |
67-
|-------|------|--------|
68-
| Entry point | `src/backend/index.ts` | |
69-
| Routes | `src/backend/src/routes/{feature}.routes.ts` | `{feature}Router` |
70-
| Controllers | `src/backend/src/controllers/{feature}.controllers.ts` | `{Feature}Controller` class |
71-
| Services | `src/backend/src/services/{feature}.services.ts` | `{Feature}Service` class |
72-
| Validation | `src/backend/src/utils/validation.utils.ts` | Shared validators |
73-
| Errors | `src/backend/src/utils/errors.utils.ts` | `HttpException` subclasses |
74-
| Express types | `src/backend/custom.d.ts` | `currentUser` and `organization` on `Request` |
66+
| Layer | Path | Naming |
67+
| ------------- | ------------------------------------------------------ | --------------------------------------------- |
68+
| Entry point | `src/backend/index.ts` | |
69+
| Routes | `src/backend/src/routes/{feature}.routes.ts` | `{feature}Router` |
70+
| Controllers | `src/backend/src/controllers/{feature}.controllers.ts` | `{Feature}Controller` class |
71+
| Services | `src/backend/src/services/{feature}.services.ts` | `{Feature}Service` class |
72+
| Validation | `src/backend/src/utils/validation.utils.ts` | Shared validators |
73+
| Errors | `src/backend/src/utils/errors.utils.ts` | `HttpException` subclasses |
74+
| Express types | `src/backend/custom.d.ts` | `currentUser` and `organization` on `Request` |
7575

7676
For query args and transformers, see the [query-args-and-transformers](../query-args-and-transformers/SKILL.md) skill.
7777

@@ -80,11 +80,13 @@ For query args and transformers, see the [query-args-and-transformers](../query-
8080
The full URL path for any endpoint is the **combination** of the base path registered in `src/backend/index.ts` and the route path in the router file. This is a very common source of confusion.
8181

8282
For example, if `index.ts` registers:
83+
8384
```typescript
8485
app.use('/calendar', calendarRouter);
8586
```
8687

8788
And the router defines:
89+
8890
```typescript
8991
calendarRouter.post('/shop/create', ...);
9092
```
@@ -103,13 +105,8 @@ Add validation rules using `express-validator` and the helpers from `validation.
103105
// src/backend/src/routes/calendar.routes.ts
104106
import express from 'express';
105107
import { body } from 'express-validator';
106-
import {
107-
nonEmptyString,
108-
isDate,
109-
validateInputs
110-
} from '../utils/validation.utils.js';
111-
import CalendarController
112-
from '../controllers/calendar.controllers.js';
108+
import { nonEmptyString, isDate, validateInputs } from '../utils/validation.utils.js';
109+
import CalendarController from '../controllers/calendar.controllers.js';
113110

114111
const calendarRouter = express.Router();
115112

@@ -135,6 +132,7 @@ export default calendarRouter;
135132
- URL params use `param()`, query strings use `query()`, body fields use `body()`.
136133

137134
**When to abstract validators:** Keep validation inline in the route by default. Only extract validators into `validation.utils.ts` when:
135+
138136
- The request body contains a **nested object** that is itself a known entity (e.g., a work package embedded inside a project create payload). Create a named validator array like `workPackageProposedChangesValidators`.
139137
- The **same set of validations** is repeated across multiple routes (e.g., `descriptionBulletsValidators` used in both work package and project routes).
140138

@@ -146,8 +144,7 @@ If creating a brand new feature router, register it in `src/backend/index.ts`:
146144

147145
```typescript
148146
// src/backend/index.ts
149-
import calendarRouter
150-
from './src/routes/calendar.routes.js';
147+
import calendarRouter from './src/routes/calendar.routes.js';
151148

152149
// ... after getUserAndOrganization middleware ...
153150
app.use('/calendar', calendarRouter);
@@ -162,30 +159,18 @@ Controllers follow a rigid structure: try/catch, extract request data, call serv
162159
```typescript
163160
// src/backend/src/controllers/calendar.controllers.ts
164161
import { NextFunction, Request, Response } from 'express';
165-
import CalendarService
166-
from '../services/calendar.services.js';
162+
import CalendarService from '../services/calendar.services.js';
167163

168164
export default class CalendarController {
169-
static async createShop(
170-
req: Request,
171-
res: Response,
172-
next: NextFunction
173-
) {
165+
static async createShop(req: Request, res: Response, next: NextFunction) {
174166
try {
175-
const { name, description, dateEstablished }
176-
= req.body;
167+
const { name, description, dateEstablished } = req.body;
177168

178169
// Parse date strings to Date objects
179170
// before passing to the service
180171
const parsedDate = new Date(dateEstablished);
181172

182-
const shop = await CalendarService.createShop(
183-
req.currentUser,
184-
name,
185-
description,
186-
parsedDate,
187-
req.organization
188-
);
173+
const shop = await CalendarService.createShop(req.currentUser, name, description, parsedDate, req.organization);
189174

190175
res.status(200).json(shop);
191176
} catch (error: unknown) {
@@ -214,16 +199,10 @@ Services contain all business logic.
214199
// src/backend/src/services/calendar.services.ts
215200
import { User, Shop, notGuest } from 'shared';
216201
import prisma from '../prisma/prisma.js';
217-
import {
218-
AccessDeniedGuestException,
219-
HttpException
220-
} from '../utils/errors.utils.js';
221-
import { shopTransformer }
222-
from '../transformers/calendar.transformer.js';
223-
import { getShopQueryArgs }
224-
from '../prisma-query-args/shop.query-args.js';
225-
import { userHasPermission }
226-
from '../utils/users.utils.js';
202+
import { AccessDeniedGuestException, HttpException } from '../utils/errors.utils.js';
203+
import { shopTransformer } from '../transformers/calendar.transformer.js';
204+
import { getShopQueryArgs } from '../prisma-query-args/shop.query-args.js';
205+
import { userHasPermission } from '../utils/users.utils.js';
227206
import { Organization } from '@prisma/client';
228207

229208
export default class CalendarService {
@@ -249,16 +228,8 @@ export default class CalendarService {
249228
organization: Organization
250229
): Promise<Shop> {
251230
// 1. Permission check
252-
if (
253-
!(await userHasPermission(
254-
submitter.userId,
255-
organization.organizationId,
256-
notGuest
257-
))
258-
) {
259-
throw new AccessDeniedGuestException(
260-
'create shops'
261-
);
231+
if (!(await userHasPermission(submitter.userId, organization.organizationId, notGuest))) {
232+
throw new AccessDeniedGuestException('create shops');
262233
}
263234

264235
// 2. Business rule validation (inline select)
@@ -272,10 +243,7 @@ export default class CalendarService {
272243
});
273244

274245
if (duplicate) {
275-
throw new HttpException(
276-
400,
277-
'A shop with that name already exists'
278-
);
246+
throw new HttpException(400, 'A shop with that name already exists');
279247
}
280248

281249
// 3. Database write (query args for response)
@@ -317,22 +285,20 @@ Every write endpoint (and some sensitive reads) needs a permission check at the
317285

318286
```typescript
319287
import {
320-
notGuest, // members and above
321-
isLeadership, // leads and above
322-
isHead, // heads and above
323-
isAdmin // admins and app-admins only
288+
notGuest, // members and above
289+
isLeadership, // leads and above
290+
isHead, // heads and above
291+
isAdmin // admins and app-admins only
324292
} from 'shared';
325293

326294
if (
327295
!(await userHasPermission(
328296
submitter.userId,
329297
organization.organizationId,
330-
isHead // choose the right level
298+
isHead // choose the right level
331299
))
332300
) {
333-
throw new AccessDeniedAdminOnlyException(
334-
'create event types'
335-
);
301+
throw new AccessDeniedAdminOnlyException('create event types');
336302
}
337303
```
338304

@@ -351,34 +317,34 @@ Match the exception class to the level: `AccessDeniedGuestException` for `notGue
351317

352318
Services throw exceptions from `src/backend/src/utils/errors.utils.ts`. The global `errorHandler` middleware catches them.
353319

354-
| Exception | Status | When to Use |
355-
|-----------|--------|-------------|
356-
| `HttpException(status, msg)` | any | General-purpose with custom status |
357-
| `NotFoundException(name, id)` | 404 | Entity not found |
358-
| `DeletedException(name, id)` | 404 | Entity is soft-deleted |
359-
| `AccessDeniedException(msg?)` | 403 | Generic permission failure |
360-
| `AccessDeniedAdminOnlyException(action)` | 403 | Non-admin attempting admin action |
361-
| `AccessDeniedMemberException(action)` | 403 | Guest/member attempting restricted action |
362-
| `AccessDeniedGuestException(action)` | 403 | Guest attempting non-guest action |
363-
| `InvalidOrganizationException(item)` | 400 | Entity not in current org |
320+
| Exception | Status | When to Use |
321+
| ---------------------------------------- | ------ | ----------------------------------------- |
322+
| `HttpException(status, msg)` | any | General-purpose with custom status |
323+
| `NotFoundException(name, id)` | 404 | Entity not found |
324+
| `DeletedException(name, id)` | 404 | Entity is soft-deleted |
325+
| `AccessDeniedException(msg?)` | 403 | Generic permission failure |
326+
| `AccessDeniedAdminOnlyException(action)` | 403 | Non-admin attempting admin action |
327+
| `AccessDeniedMemberException(action)` | 403 | Guest/member attempting restricted action |
328+
| `AccessDeniedGuestException(action)` | 403 | Guest attempting non-guest action |
329+
| `InvalidOrganizationException(item)` | 400 | Entity not in current org |
364330

365331
The `name` parameter for `NotFoundException` and `DeletedException` MUST be one of the values in the `ExceptionObjectNames` type union in `errors.utils.ts`. Add your entity to that type if it's not listed.
366332

367333
## Validation Helpers
368334

369335
`src/backend/src/utils/validation.utils.ts` provides reusable validation chains:
370336

371-
| Helper | Validates |
372-
|--------|-----------|
373-
| `nonEmptyString(body('x'))` | Non-empty string |
374-
| `intMinZero(body('x'))` | Integer ≥ 0, not a string |
375-
| `decimalMinZero(body('x'))` | Decimal ≥ 0 |
376-
| `isDate(body('x'))` | Parseable date string |
377-
| `isOptionalDate(body('x'))` | Optional parseable date |
378-
| `isRole(body('x'))` | Valid `RoleEnum` value |
379-
| `isStatus(body('x'))` | Valid `WbsElementStatus` |
380-
| `isEventStatus(body('x'))` | Valid `Event_Status` |
381-
| `validateInputs` | Runs validation, returns 400 |
337+
| Helper | Validates |
338+
| --------------------------- | ---------------------------- |
339+
| `nonEmptyString(body('x'))` | Non-empty string |
340+
| `intMinZero(body('x'))` | Integer ≥ 0, not a string |
341+
| `decimalMinZero(body('x'))` | Decimal ≥ 0 |
342+
| `isDate(body('x'))` | Parseable date string |
343+
| `isOptionalDate(body('x'))` | Optional parseable date |
344+
| `isRole(body('x'))` | Valid `RoleEnum` value |
345+
| `isStatus(body('x'))` | Valid `WbsElementStatus` |
346+
| `isEventStatus(body('x'))` | Valid `Event_Status` |
347+
| `validateInputs` | Runs validation, returns 400 |
382348

383349
For complex reusable validators, spread them: `...descriptionBulletsValidators`.
384350

0 commit comments

Comments
 (0)