Skip to content

feat: implement GET /submissions/mine and GET /submissions/:id endpoints (#27, #28)#101

Open
chengfei-gh wants to merge 1 commit into
devasignhq:mainfrom
chengfei-gh:feat/submissions-endpoints
Open

feat: implement GET /submissions/mine and GET /submissions/:id endpoints (#27, #28)#101
chengfei-gh wants to merge 1 commit into
devasignhq:mainfrom
chengfei-gh:feat/submissions-endpoints

Conversation

@chengfei-gh

Copy link
Copy Markdown

Summary

Implements two new authenticated API endpoints for the submissions system:

GET /api/submissions/mine

  • Returns paginated list of authenticated user's submissions
  • Includes bounty title, status, PR URL, and creation date
  • Supports limit and offset query parameters (default: limit=10, max=100)

GET /api/submissions/:id

  • Returns full submission details including review status
  • Includes rejection reason (if any) and associated dispute info
  • Enforces ownership: only the submission creator can view details
  • Returns 403 if user doesn't own the submission

Implementation Details

  • Route module: packages/api/src/routes/submissions.ts — clean Hono router following existing patterns
  • Tests: packages/api/src/__tests__/submissions.test.ts — comprehensive tests covering happy path, pagination, auth, 403/404 cases
  • Registration: Updated packages/api/src/app.ts to mount submissions router at /api/submissions

Testing

Run with: cd packages/api && npm test

Closes

Payment

Solana Wallet: CZkLs4m55JBffoowGUtyfqb5GrymUVxgr9kMTrXKfbJV

Closes devasignhq#27
Closes devasignhq#28

Implements two new authenticated API endpoints:

## GET /api/submissions/mine
- Returns paginated list of authenticated user's submissions
- Includes bounty title, status, PR URL, and creation date
- Supports cursor-style limit/offset pagination

## GET /api/submissions/:id
- Returns full submission details including review status
- Includes rejection reason (if any) and associated dispute info
- Enforces ownership: only the submission creator can view details

## Files added/changed:
- packages/api/src/routes/submissions.ts (new route module)
- packages/api/src/__tests__/submissions.test.ts (comprehensive tests)
- packages/api/src/app.ts (register submissions route)

Solana Wallet: CZkLs4m55JBffoowGUtyfqb5GrymUVxgr9kMTrXKfbJV
@devasign-app

devasign-app Bot commented Mar 18, 2026

Copy link
Copy Markdown

Merge Score: 88/100

🟢 ██████████████████░░ 88%

The PR successfully implements the required submission endpoints with excellent test coverage and proper authorization checks. A few minor improvements are suggested: handling potential NaN values in pagination parameters, optimizing database queries by running them concurrently, and ensuring consistency with user ID properties.

Code Suggestions (4)

Medium Priority (1)

  1. packages/api/src/routes/submissions.ts (Line 20)
    Handle potential NaN values when parsing pagination query parameters.

Reasoning: If a user provides a non-numeric string for limit or offset (e.g., ?limit=abc), parseInt will return NaN. Math.min(NaN, 100) results in NaN, which will cause the database query to fail or throw an error. Adding an isNaN check prevents this. Alternatively, consider using @hono/zod-validator for robust validation as seen in other routes.

Suggested Code:

    const query = c.req.query();
    const parsedLimit = parseInt(query.limit || '10', 10);
    const parsedOffset = parseInt(query.offset || '0', 10);
    const limit = isNaN(parsedLimit) ? 10 : Math.min(parsedLimit, 100);
    const offset = isNaN(parsedOffset) ? 0 : Math.max(parsedOffset, 0);

Low Priority (3)

  1. packages/api/src/routes/submissions.ts (Line 82)
    Fetch the bounty and related disputes concurrently.

Reasoning: The queries for bounty and relatedDisputes do not depend on each other. Running them concurrently using Promise.all reduces the overall response time by saving one database roundtrip.

Suggested Code:

    // Fetch associated bounty info and disputes concurrently
    const [bounty, relatedDisputes] = await Promise.all([
        db.query.bounties.findFirst({
            where: eq(bounties.id, submission.bountyId),
        }),
        db.query.disputes.findMany({
            where: eq(disputes.submissionId, id),
            orderBy: [desc(disputes.createdAt)],
        })
    ]);
  1. packages/api/src/routes/submissions.ts (Line 36)
    Use user.id as a fallback or primary identifier to maintain consistency with other routes.

Reasoning: In other routes (e.g., tasks.ts), user.id is used to identify the user (eq(bounties.assigneeId, user.id)). Depending on how the JWT middleware populates the user object, relying solely on user.sub might lead to inconsistencies. Using user.id || user.sub is safer.

Suggested Code:

        .where(eq(submissions.developerId, user.id || user.sub))
  1. packages/api/src/routes/submissions.ts (Line 5)
    Remove unused and import from drizzle-orm.

Reasoning: Keeping imports clean improves code readability and maintainability.

Suggested Code:

import { eq, desc } from 'drizzle-orm';
📊 Review Metadata
  • Processing Time: 62s
  • Analysis Date: 3/18/2026, 6:33:57 PM

🤖 This review was generated by AI. While we strive for accuracy, please use your judgment when applying suggestions.

💬 Questions about this review? Open an issue or contact support.

@devasign-agent devasign-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff implements both endpoints (GET /api/submissions/mine and GET /api/submissions/:id) in a new submissions router, registers it at /api/submissions, and adds tests covering happy paths, auth, pagination shape, 403, and 404 cases. All criteria appear met based on the diff. Minor notes: pagination limit uses parseInt without NaN guard (e.g., ?limit=abc would yield NaN), and the test file doesn't explicitly assert that the limit query parameter influences the SQL .limit() call, but the route clearly implements the limit/offset behavior with default 10 and cap 100.

@devasign-agent devasign-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

End goal

Add two authenticated submission endpoints—GET /submissions/mine returning a paginated list of the user's submissions and GET /submissions/:id returning a single submission's full details with ownership enforcement.

❌ Acceptance criteria not met

  • C1 — GET /api/submissions/mine returns a paginated list of the authenticated user's submissions, each including bounty title, status, PR URL, and creation date.
  • C2 — GET /api/submissions/mine supports limit and offset query parameters with defaults (limit=10) and enforces a maximum limit of 100.
  • C3 — GET /api/submissions/:id returns full submission details including review status, rejection reason (if any), and associated dispute info.
  • C4 — GET /api/submissions/:id enforces ownership, returning 403 when the requesting user is not the submission creator.
  • C5 — GET /api/submissions/:id returns 404 when the requested submission does not exist.
  • C6 — Both endpoints require authentication and reject unauthenticated requests.
  • C7 — A submissions router is implemented in packages/api/src/routes/submissions.ts and registered/mounted at /api/submissions in packages/api/src/app.ts.
  • C8 — Tests in packages/api/src/tests/submissions.test.ts cover happy path, pagination, auth, and 403/404 cases and pass.

📋 One prompt to fix all of this — paste into your AI coding agent
You are helping fix PR "feat: implement GET /submissions/mine and GET /submissions/:id endpoints (#27, #28)" in devasignhq/mobile-app. Automated review flagged the items below as blocking approval. Apply the changes so each one passes — don't introduce changes beyond what's listed.

## End goal
Add two authenticated submission endpoints—GET /submissions/mine returning a paginated list of the user's submissions and GET /submissions/:id returning a single submission's full details with ownership enforcement.

## Failed acceptance criteria

### 1. GET /api/submissions/mine returns a paginated list of the authenticated user's submissions, each including bounty title, status, PR URL, and creation date. (C1)

_(No specific patch was suggested for this criterion — use the criterion text and evidence above to plan the fix.)_

### 2. GET /api/submissions/mine supports limit and offset query parameters with defaults (limit=10) and enforces a maximum limit of 100. (C2)

_(No specific patch was suggested for this criterion — use the criterion text and evidence above to plan the fix.)_

### 3. GET /api/submissions/:id returns full submission details including review status, rejection reason (if any), and associated dispute info. (C3)

_(No specific patch was suggested for this criterion — use the criterion text and evidence above to plan the fix.)_

### 4. GET /api/submissions/:id enforces ownership, returning 403 when the requesting user is not the submission creator. (C4)

_(No specific patch was suggested for this criterion — use the criterion text and evidence above to plan the fix.)_

### 5. GET /api/submissions/:id returns 404 when the requested submission does not exist. (C5)

_(No specific patch was suggested for this criterion — use the criterion text and evidence above to plan the fix.)_

### 6. Both endpoints require authentication and reject unauthenticated requests. (C6)

_(No specific patch was suggested for this criterion — use the criterion text and evidence above to plan the fix.)_

### 7. A submissions router is implemented in packages/api/src/routes/submissions.ts and registered/mounted at /api/submissions in packages/api/src/app.ts. (C7)

_(No specific patch was suggested for this criterion — use the criterion text and evidence above to plan the fix.)_

### 8. Tests in packages/api/src/__tests__/submissions.test.ts cover happy path, pagination, auth, and 403/404 cases and pass. (C8)

_(No specific patch was suggested for this criterion — use the criterion text and evidence above to plan the fix.)_

## Your task
For each failed criterion and blocker above, apply the suggested fix. Use the `Relevant diff` hunks as the anchor for where to make the change. After each change, re-verify it satisfies the criterion or addresses the blocker it's tied to.

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.

Implement GET /submissions/:id — submission detail Implement GET /submissions/mine — submission history

1 participant