Fix entity with Json field set to the mock db#565
Conversation
WalkthroughThis update removes explicit index management from the mock database code, replacing index-based lookups with linear scans and simplifying function signatures to use entity configuration records. Additionally, it introduces a new event handler and test that processes and verifies entities with all supported data types in the test scenario. Changes
Sequence Diagram(s)sequenceDiagram
participant EventHandler
participant Context
participant MockDb
EventHandler->>Context: set EntityWithAllTypes (twice)
Context->>MockDb: Store entities in memory
Note over MockDb: No index management, linear scan on retrieval
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
codegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbs(6 hunks)scenarios/test_codegen/src/EventHandlers.ts(1 hunks)scenarios/test_codegen/test/EventHandler_test.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
scenarios/test_codegen/test/EventHandler_test.ts (1)
codegenerator/cli/templates/static/codegen/index.js (1)
BigDecimal(7-7)
scenarios/test_codegen/src/EventHandlers.ts (1)
codegenerator/cli/templates/static/codegen/index.js (1)
BigDecimal(7-7)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build_and_test
🔇 Additional comments (10)
scenarios/test_codegen/src/EventHandlers.ts (2)
424-426: Good change to use explicit case instead of default.Replacing the
defaultcase with an explicitcase "syncRegistration"improves code clarity and makes the expected behavior more predictable.
430-478: Comprehensive test handler for entity types.The new event handler provides excellent coverage for testing all supported data types. The implementation correctly uses
BigDecimalconstructors and covers all field variations (required, optional, and arrays).scenarios/test_codegen/test/EventHandler_test.ts (2)
4-4: Necessary import for BigDecimal usage.The BigDecimal import is correctly added to support the new test case.
107-151: Well-structured test for comprehensive entity handling.The test effectively validates that the event handler correctly creates entities with all supported data types. Good use of the spread operator to verify the second entity and proper type assertions with
satisfies.codegenerator/cli/templates/dynamic/codegen/src/TestHelpers_MockDb.res.hbs (6)
107-120: Simplified delete operation looks correct.The removal of index management code simplifies the delete operation while maintaining the core functionality for the mock database.
122-136: Streamlined set operation.The simplified set operation correctly handles entity updates without the overhead of index maintenance, which is appropriate for a mock database used in testing.
282-292: Appropriate simplification for mock database queries.The switch from index-based lookups to linear scans is a reasonable simplification for the mock database. The filtering logic using
Index.evaluatemaintains consistency with the expected behavior while reducing complexity.
302-303: Clean refactoring of executeRowsEntity.The simplified function signature using
entityConfigimproves code clarity and reduces parameter passing overhead.Also applies to: 307-307, 318-318
337-337: Good optimization to cache table reference.Caching the
mockDbTablereference avoids repeated lookups during iteration, improving performance.Also applies to: 342-342
367-372: Elegant simplification using centralized entity configuration.Replacing individual entity execution calls with a loop over
Config.codegenPersistence.allEntitiesprovides better maintainability and automatically handles all entity types without manual enumeration.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
| //ENTITY EXECUTION | ||
| {{#each entities as | entity |}} | ||
| mockDb->executeRowsEntity( | ||
| ~inMemoryStore, | ||
| ~entityMod=module(Entities.{{entity.name.capitalized}}), | ||
| ~getKey=entity => entity.id, | ||
| ) | ||
| {{/each}} |
There was a problem hiding this comment.
Nice, do we still need a .hbs file?
Summary by CodeRabbit