fix: page router validations and add test cases #720

karrui · 2024-10-02T07:22:14Z

This PR starts the chain of PRs for adding test cases for all our procedures in the Studio application so we can be more confident when shipping code.

Solution

Breaking Changes

No - this PR is backwards compatible

Features:

Details ...

Improvements:

Add Tagged types to jsonb columns so we remember to call jsonb() function before insertion into the database. This prevents runtime errors.
Update updatePageBlobSchema to validate the type of content in the schema itself.
Update all *select constants to use satisfies keyword for better type inference when used.
Throw 404 errors when resource is not found in all page.router procedures
Validate whether folderId is a folder when calling createPage procedure and passing the folderId arg.
use transaction when updating page in updatePage procedure
add foreign key violation error handling

Bug Fixes:

use correct fullPage.publishedVersionId check in readPageAndBlob procedure

Tests

Tests are added for all procedures in the page router.

A few tests regarding permissions are skipped until permissions are in cc @seaerchin

vercel · 2024-10-02T07:22:15Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
isomer-studio	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 11, 2024 9:52am

karrui · 2024-10-02T07:22:29Z

feat: add more validation to resource.router and add test cases #725 : 2 dependent PRs (#767 , #768 )
fix: page router validations and add test cases #720 👈
feat: replace PGLite with Testcontainers for tests #722
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @karrui and the rest of your teammates on Graphite

datadog-opengovsg · 2024-10-02T10:21:43Z

Datadog Report

Branch report: page_router_tests
Commit report: 19ee86d
Test service: isomer-studio

✅ 0 Failed, 35 Passed, 1 Skipped, 22.48s Total Time

harishv7 · 2024-10-03T12:26:46Z

apps/studio/src/schemas/page.ts

