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

When mocking a cell, set the id value as a String or Number based on the type of primary key #4778

Merged
merged 9 commits into from
Mar 19, 2022

Conversation

dthyresson
Copy link
Contributor

@dthyresson dthyresson commented Mar 16, 2022

Fixes #4531

Prior to this PR, a cell mocked valid was always number: 42 (or 42, 43,44 in lists).

But, this could be problematic if your model id was a String (like uuid or cuids).

This PR detects if the idType is a String and uses a string value -- otherwise numeric.

Note the value is "42" or 42.

In the list to keep the template simple, the ids are 3 of the same value ... that is they do not increment. This avoided more template vars to pass or logic.

For example:

// Define your own mock data here:
export const standard = () => ({
  books: [{ id: 42 }, { id: 42 }, { id: 42 }],
})

or

// Define your own mock data here:
export const standard = () => ({
  room: {
    id: '42',
  },
})

or

// Define your own mock data here:
export const standard = () => ({
  rooms: [{ id: '42' }, { id: '42' }, { id: '42' }],
})

@dthyresson dthyresson added the release:fix This PR is a fix label Mar 16, 2022
@dthyresson dthyresson self-assigned this Mar 16, 2022
@dthyresson dthyresson requested a review from Tobbe March 16, 2022 18:25
@dthyresson dthyresson marked this pull request as ready for review March 16, 2022 18:25
@netlify
Copy link

netlify bot commented Mar 18, 2022

✅ Deploy Preview for redwoodjs-docs canceled.

🔨 Explore the source changes: 32560a1

🔍 Inspect the deploy log: https://app.netlify.com/sites/redwoodjs-docs/deploys/6235bebd2bd3c1000891bceb

@Tobbe
Copy link
Member

Tobbe commented Mar 18, 2022

In the list to keep the template simple, the ids are 3 of the same value ... that is they do not increment. This avoided more template vars to pass or logic.

@dthyresson Unfortunately this is going to be a problem in the cell component where we use the id as a unique key (see key={item.id}):

export const Success = ({ userExamples }: CellSuccessProps<UserExamplesQuery>) => {
  return (
    <ul>
      {userExamples.map((item) => {
        return <li key={item.id}>{JSON.stringify(item)}</li>
      })}
    </ul>
  )
}

@dthyresson
Copy link
Contributor Author

@Tobbe I have updated the generator to include multiple mocked values. And now there are tests that actually check the mock files for both numeric and string values.

@dthyresson dthyresson requested a review from Tobbe March 18, 2022 21:54
@dthyresson dthyresson enabled auto-merge (squash) March 19, 2022 11:30
@dthyresson dthyresson merged commit 1b5d8e3 into redwoodjs:main Mar 19, 2022
@jtoar jtoar added this to the next-release milestone Mar 19, 2022
@thedavidprice thedavidprice modified the milestones: next-release, v0.50.0 Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix This PR is a fix
Projects
No open projects
Status: Archived
Development

Successfully merging this pull request may close these issues.

Generating a Cell backed by a Model with a String Id fails in Storybook because expects an Int
4 participants