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.

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

update scaffold generator to use emptyAsUndefined when its needed #8031

Merged
merged 8 commits into from Apr 10, 2023

Conversation

samthuang
Copy link
Contributor

@samthuang samthuang commented Apr 7, 2023

Update the use of scaffold generator to include emptyAs('undefined') when the FormField component is a relational field and is not required.

resolves: #4354

@replay-io
Copy link

replay-io bot commented Apr 7, 2023

16 replays were recorded for 05f56fd.

image 0 Failed
image 16 Passed
    requireAuth graphql checks
          ```
          locator.waitFor: Target closed
          =========================== logs ===========================
          waiting for locator('.rw-form-error-title').locator('text=You don\'t have permission to do that') to be visible
          ============================================================
          ```
        </ol>
      </details>
      <li><a href=https://app.replay.io/recording/396512f4-3f38-4e46-a92d-35887f749c2a>useAuth hook, auth redirects checks</a></li>
      <li><a href=https://app.replay.io/recording/364eff4f-550d-462f-8949-df45f2d8c524>Check that a specific blog post is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/1bdfff85-ffff-4a14-9bb1-6a20cb5358d3>Check that about is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/91bd29cd-800a-4bdc-9c35-c3cfd1127d9b>Check that homepage is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/18e44ddc-4079-4907-9da9-0c60af4af36e>Check that meta-tags are rendering the correct dynamic data</a></li>
      <li><a href=https://app.replay.io/recording/681d81c1-719a-4403-8e78-7c1dd68ab2dd>Check that you can navigate from home page to specific blog post</a></li>
      <li><a href=https://app.replay.io/recording/16b2db8b-463b-4282-9541-5fcdfc3f9d26>Waterfall prerendering (nested cells)</a></li>
      <li><a href=https://app.replay.io/recording/62dbcc1c-7b2a-4c0d-9824-2f0863f830d7>RBAC: Admin user should be able to delete contacts</a></li>
      <li><a href=https://app.replay.io/recording/41f57e44-78e0-49bf-9411-1aedd89beaf5>RBAC: Should not be able to delete contact as non-admin user</a></li>
      <li><a href=https://app.replay.io/recording/452f40d0-4ef1-48fe-a9d2-a98bcb16bbac>Smoke test with dev server</a></li>
      <li><a href=https://app.replay.io/recording/7fddf791-3ba0-40b5-959e-d5f121022d81>Smoke test with rw serve</a></li>
      <li><a href=https://app.replay.io/recording/c2891041-9c9b-47a7-8a3e-c15d1bc7aacc>Loads Cell mocks when Cell is nested in another story</a></li>
      <li><a href=https://app.replay.io/recording/8aaeb430-9219-4cfd-b5b1-582e6dc9d5ad>Loads Cell Stories</a></li>
      <li><a href=https://app.replay.io/recording/6bd00a19-f4e5-4f20-959f-57a0f0494e6c>Loads MDX Stories</a></li>
      <li><a href=https://app.replay.io/recording/edb640ff-b016-47a9-97f5-282bf9ce782a>Mocks current user, and updates UI while dev server is running</a></li>
      

View test run on Replay ↗︎

@thedavidprice thedavidprice requested a review from Tobbe April 7, 2023 18:52
@thedavidprice
Copy link
Contributor

Thanks @samthuang 🚀

@Tobbe would you be able to help with this one?

@Tobbe
Copy link
Member

Tobbe commented Apr 7, 2023

Thanks for jumping in to help @samthuang! Much appreciated 🙂

Try doing a console.log of the prisma dmmf and you'll see that it has all the info you need to figure out if a field is relational or not.

You can start looking in packages/cli/src/lib/schemaHelpers.js where you'll see that we use getDMMF from @prisma/internals. Following the code execution path you'll see that getSchema() in that schemaHelpers file has the dmmf info, and that function is already called from the scaffold generator.

I hope that info helps you get started. Let me know if you need anymore guidance

In addition to checking the field name ends with 'id', checks that the
field name matches one of the model's relations
@samthuang
Copy link
Contributor Author

Hello, hello, @Tobbe - thank you for pointing me to the right place to look. It's beneficial! I get to learn a few things about how Prisma types its Field on a Model. Super cool.

I believe it's probably best to add a test for this. Can you be so kind and show me where and how to do so? thanks!

@samthuang samthuang marked this pull request as ready for review April 9, 2023 10:59
@Tobbe
Copy link
Member

Tobbe commented Apr 9, 2023

I believe it's probably best to add a test for this.

A test would be great! Take a look at packages/cli/src/commands/generate/scaffold/__tests__/editableColumns.test.js It uses mocks to mock out the filesystem. So you can call the scaffold generator and the files will be generated in memory. You can then verify that emptyAs has been generated as you expect. If you need to you can add a new model to packages/cli/src/commands/generate/scaffold/__tests__/fixtures/schema.prisma, or update an existing model if you find something that's already close to what you need.

@samthuang
Copy link
Contributor Author

Thank you for the swift response ⚡ and the pointers. Please let me know what you think of the test and the approach. cheers

@samthuang samthuang changed the title update use scaffold generator to use emptyAsUndefined when its needed update scaffold generator to use emptyAsUndefined when its needed Apr 9, 2023
@Tobbe
Copy link
Member

Tobbe commented Apr 9, 2023

I'm sorry I wasn't more clear. I didn't mean for you to add the test to editableColumns. I just used that as an example because it was pretty small and isolated. I think it's best to keep that one as it was, just for testing the exclusion of fields with default values. Please see if you can find some other test you can integrate the emptyAs test into. Or create a separate test and model for it, if needed. How you actually test your fix is perfect, just need to be moved somewhere else 🙂

@samthuang
Copy link
Contributor Author

Good point. I took it too literally 😆. I created a separate test. Let me know if the test can be further improved on. 2f90098.

@Tobbe Tobbe added the release:feature This PR introduces a new feature label Apr 10, 2023
@Tobbe Tobbe enabled auto-merge (squash) April 10, 2023 05:18
@Tobbe Tobbe added the fixture-ok Override the test project fixture check label Apr 10, 2023
@Tobbe Tobbe merged commit 2327e8c into redwoodjs:main Apr 10, 2023
30 of 32 checks passed
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Apr 10, 2023
jtoar pushed a commit that referenced this pull request Apr 11, 2023
)

* update use scaffold generator to use emptyAsUndefined when its needed

* identify that a prisma model field is a relational field

In addition to checking the field name ends with 'id', checks that the
field name matches one of the model's relations

* test includes emptyAsUndefined on optional relation form field

* isolate the use emptyAsUndefined test

* Minor tweaks to the test

---------

Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
jtoar pushed a commit that referenced this pull request Apr 11, 2023
)

* update use scaffold generator to use emptyAsUndefined when its needed

* identify that a prisma model field is a relational field

In addition to checking the field name ends with 'id', checks that the
field name matches one of the model's relations

* test includes emptyAsUndefined on optional relation form field

* isolate the use emptyAsUndefined test

* Minor tweaks to the test

---------

Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
@jtoar jtoar modified the milestones: next-release, v4.5.0 Apr 11, 2023
@samthuang samthuang deleted the sth-use-emptyAsUndefined-in-scaffold branch April 18, 2023 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixture-ok Override the test project fixture check release:feature This PR introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make scaffold generators use emptyAsUndefined
4 participants