Use zod for validation on create endpoint #1391

Victor-M-Giraldo · 2024-02-24T22:26:46Z

related to #1339

coderabbitai · 2024-02-24T22:26:51Z

Important

Auto Review Skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit-tests for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit tests for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit tests.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
The JSON schema for the configuration file is available here.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Victor-M-Giraldo · 2024-02-24T22:28:52Z

I think this works. Zod can also handle validation on the lengths and stuff and there is a library called zod-validation-error which can give nicer errors from what I was reading.

Let me know if you want any changes/if I missed anything.

edit: Also, the checks are repeated since I didn't know which you would prefer.

dyc3

Ideally, we gut all the manual validation we are doing and replace it all with zod.

How does this make sure that createRoomSchema stays in sync with the type definition OttApiRequestRoomCreate? Failing to do so will cause problems in the future.

And make the PR title more descriptive.

server/api/room.ts

Victor-M-Giraldo · 2024-02-24T23:33:39Z

I'll move as much of the validation as I can to Zod. So OttApiRequestRoomCreate is like a schema for the request body in the create endpoint? I can see why the two being out of sync would be a problem but I'm not really sure how to account for that. I'll try and find a fix, but I might need more help if I can't.

dyc3 · 2024-02-24T23:40:04Z

I remember seeing a zod.infer somewhere in the docs.

It might be a good idea to move the zod schemas to ott-common instead, depending on how badly it impacts the bundle size. Vitest also has a thing to test types that would also work.

Victor-M-Giraldo · 2024-02-24T23:49:23Z

zod.infer returns an equivalent type definition from the defined schema. That sounds like what you want I think.

If you find it necessary I can move the zod schemas to ott-common, just give me the confirmation.

I will also take a look at Vitest testing types thing.

dyc3 · 2024-02-24T23:55:47Z

Sounds good, but I want to see how badly it affects the bundle size when you build ott-client because that affects site performance.

Victor-M-Giraldo · 2024-02-24T23:57:04Z

Is there any difference in functionality if OttApiRequestRoomCreate is a type rather than an interface? z.infer gives you the type, but is it functionally the same as an interface?

dyc3 · 2024-02-25T02:21:42Z

Seems like they are basically the same thing. https://stackoverflow.com/questions/37233735/interfaces-vs-types-in-typescript

Victor-M-Giraldo · 2024-02-25T02:39:46Z

Adding anything to common/models/rest-api.ts makes yarn lint fail, but that is where I'm trying to replace the interface with the type.

Victor-M-Giraldo · 2024-02-25T03:18:20Z

It was because zod got removed from package.json when the master branch was merged I think. Hopefully it works now. I also broke some of the tests so I'm working on that now.

Victor-M-Giraldo · 2024-02-29T00:04:50Z

Little stumped on why the error handler isn't catch the zod error. Also, even when I wrap the schema parsing in a try catch, the request is succeeding in the tests and I can't for the life of me find out why.

If you could push me in the right direction/not straight up give me the answer that would be great.

server/api/room.ts

…aldo/opentogethertube into fix-message-parsing

Victor-M-Giraldo · 2024-02-29T20:51:20Z

Couldn't import reserved room names into zod-schemas.ts because of lint errors so I just defined them there and got rid of the old ones.
I got rid of the error in the async function since we only want to check the name, but now whenever I run the tests I get a bunch of weird output.

codecov · 2024-02-29T20:54:15Z

Codecov Report

Attention: Patch coverage is 54.54545% with 20 lines in your changes are missing coverage. Please review.

Project coverage is 55.9188%. Comparing base (99c463a) to head (cd4ad07).
Report is 52 commits behind head on master.

Files	Patch %	Lines
common/models/zod-schemas.ts	0.0000%	19 Missing and 1 partial ⚠️

Additional details and impacted files

