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

Fix: Create random users in batches in create-data script #2635

Merged
merged 2 commits into from
Jul 3, 2024

Conversation

mmioana
Copy link
Contributor

@mmioana mmioana commented Jul 1, 2024

Description

Refactored the create-data script in order to minimise failure while creating too many users all at once.
The random users creation is batched in chunks of 3.

Also, added a new method graphqlRequestPreconfigured to enclose the GRAPHQL_URI and API_KEY used for the graphqlRequest

Add character limits for

  • user fields: displayName, bio and location
  • colony fields: displayName and objective.title

Testing

  • Step 1. Run npm ci
  • Step 2. Run npm run dev
  • Step 3. Run node ./scripts/create-data.js --yes
  • Step 4. Check if there are any errors while creating the data

Resolves #2606

Create random users in batches of 3
Use graphqlRequestPreconfigured to enclose the GRAPHQL_URI and API_KEY
Add character limits for some user and colony fields
@mmioana mmioana requested review from a team as code owners July 1, 2024 16:53
bassgeta
bassgeta previously approved these changes Jul 2, 2024
Copy link
Contributor

@bassgeta bassgeta left a comment

Choose a reason for hiding this comment

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

image
Data gets populated for me, script runs fine, so nice job!
Just left a tiny nitpick for the objective description length, since we're already clipping all the texts :)

scripts/create-data.js Show resolved Hide resolved
iamsamgibbs
iamsamgibbs previously approved these changes Jul 2, 2024
Copy link
Contributor

@iamsamgibbs iamsamgibbs left a comment

Choose a reason for hiding this comment

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

Looks good to me! Script runs correctly and users are created.

Screenshot 2024-07-02 at 15 35 54 Screenshot 2024-07-02 at 15 36 10 Screenshot 2024-07-02 at 15 36 58

At some point we might want to update the example text in tempColonyData.js to fit the actual character limits but that is very much a nice to have.

davecreaser
davecreaser previously approved these changes Jul 2, 2024
Copy link
Contributor

@davecreaser davecreaser left a comment

Choose a reason for hiding this comment

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

Good stuff, the script is still working as expected and creating all the users. Approved!
Screenshot 2024-07-02 at 16 52 15
Screenshot 2024-07-02 at 16 58 41

@@ -195,6 +207,9 @@ const getColonyMetadata = /* GraphQL */ `
}
`;

const graphqlRequestPreconfigured = async (queryOrMutation, variables) =>
graphqlRequest(queryOrMutation, variables, GRAPHQL_URI, API_KEY)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Big fan of this. 📈

Add char limit for the colony objective description in create-data script
@mmioana mmioana dismissed stale reviews from davecreaser, iamsamgibbs, and bassgeta via 87b56f6 July 2, 2024 16:44
Copy link
Contributor

@bassgeta bassgeta left a comment

Choose a reason for hiding this comment

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

image
Still works, planex status = populated.
Nice work on this, let's get this one merged!

@mmioana mmioana merged commit d7b3af9 into master Jul 3, 2024
4 of 6 checks passed
@mmioana mmioana deleted the fix/2606-update-create-data-script branch July 3, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants