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

feat: generate id field using uuid #191

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rodrigodosanjosoliveira
Copy link
Contributor

Fixes #158

Proposed Changes

  1. Generate the id field as a string
  2. Using the uuidV4 package

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request
  • Remember to check if code coverage decrease, if so, please implement the necessary tests

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

@codecov
Copy link

codecov bot commented Jul 4, 2023

Codecov Report

Merging #191 (d137937) into main (1099e97) will not change coverage.
The diff coverage is n/a.

❗ Current head d137937 differs from pull request most recent head 6fd4b18. Consider uploading reports for the commit 6fd4b18 to get more accurate results

@@           Coverage Diff           @@
##             main     #191   +/-   ##
=======================================
  Coverage   68.75%   68.75%           
=======================================
  Files          28       28           
  Lines         560      560           
=======================================
  Hits          385      385           
  Misses        175      175           
Impacted Files Coverage Δ
src/generators/src/packagejson.js 100.00% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -54,7 +54,8 @@ module.exports =
'"nodemon": "^2.0.19"',
'"mocha": "^10.0.0"',
'"lodash.camelcase": "^4.3.0"',
'"sugar-env": "^1.5.14"'
'"sugar-env": "^1.5.14"',
'"uuid": "^9.0.0"'
Copy link
Member

Choose a reason for hiding this comment

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

const dependency = { <%- props.name.pascalCase %>Repository }

const create<%- props.name.pascalCase %> = injection =>
usecase('Create <%- props.name.pascalCase %>', {
// Input/Request metadata and validation
// Input/Request metadata and validation
Copy link
Member

Choose a reason for hiding this comment

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

Let's try to avoid unnecessary merges (spaces, tabs, etc)

<% if(!props.mongo){ %>ctx.<%- props.name.camelCase %>.id = Math.floor(Math.random() * 100000).toString()<% } %>

if (!ctx.<%- props.name.camelCase %>.isValid())
ctx.<%- props.name.camelCase %>.id = uuid.v4()
Copy link
Member

Choose a reason for hiding this comment

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

it seems a mongo if was removed. is it working with mongo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Worked perfectly

@dalssoft
Copy link
Member

dalssoft commented Jul 4, 2023

Thanks @rodrigodosanjosoliveira for the contribution

@dalssoft
Copy link
Member

dalssoft commented Jul 4, 2023

On the migrations you would need something like this:
table.uuid('id').primary().defaultTo(knex.raw('uuid_generate_v4()')) (Postgress)
where this line could change for each DB supported (PG, SQL Server, MySQL, etc)

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.

Uses ObjectID on id field for all databases choices
2 participants