search

baopham / laravel-dynamodb

Eloquent syntax for DynamoDB
https://packagist.org/packages/baopham/dynamodb
MIT License
490 stars 127 forks source link

Allow KeyConditionExpression when model has composite key and partition key only is passed in where #254

Closed thebatclaudio closed 1 year ago

thebatclaudio commented 2 years ago

Resolve #253

thebatclaudio commented 2 years ago

Hello @baopham, have you got any news about when you will accept this PR? Sorry if I'm asking, but I need this fix on my project and I would like to use your composer package instead of my fork on github.

Thank you

JackPriceBurns commented 2 years ago

It seems like Travis CI has stopped working and isn't running the tests anymore.. I can't seem to get it going again (think it has to be baopham to do that), I'll pull down this code and manually check, but the PR looks good overall 👍

thebatclaudio commented 2 years ago

Hello @JackPriceBurns, thank you. I just pushed another commit

thebatclaudio commented 2 years ago

Ops, I broke something

thebatclaudio commented 2 years ago

Fixed: array_search can return 0, so I have to check if the result is === false (instead of use !array_search(...))

thebatclaudio commented 2 years ago

Hi @baopham and @JackPriceBurns, sorry if I'm asking again, but have you got any news about when you will accept this PR? How I was saying before, I need this fixes on my project and I would like to use your composer package instead of my fork on github (that is a problem for CI).

Thank you

thebatclaudio commented 2 years ago

I fixed another bug: when using aws_iam_role the DYNAMODB_DEBUG environment variable was ignored.

I'm still waiting for the PR. Thank you.

thebatclaudio commented 2 years ago

Hello?

baopham commented 1 year ago

@nelson6e65 I think this looks good. Could you help check? if it looks good to you, will need your help to merge and publish a patch version for this PR.

nelson6e65 commented 1 year ago

I was having trouble on setting up my local environment to test, @baopham. That's why I could not test this locally. (Maybe you can help me this Saturday on this).

Are there any failing test cases for this bug?

baopham commented 1 year ago

I was having trouble on setting up my local environment to test, @baopham. That's why I could not test this locally. (Maybe you can help me this Saturday on this).

Are there any failing test cases for this bug?

@nelson6e65 actually... I no longer work with PHP so I don't have anything setup unfortunately

nelson6e65 commented 1 year ago

@thebatclaudio Thanks for this fix. I think that with those linted changes, this PR will be ready. 😃

scrutinizer-notifier commented 1 year ago

The inspection completed: No new issues

nelson6e65 commented 1 year ago

This is available as v6.2.0 🚀.