Repository navigation
Test/template community routes - #94
Conversation
…mplates tables These two tables were defined in the Prisma schema but never had a corresponding CREATE TABLE migration. The database was bootstrapped via prisma db push, which left the migration history empty while the tables existed only if the full schema had been pushed at a point that included them. - Creates emplate_reviews with stars, text, templateId, userId fields - Creates user_owned_templates with userId, templateId, clonedFormId - Adds all unique constraints, indexes, and foreign keys matching the schema - Uses IF NOT EXISTS guards for idempotent application Closes: relates to Openlabsops#55
The api-tests package sits two levels below the monorepo root where the .env file lives. Without an explicit envFile entry, `bun test` cannot resolve DATABASE_URL, BETTER_AUTH_SECRET, or any other required secrets, causing every test file to fail immediately with 'DATABASE_URL is required'. Added: envFile = "../../.env" This allows running `bun test integration-tests/` directly from the packages/api-tests directory without needing to pass the flag manually.
…emplate Template community tests involve purchase and review records which must be deleted before each test to guarantee isolation. Without clearing these tables, foreign-key constraints (templateReview -> template, userOwnedTemplate -> template) would also prevent the existing `prisma.template.deleteMany()` call from completing. Deletion order matters due to FK constraints: 1. templateReview (references template + user) 2. userOwnedTemplate (references template + user + form) 3. existing tables (response, form, session, account, verification, user, template)
…absops#55) Implements the four integration test cases required by Issue Openlabsops#55: Test Case 1 — POST /api/v1/templates - 401 when unauthenticated - 400 when title is missing - 400 when neither formId nor fields provided - 201 with correct data persisted to DB when valid payload supplied - 201 when snapshotting fields from an existing owned form via formId Test Case 2 — GET /api/v1/templates/community - 200 accessible without authentication - Returns only isPublic=true templates (private ones excluded) - Respects ?category= query filter - Returns correct pagination metadata (total, page, limit, totalPages) - Includes creator user object (id, name) in each listing Test Case 3 — POST /api/v1/templates/:id/purchase - 401 when unauthenticated - 404 for a non-existent template id - 400 when buyer is the template creator (own-purchase guard) - 201: creates UserOwnedTemplate record + clones a Form owned by buyer - Cloned Form is unpublished so the buyer can customize before publishing - Template useCount incremented atomically inside the transaction - 409 on duplicate purchase (unique constraint on userId+templateId) Test Case 4 — POST /api/v1/templates/:id/reviews - 401 when unauthenticated - 403 when user has NOT purchased the template (key requirement of Openlabsops#55) - 403 when the template creator tries to review their own template - 201 after purchase: review persisted and returned with correct stars/text - 400 when stars value exceeds the valid range (> 5) - 409 on duplicate review (unique constraint on templateId+userId) Total: 22 tests, 81 assertions, all passing.
- test: replace Date.now() with crypto.randomUUID() to eliminate flaky test risk caused by millisecond collisions on unique email constraints - test: replace any types in createPublicTemplate helper with AxiosInstance and Record - test: add coverage for purchasing a private template and reviewing a non-existent template - api: fix security bug in purchaseTemplate controller by asserting template.isPublic is true before cloning (blocks purchasing private templates) Closes Openlabsops#55
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used🪛 SQLFluff (4.2.2)packages/db/prisma/migrations/20260630090000_add_template_community_tables/migration.sql[error] 61-61: ADD CONSTRAINT ... FOREIGN KEY should use NOT VALID to avoid locking the table while validating existing rows. (PG01) 🔇 Additional comments (2)
📝 WalkthroughWalkthroughAdds database support and integration tests for template ownership and reviews, and prevents users from purchasing private templates with a ChangesTemplate Community Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/api-tests/integration-tests/modules/template.test.ts`:
- Around line 117-136: The `formId` snapshot test in `template.test.ts` only
checks status, success, and `userId`, so it does not verify the behavior it
claims to cover. Update the `it("should snapshot fields from an existing form
when formId is provided")` case to assert that the created template’s `fields`
match the source form’s fields returned by `/api/v1/forms` and/or the template
response from `/api/v1/templates`, so the test fails if snapshotting breaks.
In
`@packages/db/prisma/migrations/20260630090000_add_template_community_tables/migration.sql`:
- Around line 51-57: The migration for UserOwnedTemplate is missing the foreign
key for clonedFormId, so add the missing constraint in the same migration
alongside the existing userId and templateId FKs. Update the migration SQL to
create a FOREIGN KEY on "clonedFormId" that references "forms"("id") with ON
DELETE SET NULL (and matching update behavior if needed), using the
UserOwnedTemplate relation defined in the Prisma schema to locate the correct
column and table names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 777e7406-e099-491c-977e-e3974f73f320
📒 Files selected for processing (5)
apps/api/src/controllers/template.controller.tspackages/api-tests/bunfig.tomlpackages/api-tests/integration-tests/modules/template.test.tspackages/api-tests/integration-tests/setup/db.tspackages/db/prisma/migrations/20260630090000_add_template_community_tables/migration.sql
📜 Review details
🧰 Additional context used
🪛 SQLFluff (4.2.2)
packages/db/prisma/migrations/20260630090000_add_template_community_tables/migration.sql
[error] 29-29: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 32-32: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 38-38: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 41-41: CREATE INDEX should use CONCURRENTLY to avoid locking the table during the build.
(PG01)
[error] 45-45: ADD CONSTRAINT ... FOREIGN KEY should use NOT VALID to avoid locking the table while validating existing rows.
(PG01)
[error] 49-49: ADD CONSTRAINT ... FOREIGN KEY should use NOT VALID to avoid locking the table while validating existing rows.
(PG01)
[error] 53-53: ADD CONSTRAINT ... FOREIGN KEY should use NOT VALID to avoid locking the table while validating existing rows.
(PG01)
[error] 57-57: ADD CONSTRAINT ... FOREIGN KEY should use NOT VALID to avoid locking the table while validating existing rows.
(PG01)
🪛 Squawk (2.59.0)
packages/db/prisma/migrations/20260630090000_add_template_community_tables/migration.sql
[warning] 4-4: Using 32-bit integer fields can result in hitting the max int limit. Use 64-bit integer values instead to prevent hitting this limit.
(prefer-bigint-over-int)
[warning] 8-8: When Postgres stores a datetime in a timestamp field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time. Use timestamptz instead of timestamp for your column type.
(prefer-timestamp-tz)
[warning] 9-9: When Postgres stores a datetime in a timestamp field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time. Use timestamptz instead of timestamp for your column type.
(prefer-timestamp-tz)
[warning] 20-20: When Postgres stores a datetime in a timestamp field, Postgres drops the UTC offset. This means 2019-10-11 21:11:24+02 and 2019-10-11 21:11:24-06 will both be stored as 2019-10-11 21:11:24 in the database, even though they are eight hours apart in time. Use timestamptz instead of timestamp for your column type.
(prefer-timestamp-tz)
🔇 Additional comments (5)
apps/api/src/controllers/template.controller.ts (1)
323-323: LGTM!Also applies to: 335-341
packages/api-tests/bunfig.toml (1)
3-3: LGTM!packages/api-tests/integration-tests/setup/db.ts (1)
9-10: LGTM! The deletion order is correct —templateReviewanduserOwnedTemplateare removed before their parenttemplateanduserrecords.packages/api-tests/integration-tests/modules/template.test.ts (2)
89-115: LGTM! The test suite is well-structured with comprehensive coverage of auth, validation, purchase, and review flows. Good use of unique emails viarandomUUID()for test isolation and direct Prisma queries to verify persisted state.Also applies to: 143-247, 253-401, 407-563
270-278: 🩺 Stability & AvailabilityEnsure the missing-template IDs match the actual
Template.idformat. Both 404 cases usenonexistent-id; ifTemplate.idis UUID-typed, a malformed value can fail before the 404 branch and surface as a 500 instead.
- test: add field comparison assertion to snapshot test to ensure fields are actually cloned - db: add missing foreign key for clonedFormId in user_owned_templates migration
Also @silky-x0 and @yashraj639 is doing testing without env or running server in background. |
|
@Basharkhan7776 To clarify our testing setup: we are currently injecting the Additionally, regarding the server: our integration tests don't require manually starting the API server in the background. Our test Also in |
|
@yashraj639 do confirm if you're also using the same! |
Yes same setup on my end. Docker Postgres locally, DATABASE_URL injected inline in the terminal, and the bootstrap spins up the server programmatically so no manual server start needed. Works cleanly. |
The tables (template_reviews and user_owned_templates) were already in the schema.prisma file, but the actual SQL migration file was missing from the repo (likely because prisma db push was used previously instead of prisma migrate dev) |
…actices - Reverted bunfig.toml to no longer load the root .env file automatically. - Tests will now rely on inline environment variables (e.g. DATABASE_URL) as per team convention.
I originally added the .env mapping just as a quick convenience so I didn't have to manually paste the DATABASE_URL inline every time I ran bun test. To be honest, I completely forgot to check the test-onboarding.md file which already outlined the team's standard local testing setup! That's my bad for missing it. |
|
@David-2610 mapping Also Did You ran checked test cases passes successfully and DB URL is injected via |
Yes everything was working fine |
|
Chill just I am asking for the view points of the team. Sorry I you feel uncomfortable. |
[Test] Template Community Routes — PR Summary
Closes: #55
Type:
testOverview
Implements the integration test suite for the Template Marketplace described in Issue #55. This PR adds comprehensive coverage for all required community template routes along with the supporting infrastructure required for reliable test execution.
Changes Made
Integration Tests
Added integration tests for:
POST /api/v1/templatesfieldsandformIdGET /api/v1/templates/communityPOST /api/v1/templates/:id/purchaseuseCountincrementPOST /api/v1/templates/:id/reviewsSupporting Changes
Test Results