herbsjs / herbs-cli

Herbs CLI
https://herbsjs.org/
MIT License
29 stars 30 forks source link

feat(update): automatically identify pk and fks into repository layer #150

Closed danielsous closed 1 month ago

danielsous commented 2 years ago

Fixes #26

Proposed Changes

  1. Use the power of the CLI to identify primary keys and foreign keys directly from the entity's implementation.

Readiness Checklist

Author/Contributor

Reviewing Maintainer

codecov[bot] commented 2 years ago

Codecov Report

Merging #150 (e0b4396) into main (56e99b7) will increase coverage by 1.12%. The diff coverage is 100.00%.

:exclamation: Current head e0b4396 differs from pull request most recent head d9e29bc. Consider uploading reports for the commit d9e29bc to get more accurate results

@@            Coverage Diff             @@
##             main     #150      +/-   ##
==========================================
+ Coverage   86.28%   87.41%   +1.12%     
==========================================
  Files          25       25              
  Lines         423      437      +14     
==========================================
+ Hits          365      382      +17     
+ Misses         58       55       -3     
Impacted Files Coverage Δ
...rs/src/infra/database/repositories/repositories.js 97.50% <100.00%> (+1.34%) :arrow_up:
...rators/src/infra/database/migrations/migrations.js 94.73% <0.00%> (+2.63%) :arrow_up:
src/commands/herbs.js 100.00% <0.00%> (+50.00%) :arrow_up:

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

dalssoft commented 2 years ago

@danielsous the current implementation is not handling multiples IDs:

const Project =
    entity('Project', {
        id1: id(String),
        id2: id(String),
        name: field(String),
    })
danielsous commented 2 years ago

@dalssoft, @italojs, @jhomarolo

dalssoft commented 2 years ago

@danielsous great contrib! having support for entities relationship is one of the most important feature for now.

just a few points:

dalssoft commented 2 years ago

I don't think it is necessary to explicitly inform PKs (ids: [) on the repository since it is able to detect ids using metadata.

dalssoft commented 2 years ago

1 to 1 scenario:

Testing with two entities with relationship between then:

const User =
        entity('User', {
          id: id(String),
          nickname: field(String),
        })

const Project =
        entity('Project', {
          id: id(String),
          user: field(User),
        })

The repository (ProjectRepository) was created with foreignKeys: [{ user_id: String ,}] which is correct, but the migration (20220904220920_projects.js) had table.undefined('user') as the field definition, which is a bug.

dalssoft commented 2 years ago

1 to Many scenario:

Testing with two entities with relationship between then:

const User =
        entity('User', {
          id: id(String),
          nickname: field(String),
        })

const Project =
        entity('Project', {
          id: id(String),
          user: field([User]),     // <----- Many users
        })

When herbs update it throws a expection:

Generating Entities
Generating Repositories
  New: C:\Users\David\Projects\herbs-project\src\infra\data\repositories\userRepository.js
C:\Users\David\Projects\herbs-cli\node_modules\gluegun\build\index.js:15
    throw up;
    ^

TypeError: Cannot read properties of undefined (reading 'meta')
    at C:\Users\David\Projects\herbs-cli\src\generators\src\infra\database\repositories\repositories.js:13:51
    at Array.map (<anonymous>)
    at generateForeignKeysField (C:\Users\David\Projects\herbs-cli\src\generators\src\infra\database\repositories\repositories.js:10:25)
    at generateRepositories (C:\Users\David\Projects\herbs-cli\src\generators\src\infra\database\repositories\repositories.js:40:45)
dalssoft commented 2 years ago

@danielsous I think your PR is very close to the effort started by this PR https://github.com/herbsjs/herbs-cli/pull/144 created by @endersoncosta regarding issue https://github.com/herbsjs/herbs-cli/issues/145 . You guys should join forces to finish it. As I said, this feature is veeery import for the project