search

dereuromark / cakephp-hashid

CakePHP plugin to use hashids for your database table lookups
MIT License
36 stars 13 forks source link

existsHashed() for exists() #14

Closed dereuromark closed 6 years ago

dereuromark commented 7 years ago

https://github.com/dereuromark/cakephp-hashid/issues/13

or is there a better way to provide this? Would be nice if the core used ->select($this->_table->primaryKey()).

dereuromark commented 7 years ago

I wonder if we should make it

'exists' => 'existsHashed',

so it will be autousing the hashed version here from the behavior when called?

dereuromark commented 7 years ago

@chopstik What do you think?

chopstik commented 7 years ago

Apologies for the radio silence on this issue. Appreciate you coming back to it. My only concern would be extra coding required to implement existsHashed() instead of overriding exists(). In our situation we use exists() everywhere - well did, we're bringing an app from v2 -> v3 - and found using the Exceptions thrown on ->get() better/quicker than ->exists(). We switched Hashid off until we found a solution, but if we left Hashid behaviour on fulltime we'd need to replace all the exists() instances. Unless I'm misunderstanding your commit?

dereuromark commented 7 years ago

Well, with the above mapping I think it is possible to drop-in the behavior and it will automatically use exists(), right? Then I agree, it would be nice than having to change code everywhere. Just wanted to confirm that it works with 'exists' => 'existsHashed', then I would change the code before merge and release.

chopstik commented 7 years ago

Great. I'll pull and test and get back to you Monday if that's OK?

dereuromark commented 6 years ago

@chopstik ping

dereuromark commented 6 years ago

Hm from what I can see the behavior is now fixed, I can not find an issue with current CakePHP core and the master branch. exists() works fine..