Skip to content

Conversation

@xlamn
Copy link
Collaborator

@xlamn xlamn commented Jan 2, 2026

Release Checklist

Pre-Release

  • Check migrations
    • No database related infos (sqldb-xxx)
    • Impact on GS (new/removed columns)
  • Check for linter errors (in PR)
  • Test basic user operations (on DFX services)
    • Login/logout
    • Buy/sell payment request
    • KYC page

Post-Release

  • Test basic user operations
  • Monitor application insights log

@TaprootFreak
Copy link
Collaborator

Code Review

1. HTTP Status Code Inkonsistenz ⚠️

Die Exceptions erben von BadRequestException (HTTP 400), aber der Body enthält statusCode: 403:

export class RegistrationRequiredException extends BadRequestException {
    constructor(message = 'RealUnit registration required') {
        super({
            statusCode: 403,  // Body sagt 403, aber HTTP Response ist 400
            error: 'Bad Request',
            // ...
        });
    }
}

Empfehlung: Wenn 403 gewünscht ist, sollte ForbiddenException verwendet werden:

import { ForbiddenException } from '@nestjs/common';

export class RegistrationRequiredException extends ForbiddenException {
    constructor(message = 'RealUnit registration required') {
        super({
            statusCode: 403,
            error: 'Forbidden',
            code: 'REGISTRATION_REQUIRED',
            message,
        });
    }
}

2. TypeScript Type: String vs string

message: String,  // ❌ Wrapper-Typ

Sollte der primitive Typ sein:

message: string,  // ✅ Primitiver Typ

3. Dateiname-Konvention

buy_exceptions.ts verwendet Unterstriche. Die Projekt-Konvention ist kebab-case:

buy-exceptions.ts


4. Logik-Änderung

Die Validierungsreihenfolge wurde geändert:

  • Vorher: KYC Level → Registration
  • Nachher: Registration → KYC Level

Falls beabsichtigt, ist das in Ordnung - nur zur Info, da es das User-Feedback ändert.

@TaprootFreak TaprootFreak merged commit bf08a27 into develop Jan 2, 2026
1 check passed
@TaprootFreak TaprootFreak deleted the feat/buy-different-exceptions branch January 2, 2026 11:39
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