search

sensedeep / dynamodb-onetable

DynamoDB access and management for one table designs with NodeJS
https://doc.onetable.io/
MIT License
690 stars 109 forks source link

params.fields are not respected when using table.getItem() or table.queryItems() #498

Closed ThijsJung closed 1 year ago

ThijsJung commented 1 year ago

More details can be found in the issue I filed (#497). I gave it a go and tried to scratch my own itch, let me know what you think. Please feel free to tell me if I forgot anything or if you think there's a better way to fix this issue. Thanks!

mobsense commented 1 year ago

Thank you for contributing this. Looks pretty good. I'll give it a deeper look over the next few days.

One thing: keep debug.ts unchanged as it is the file for users to modify and submit issues. Create a new TS file if you need.

Thank you!

ThijsJung commented 1 year ago

Thanks for your fast reply! Sure, take your time, and let me know if you see anything that you'd like to see changed. Now that I think about it, I'm not 100% certain to updated all the test that are applicable, so if you know of another methods that accept fields that I missed, let me know.

I'll take a moment to revert debug.ts and will put mine in a gist instead.

Thanks!

ThijsJung commented 1 year ago

By the way, while working on this I ran into a couple of things that are not broken but that might clean things up a bit (1) or make development (IMO) a bit more convenient (2). Let me know if you're interested in a P.R. for these (kinds of) items, I'd be happy to contribute.

Cheers!

  1. When instantiating a genericModel we pass it both the name (_Generic) and an option called (generic: true). This can be simplified, I don't think we need the option.
  2. While developing I didn't manage to run a test separately (in for instance, in the crud tests. This seems to be because the user is created in a separate test which means that that test has to always be run. I also found that if do limit the amount of fields returned, some of my later tests would break because they expected the fields to be present on the user (for instance, the id).
mobsense commented 1 year ago

Hi,

Since there are conflict, I've cherry picked your mods and made them manually instead of asking you to (again) update your PR.

Thank you for submitting.

Changes are now comitted and will be released in 2.7.0

ThijsJung commented 1 year ago

Perfect, thanks for the update. I'll keep an eye out for the new release. I was happy to help out, keep up the great work!