Blitzapps / blitz-orm

GNU Affero General Public License v3.0
23 stars 9 forks source link

Draft: Feat-multiVal #71

Closed lveillard closed 3 months ago

lveillard commented 3 months ago
:rocket: This description was created by Ellipsis for commit 251b359a57617731492cd8f6487d1c8266f9c13e

Summary:

This PR introduces multi-value field handling in queries for TypeDB and SurrealDB, with comprehensive updates across scripts, configurations, and tests.

Key points:


Generated with :heart: by ellipsis.dev

sweep-ai[bot] commented 3 months ago

Sweep: PR Review

Author: lveillard

Sweep Found These Issues

src/types/schema/fields.ts
  • Sweep has identified a redundant function: While fn? provides a unique boolean type check not found in the other code, it is an extremely simple utility that could be inlined anywhere a boolean check is needed without loss of functionality. It does not seem critical to keep as a standalone function.
  • https://github.com/Blitzapps/blitz-orm/blob/8756d10406b1cd1fb774b25f028ed9a0e0ce21ff/src%2Ftypes%2Fschema%2Ffields.ts#L47 [View Diff](https://github.com/Blitzapps/blitz-orm/pull/71/files#diff-5954f16d473709529c8c39b76c9ca370f8cc5317ed1a870e8e1e7de0755f48abR47)

Potential Issues

Sweep is unsure if these are issues, but they might be worth checking out.

src/stateMachine/query/tql/parse.ts
  • The code does not handle the case where multiValKey does not exist or is not an array, which could cause errors.
  • https://github.com/Blitzapps/blitz-orm/blob/8756d10406b1cd1fb774b25f028ed9a0e0ce21ff/src%2FstateMachine%2Fquery%2Ftql%2Fparse.ts#L97-L101 [View Diff](https://github.com/Blitzapps/blitz-orm/pull/71/files#diff-deb9d79ade443bc733aa99efde9f8dd99556309790962bed17f2d3904db3953dR97-R101)
tests/typedb/mocks/schema.tql
  • The removal and reintroduction of Color·name and Color·value attributes might affect existing queries or operations that rely on these attributes being defined at the top level.
  • https://github.com/Blitzapps/blitz-orm/blob/8756d10406b1cd1fb774b25f028ed9a0e0ce21ff/tests%2Ftypedb%2Fmocks%2Fschema.tql#L13-L15 [View Diff](https://github.com/Blitzapps/blitz-orm/pull/71/files#diff-5f8125ba7758d3de3d11fb69877b4641176d2dc3646a8626d8c07dca790abc60R13-R15)

Summary

This pull request introduced support for MultiValues queries and updated the testing framework to conditionally ignore certain todos based on the database being tested. The main feature was implemented in `src/stateMachine/query/tql/build.ts` by adding logic to handle fields with a `contentType` of 'FLEX'. The `processDataFields` function was updated to identify these fields and generate the necessary query lines to fetch their attributes. In `src/stateMachine/query/tql/parse.ts`, the `parseFields` function was modified to process and replace multi-value keys in the object, ensuring that the correct values are assigned. A new type `MultiField` was added to `src/types/schema/fields.ts`, allowing fields with flexible content types to be defined and validated. This type was also included in the `AllDataField` union type to enhance versatility. The `tests/mocks/schema.ts` and `tests/surrealdb/mocks/schema.surql` files were updated to include a new `freeForAll` field with a `FLEX` content type in the `Color` entity, and corresponding data entries were added in `tests/surrealdb/mocks/data.surql`. Several test cases were added and updated in `tests/unit/queries/query.ts` to verify the functionality of MultiValues queries, including tests for querying and filtering by multi-value fields. Minor changes included reformatting code for readability and updating test descriptions to include `{TS}` tags for categorization.

The following files were not reviewed because our filter identified them as typically non-human-readable (auto-generated) or less important files (e.g., dist files, package.json, images). If this is an error, please let us know.

ellipsis-dev[bot] commented 3 months ago

Sorry, I don't know how to help with that. If you tag me (@ellipsis-dev) in a comment, I can do one of these things (but not multiple at once):

lveillard commented 3 months ago

@ellipsis-dev, can you review and summarize this pull request

ellipsis-dev[bot] commented 3 months ago

OK! Reviewing this PR...


Responding to this comment by @lveillard. For more information about Ellipsis, check the documentation.

lveillard commented 3 months ago

@ellipsis-dev summarize this PR

ellipsis-dev[bot] commented 3 months ago

OK! Reviewing this PR...


Responding to this comment by @lveillard. For more information about Ellipsis, check the documentation.

lveillard commented 3 months ago

@ellipsis-dev this PR introduced a bug: cannot read properties of undefined (reading thing)

Where in this PR we introduced a .thing that might fail?