Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix custom key routing #30

Merged
merged 4 commits into from
May 26, 2021
Merged

fix custom key routing #30

merged 4 commits into from
May 26, 2021

Conversation

ahmedbally
Copy link
Contributor

@antonkomarev
Copy link
Member

Thank you for the contribution @ahmedbally! It will be great to cover this in tests to not break this feature in the future: https://github.com/cybercog/laravel-optimus/blob/master/tests/Traits/OptimusEncodedRouteKeyTest.php

@ahmedbally
Copy link
Contributor Author

I hope you find it well
I tried my best 👍
if you could re-run tests it will give you success

@antonkomarev
Copy link
Member

Your code is OK, it's scrutenizer config issue. Thank you for your efforts. I will test it and release then.

@antonkomarev
Copy link
Member

I've tested it, this way we have a breaking change. For example when we have our encoded key stored in public_id property. Then we can not bypass it anymore.

This would not work anymore:

Route::get('/posts/{public_id:encodedId}', function (Post $post) {
    return $post;
});

@antonkomarev
Copy link
Member

But since originally we haven't used $field parameter - it is not a case. Nobody can use it this way.

@antonkomarev antonkomarev merged commit 94c9731 into cybercog:master May 26, 2021
@antonkomarev
Copy link
Member

antonkomarev commented May 26, 2021

Merged it, and will do small tweak in #31

public function resolveRouteBinding($value, $field = null)
{
    if ($field === null) {
        $field = $this->getRouteKeyName();
    }

    if (is_string($value) && ctype_digit($value)) {
        $value = (int) $value;
    }

    if (is_int($value) && $field === $this->getRouteKeyName()) {
        $value = $this->getOptimus()->decode($value);
    }

    return $this->where($field, $value)->first();
}

It will allow us to explicitly use route key too:

Route::get('/posts/{id:encodedId}', function (Post $post) {
    return $post;
});

@ahmedbally
Copy link
Contributor Author

this get into my mind when i was taking a shower 😂 😂 😂

@ahmedbally
Copy link
Contributor Author

thank you for your tweaks

@antonkomarev
Copy link
Member

Laravel Optimus v3.7.0 has been released! @ahmedbally you are in contributors list now 🥇

@antonkomarev
Copy link
Member

antonkomarev commented May 26, 2021

If you have a twitter - I could mention you in the release post.

@ahmedbally
Copy link
Contributor Author

I am very happy to be in contributors list
that's my twitter
https://twitter.com/ahmedbally
thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants