Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

validateUniquess optional prismaClient parameter #5763

Merged
merged 11 commits into from
Jun 23, 2022
Merged

validateUniquess optional prismaClient parameter #5763

merged 11 commits into from
Jun 23, 2022

Conversation

AlejandroFrias
Copy link
Contributor

@AlejandroFrias AlejandroFrias commented Jun 14, 2022 •
edited
Loading

Discord thread started: https://discord.com/channels/679514959968993311/716252919875240007/984849006511394816

Per the prisma docs, you can programmatically override the datasource's url when instantiating a new PrismaClient.

uniquenessValidator util is creating its own PrismaClient to wrap the transaction in, which will not respect any overrides you are setting in the api/src/lib/db.ts file. This is fine when you haven't customized anything, so seems ok for a default. This PR is adding an option to allow passing in your own PrismaClient to use instead in the event you are customizing it.

image

@netlify
Copy link

netlify bot commented Jun 14, 2022 •
edited
Loading

✅ Deploy Preview for redwoodjs-docs ready!

Name Link
🔨 Latest commit 5919716
🔍 Latest deploy log https://app.netlify.com/sites/redwoodjs-docs/deploys/62b350de4496b600089e616c
😎 Deploy Preview https://deploy-preview-5763--redwoodjs-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@dthyresson dthyresson requested a review from cannikin June 14, 2022 19:40
@dthyresson dthyresson added the release:fix This PR is a fix label Jun 14, 2022
cannikin
cannikin requested changes Jun 14, 2022
View reviewed changes
Copy link
Member

@cannikin cannikin left a comment

Choose a reason for hiding this comment

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

We should probably update the docs as well to let people know this is now an option!

packages/api/src/validations/validations.ts Outdated Show resolved Hide resolved
AlejandroFrias
AlejandroFrias commented Jun 15, 2022
View reviewed changes
Copy link
Contributor Author

@AlejandroFrias AlejandroFrias left a comment

Choose a reason for hiding this comment

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

Sounds good. I'll take a crack at documentation updates now and change the name

AlejandroFrias
AlejandroFrias commented Jun 15, 2022
View reviewed changes
Copy link
Contributor Author

@AlejandroFrias AlejandroFrias left a comment

Choose a reason for hiding this comment

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

Though, the way it's implemented there's definitely only support for a Prisma Client, right? It explicitly is using the $transaction prop. I think this particular help is specific to a Prisma. The docs even say it's an experimental feature for Prisma already.

@cannikin
Copy link
Member

Yeah it's only Prisma for now, but you never know! Plus we use db everywhere else.

@AlejandroFrias
Copy link
Contributor Author

@cannikin , Do you all have preferences for commit histories? Are you squashing and committing or would you like me to rebase and squash my commits separately so you can merge?

@cannikin
Copy link
Member

Do you all have preferences for commit histories? Are you squashing and committing or would you like me to rebase and squash my commits separately so you can merge?

You can just leave everything as individual commits, we do a squash and merge when we approve the PR!

alejandro-myriad and others added 3 commits June 16, 2022 12:10
@alejandro-myriad
example usage of db option
dbc85c7
@alejandro-myriad
Merge branch 'AF-validateUniques_optional_prismaClient_parameter' of …
3535b4e 
…github.com:LeftLaneSoftware/redwood into AF-validateUniques_optional_prismaClient_parameter
@AlejandroFrias
Merge branch 'redwoodjs:main' into AF-validateUniques_optional_prisma…
dbed796 
…Client_parameter
@AlejandroFrias
Copy link
Contributor Author

What is the failing "Only doc changes?" check about? Am I not updating something for the docs change?

@AlejandroFrias
Copy link
Contributor Author

LGTY @cannikin ? I think this is ready to merge

@alejandro-myriad
tested docs
04a52e3
@alejandro-myriad
Merge branch 'AF-validateUniques_optional_prismaClient_parameter' of …
7d68cf8 
…github.com:LeftLaneSoftware/redwood into AF-validateUniques_optional_prismaClient_parameter
@alejandro-myriad
docs fine tuning
d8f87df
@cannikin
Copy link
Member

@AlejandroFrias Added a comment to that one code block, wondering if now that message is optional, what happens if no message is provided? Is there a fallback generic "The value in email is already taken"?

@cannikin
Copy link
Member

@alejandro-myriad Can pull down the latest main and merge into your branch? GitHub is telling me it's out of date and won't let me merge!

@cannikin cannikin merged commit 9ce4efa into redwoodjs:main Jun 23, 2022
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Jun 23, 2022
@cannikin
Copy link
Member

Nevermind, tests passed!

dac09 added a commit to dac09/redwood that referenced this pull request Jun 23, 2022
@dac09
Merge branch 'main' of github.com:redwoodjs/redwood into feat/ts-stri…
1c89c0a 
…ctmode-gen

* 'main' of github.com:redwoodjs/redwood:
  validateUniquess optional prismaClient parameter (redwoodjs#5763)
  fix(deps): update dependency prettier to v2.7.1 (redwoodjs#5808)
  fix(deps): update dependency eslint to v8.18.0 (redwoodjs#5806)
  fix(deps): update dependency systeminformation to v5.11.21 (redwoodjs#5805)
  fix(deps): update dependency @apollo/client to v3.6.9 (redwoodjs#5804)
  chore(deps): update dependency firebase to v9.8.3 (redwoodjs#5799)
  Add Azure AD B2C auth provider compatibility (redwoodjs#5781)
  chore(deps): update dependency esbuild to v0.14.47 (redwoodjs#5798)
  Maps JSON GraphQL Scalars to Prisma Json field types for compatibility (redwoodjs#5796)
  fix(deps): update prisma monorepo to v3.15.2 (redwoodjs#5789)
  fix(deps): update dependency core-js to v3.23.2 (redwoodjs#5790)
  fix(deps): update dependency webpack to v5.73.0 (redwoodjs#5755)
  docs: update disable api layer/database to include disabling prisma (redwoodjs#5528)
  Fix typo in testing docs (redwoodjs#5782)
  fix typo (redwoodjs#5777)
  docs: Replacing Prisma.xxx types with types from 'types/graphql' (redwoodjs#5740)
  Reorganize auth docs into sub-categories (redwoodjs#5787)
dac09 added a commit that referenced this pull request Jun 27, 2022
@dac09
Merge branch 'main' of github.com:redwoodjs/redwood into ds-add-githu…
0078622 
…b-issue-forms

* 'main' of github.com:redwoodjs/redwood: (40 commits)
  Update link to auth/providers implementations (#5826)
  fix(deps): update dependency qs to v6.11.0 (#5836)
  Docs -> Tutorial - Update comment-form.md - Minor Typo (#5837)
  fix: have user config for `addons` and `stories` take precedence (#5780)
  chore(deps): update dependency @tsconfig/docusaurus to v1.0.6 (#5834)
  chore(deps): update dependency @auth0/auth0-spa-js to v1.22.1 (#5831)
  Fix misnamed forbidden page route (#5832)
  fix(deps): update dependency concurrently to v7.2.2 (#5830)
  Router tests: More advanced auth mock (#5742)
  fix(deps): update dependency react-hook-form to v7.33.0 (#5809)
  fix(deps): update dependency qs to v6.10.5 (#5829)
  fix some grammar issues (#5778)
  fix(deps): update dependency ci-info to v3.3.2 (#5828)
  validateUniquess optional prismaClient parameter (#5763)
  fix(deps): update dependency prettier to v2.7.1 (#5808)
  fix(deps): update dependency eslint to v8.18.0 (#5806)
  fix(deps): update dependency systeminformation to v5.11.21 (#5805)
  fix(deps): update dependency @apollo/client to v3.6.9 (#5804)
  chore(deps): update dependency firebase to v9.8.3 (#5799)
  Add Azure AD B2C auth provider compatibility (#5781)
  ...
@jtoar jtoar modified the milestones: next-release, v2.1.0 Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cannikin cannikin cannikin approved these changes

Assignees

@cannikin cannikin

Labels
release:fix This PR is a fix
Projects
Main
Status: Archived
Milestone
v2.1.0
Development

Successfully merging this pull request may close these issues.
5 participants
@AlejandroFrias @cannikin @dthyresson @jtoar @alejandro-myriad