+    }
+    ctx.addIssue({
+      code: z.ZodIssueCode.custom,
+      message: "Invalid page content",


we are trying to abort early right, do we need the FATAL: true flag?

it's ok, this will still cause the validation to fail. FATAL just means zod stops validating immediately. No big deal for it to gather all validation errors before returning to the user

harishv7 · 2024-10-03T12:31:56Z

apps/studio/src/server/modules/site/site.service.ts

@@ -38,7 +38,7 @@ export const setSiteConfig = async (
 ) => {
  return db
    .updateTable("Site")
-    .set({ config })
+    .set({ config: jsonb(config) })


previously when we didn't cast it as jsonb it still worked right? how does the cast help now?

It still worked because of luck. we did not trigger any ambiguity on the pg engine. In more complex shapes, postgres may throw. So this is insurance.

so correct errors are returned

karrui · 2024-10-11T09:55:21Z

Merge activity

Oct 11, 5:55 AM EDT: A user started a stack merge that includes this pull request via Graphite.
Oct 11, 6:00 AM EDT: Graphite couldn't merge this PR because it failed for an unknown reason.
Oct 11, 6:07 AM EDT: A user started a stack merge that includes this pull request via Graphite.
Oct 11, 6:07 AM EDT: A user merged this pull request with Graphite.

vercel bot had a problem deploying to Preview October 2, 2024 07:22 Failure

karrui temporarily deployed to staging October 2, 2024 07:22 — with GitHub Actions Inactive

karrui temporarily deployed to staging October 2, 2024 07:58 — with GitHub Actions Inactive

vercel bot deployed to Preview October 2, 2024 08:02 View deployment

karrui temporarily deployed to staging October 2, 2024 08:31 — with GitHub Actions Inactive

vercel bot deployed to Preview October 2, 2024 08:34 View deployment

karrui changed the title ~~refactor: use vite-tsconfig-paths for vitest config~~ wip: update test pipelines Oct 2, 2024

karrui force-pushed the page_router_tests branch from c3b1c28 to b8363e8 Compare October 2, 2024 10:18

karrui changed the base branch from main to migrate_to_testcontainers October 2, 2024 10:18

karrui temporarily deployed to staging October 2, 2024 10:18 — with GitHub Actions Inactive

karrui mentioned this pull request Oct 2, 2024

feat: replace PGLite with Testcontainers for tests #722

Merged

1 task

vercel bot deployed to Preview October 2, 2024 10:24 View deployment

karrui force-pushed the page_router_tests branch from b8363e8 to b61cd8d Compare October 3, 2024 04:26

vercel bot deployed to Preview October 3, 2024 04:26 View deployment

karrui temporarily deployed to staging October 3, 2024 04:26 — with GitHub Actions Inactive

karrui force-pushed the page_router_tests branch from b61cd8d to 1a57ee8 Compare October 3, 2024 04:27

vercel bot deployed to Preview October 3, 2024 04:27 View deployment

karrui temporarily deployed to staging October 3, 2024 04:28 — with GitHub Actions Inactive

vercel bot deployed to Preview October 3, 2024 07:36 View deployment

karrui temporarily deployed to staging October 3, 2024 07:36 — with GitHub Actions Inactive

karrui changed the title ~~wip: update test pipelines~~ fix: page router validations and add test cases Oct 3, 2024

karrui marked this pull request as ready for review October 3, 2024 08:37

karrui requested a review from a team as a code owner October 3, 2024 08:37

vercel bot deployed to Preview October 3, 2024 08:38 View deployment

karrui temporarily deployed to staging October 3, 2024 08:38 — with GitHub Actions Inactive

harishv7 reviewed Oct 3, 2024

View reviewed changes

karrui added 15 commits October 11, 2024 15:26

fix: lint

e374acd

test: add tests for updatePageBlob

e9539e9

feat: use transaction when updating page blob

cd5d2ef

fix: remove it.only

ce5206a

feat: change updateBlobById to only allow Transaction

2cfde9b

feat(updateBlobById): return 404 if resource not found

72c4cf5

feat: add folderId validation and fkey violation error for createPage

082fa8c

test: add test cases for createPage

9667fbf

feat: add 404 error handling for getRootPage, test cases

b5d80a2

feat: update try-catch when parsing settings meta

bb6c26a

so correct errors are returned

feat: add tests for page.updateSettings

6ec7ed7

feat: guard against invalid resource when fetching permalink

99a4afe

test: add test cases for getFullPermalink

5480183

test: add test cases for getPermalinkTree

3ef1598

fix: use tx instead of db when inside a tx

c768018

karrui force-pushed the migrate_to_testcontainers branch from 7815411 to fa38573 Compare October 11, 2024 07:29

karrui force-pushed the page_router_tests branch from f69ee9e to c768018 Compare October 11, 2024 07:29

vercel bot deployed to Preview October 11, 2024 07:29 View deployment

This was referenced Oct 11, 2024

feat(folder.router): improve validation, add integration tests #767

Draft

test(wip): add resource service unit tests and refactor helpers #768

Draft

feat: update test cases due to merge, add test case for root page

09673fe

vercel bot had a problem deploying to Preview October 11, 2024 09:38 Failure

fix: types

215adc2

vercel bot deployed to Preview October 11, 2024 09:48 View deployment

Base automatically changed from migrate_to_testcontainers to main October 11, 2024 09:55

karrui enabled auto-merge (squash) October 11, 2024 10:05

karrui disabled auto-merge October 11, 2024 10:07

karrui merged commit 5569c85 into main Oct 11, 2024
15 checks passed

karrui deleted the page_router_tests branch October 11, 2024 10:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: page router validations and add test cases #720

fix: page router validations and add test cases #720

karrui commented Oct 2, 2024 •

edited

Loading

vercel bot commented Oct 2, 2024 •

edited

Loading

karrui commented Oct 2, 2024 •

edited

Loading

datadog-opengovsg bot commented Oct 2, 2024 •

edited

Loading

harishv7 Oct 3, 2024

karrui Oct 4, 2024

harishv7 Oct 3, 2024

karrui Oct 4, 2024

karrui commented Oct 11, 2024 •

edited

Loading

fix: page router validations and add test cases #720

fix: page router validations and add test cases #720

Conversation

karrui commented Oct 2, 2024 • edited Loading

Solution

Tests

vercel bot commented Oct 2, 2024 • edited Loading

karrui commented Oct 2, 2024 • edited Loading

datadog-opengovsg bot commented Oct 2, 2024 • edited Loading

Datadog Report

harishv7 Oct 3, 2024

Choose a reason for hiding this comment

karrui Oct 4, 2024

Choose a reason for hiding this comment

harishv7 Oct 3, 2024

Choose a reason for hiding this comment

karrui Oct 4, 2024

Choose a reason for hiding this comment

karrui commented Oct 11, 2024 • edited Loading

Merge activity

karrui commented Oct 2, 2024 •

edited

Loading

vercel bot commented Oct 2, 2024 •

edited

Loading

karrui commented Oct 2, 2024 •

edited

Loading

datadog-opengovsg bot commented Oct 2, 2024 •

edited

Loading

karrui commented Oct 11, 2024 •

edited

Loading