@@               Coverage Diff                @@
##             master      #1391        +/-   ##
================================================
+ Coverage   54.1619%   55.9188%   +1.7569%     
================================================
  Files           158        162         +4     
  Lines         24652      24743        +91     
  Branches       1437       1416        -21     
================================================
+ Hits          13352      13836       +484     
+ Misses        11243      10853       -390     
+ Partials         57         54         -3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dyc3

Looks good. Just a couple of easy fixes.

common/models/zod-schemas.ts

server/api/room.ts

common/models/zod-schemas.ts

server/package.json

Victor-M-Giraldo · 2024-03-06T16:17:05Z

Why do description and queueMode need to be in this schema? They weren't in the interface for the create endpoint.

dyc3 · 2024-03-06T16:27:52Z

They weren't in there because the type wasn't complete. They need to be in there because those are all the fields that are shown in the UI when creating a permanent room. You will find this to be the case for most of the rest api types.

server/api/room.ts

Victor-M-Giraldo · 2024-03-06T16:30:20Z

Ah okay, makes sense.

Forgot to remove those, will do that now.

cypress · 2024-03-06T17:53:33Z

Passing run #1218 ↗︎

0	75	1	0	0

Details:

Use zod for validation on create endpoint (#1391)
Project: OpenTogetherTube	Commit: `1137ce74ed`
Status: Passed	Duration: 03:07 💡
Started: Mar 6, 2024 5:50 PM	Ended: Mar 6, 2024 5:53 PM

Review all test suite changes for PR #1391 ↗︎

Victor added 2 commits February 24, 2024 16:16

add @liveinstantly/dash-mpd-parser and zod dependencies

eb7c88a

add createRoomSchema for room creation validation

0cd384e

Merge branch 'master' into fix-message-parsing

98719b5

dyc3 requested changes Feb 24, 2024

View reviewed changes

server/api/room.ts Outdated Show resolved Hide resolved

Victor-M-Giraldo changed the title ~~fix message parsing~~ Use zod for validation on create endpoint Feb 24, 2024

Victor Giraldo added 3 commits February 24, 2024 21:32

use zod validations in createRoom rather than manual checks.

45a1ade

actually get rid of duplicate checks

ffebe51

try to use infer

6c050a4

add zod back to package.json

f36aed9

Victor Giraldo added 8 commits February 24, 2024 22:22

fix lint

4c6db88

add zod-validation-error dependency

8365646

use z.infer for OttApiRequestRoomCreate

5c24cf1

move zod schema to ott common and fix lint

d6a88cf

fix import

2bd0606

add error messages

8bea606

added zoderror handling to errorhandler

0766c16

add zod error to error handler

e1b46a7

dyc3 reviewed Feb 29, 2024

View reviewed changes

server/api/room.ts Outdated Show resolved Hide resolved

dyc3 reviewed Feb 29, 2024

View reviewed changes

server/api/room.ts Outdated Show resolved Hide resolved

Victor Giraldo added 5 commits February 28, 2024 23:15

fix error message

45aaad4

Merge branch 'fix-message-parsing' of https://github.com/Victor-M-Gir…

c60dada

…aldo/opentogethertube into fix-message-parsing

updated zod schema to account for regex and reserved room names.

3985d45

removed duplicate checks.

fad7b41

update unit tests in create endpoint

dbc1457

Victor-M-Giraldo requested a review from dyc3 February 29, 2024 21:01

Victor-M-Giraldo marked this pull request as ready for review March 2, 2024 02:08

dyc3 requested changes Mar 6, 2024

View reviewed changes

common/models/zod-schemas.ts Outdated Show resolved Hide resolved

server/api/room.ts Outdated Show resolved Hide resolved

server/api/room.ts Outdated Show resolved Hide resolved

common/models/zod-schemas.ts Outdated Show resolved Hide resolved

server/package.json Show resolved Hide resolved

Victor Giraldo added 2 commits March 6, 2024 11:10

update zod schema

bfd98c6

add zod dependency to ott-common

c0a4796

dyc3 requested changes Mar 6, 2024

View reviewed changes

server/api/room.ts Outdated Show resolved Hide resolved

server/api/room.ts Outdated Show resolved Hide resolved

remove duplicate checks

cd4ad07

dyc3 approved these changes Mar 6, 2024

View reviewed changes

dyc3 merged commit 1137ce7 into dyc3:master Mar 6, 2024
20 checks passed

Victor-M-Giraldo deleted the fix-message-parsing branch March 21, 2024 19:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use zod for validation on create endpoint #1391

Use zod for validation on create endpoint #1391

Victor-M-Giraldo commented Feb 24, 2024 •

edited by dyc3

Loading

coderabbitai bot commented Feb 24, 2024 •

edited

Loading

Auto Review Skipped

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

CodeRabbit Discord Community

Victor-M-Giraldo commented Feb 24, 2024 •

edited

Loading

dyc3 left a comment •

edited

Loading

Victor-M-Giraldo commented Feb 24, 2024

dyc3 commented Feb 24, 2024

Victor-M-Giraldo commented Feb 24, 2024

dyc3 commented Feb 24, 2024

Victor-M-Giraldo commented Feb 24, 2024

dyc3 commented Feb 25, 2024

Victor-M-Giraldo commented Feb 25, 2024 •

edited

Loading

Victor-M-Giraldo commented Feb 25, 2024 •

edited

Loading

Victor-M-Giraldo commented Feb 29, 2024

Victor-M-Giraldo commented Feb 29, 2024

codecov bot commented Feb 29, 2024 •

edited

Loading

dyc3 left a comment

Victor-M-Giraldo commented Mar 6, 2024

dyc3 commented Mar 6, 2024 •

edited

Loading

Victor-M-Giraldo commented Mar 6, 2024

cypress bot commented Mar 6, 2024

Use zod for validation on create endpoint #1391

Use zod for validation on create endpoint #1391

Conversation

Victor-M-Giraldo commented Feb 24, 2024 • edited by dyc3 Loading

coderabbitai bot commented Feb 24, 2024 • edited Loading

Auto Review Skipped

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

CodeRabbit Discord Community

Victor-M-Giraldo commented Feb 24, 2024 • edited Loading

dyc3 left a comment • edited Loading

Choose a reason for hiding this comment

Victor-M-Giraldo commented Feb 24, 2024

dyc3 commented Feb 24, 2024

Victor-M-Giraldo commented Feb 24, 2024

dyc3 commented Feb 24, 2024

Victor-M-Giraldo commented Feb 24, 2024

dyc3 commented Feb 25, 2024

Victor-M-Giraldo commented Feb 25, 2024 • edited Loading

Victor-M-Giraldo commented Feb 25, 2024 • edited Loading

Victor-M-Giraldo commented Feb 29, 2024

Victor-M-Giraldo commented Feb 29, 2024

codecov bot commented Feb 29, 2024 • edited Loading

Codecov Report

dyc3 left a comment

Choose a reason for hiding this comment

Victor-M-Giraldo commented Mar 6, 2024

dyc3 commented Mar 6, 2024 • edited Loading

Victor-M-Giraldo commented Mar 6, 2024

cypress bot commented Mar 6, 2024

Passing run #1218 ↗︎

Review all test suite changes for PR #1391 ↗︎

Victor-M-Giraldo commented Feb 24, 2024 •

edited by dyc3

Loading

coderabbitai bot commented Feb 24, 2024 •

edited

Loading

CodeRabbit Configration File (`.coderabbit.yaml`)

Victor-M-Giraldo commented Feb 24, 2024 •

edited

Loading

dyc3 left a comment •

edited

Loading

Victor-M-Giraldo commented Feb 25, 2024 •

edited

Loading

Victor-M-Giraldo commented Feb 25, 2024 •

edited

Loading

codecov bot commented Feb 29, 2024 •

edited

Loading

dyc3 commented Mar 6, 2024 •

edited

Loading