Skip to content
New issue

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

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

Already on GitHub? # to your account

[BUGFIX] Fix unique validation in Postgres #19

Merged
merged 5 commits into from
Jun 11, 2020

Conversation

willselby
Copy link
Contributor

@willselby willselby commented Jun 10, 2020

Unique validation fails on subscriber creation when using Postgres because $this->subscriber is an empty string.

https://laracatch.com/share/d145e815-16ba-4f60-9d90-98f19c35c65a

@MaizerGomes
Copy link
Contributor

I ran into the same problem, but using the Rule class fixed it.

Rule::unique('subscribers', 'email')
->ignore($this->subscriber, 'id')
->where(function ($query) {
$query->where('workspace_id', auth()->user()->currentWorkspace()->id);
})

@joshuafranks
Copy link
Contributor

joshuafranks commented Jun 10, 2020

Thanks for your input @MaizerGomes, let me chat with @willselby tomorrow AM and we'll decide which approach to go with. Both seem perfectly valid, though yours is a little more verbose. Either way, I think the logic should be extracted to its own protected method within the SubscriberRequest class. I think this reduces visual debt and aids readability.

@MaizerGomes
Copy link
Contributor

Thanks for your input @MaizerGomes, let me chat with @willselby tomorrow AM and we'll decide which approach to go with. Both seem perfectly valid, though yours is a little more verbose. Either way, I think the logic should be extracted to its own protected method within the SubscriberRequest class. I think this reduces visual debt and aids readability.

You're welcome.

Well, I think using the Rule class adds more readability and since it has been referenced in the official laravel documentation since version 5.3 I think is easier to understand what's going on if something goes wrong.

By the way, I was trying to figure out where the property "subscriber" in the request comes from.
When using the update method in the SubscriberController it gets filled with the id of the subscriber event though no "subscriber" value is being sent by the submit form.
Is this some laravel magic behind the scenes that I don't know about?
Am I missing something?

Joshua Franks added 2 commits June 11, 2020 11:59
@joshuafranks
Copy link
Contributor

joshuafranks commented Jun 11, 2020

Well, I think using the Rule class adds more readability and since it has been referenced in the official laravel documentation since version 5.3 I think is easier to understand what's going on if something goes wrong.

Yes, I agree. The Rule class definitely makes things a little clearer and I think we should stick to the established Laravel conventions where we can. We'll go with that approach. Thanks for your input!

By the way, I was trying to figure out where the property "subscriber" in the request comes from.
When using the update method in the SubscriberController it gets filled with the id of the subscriber event though no "subscriber" value is being sent by the submit form.
Is this some laravel magic behind the scenes that I don't know about?
Am I missing something?

There's a little bit of Laravel magic going on behind the scenes here.

If you take a look in routes/web.php, you'll see that we declare a resourceful subscriber route:

$appRouter->resource('subscribers', 'Subscribers\SubscribersController');

This single declaration will generate multiple routes. Because we specify subscribers as our resource name, Laravel will generate the URIs around that. /subscribers/{subscriber} is the URI that gets generated for the PUT/PATCH update endpoint.

When we're in a FormRequest class, $this refers to our request object. You'll see that the base Request class contains the following magic method:

public function __get($key)
{
    return Arr::get($this->all(), $key, function () use ($key) {
        return $this->route($key);
    });
}

When we do $this->subscriber, the magic method is invoked. First, Laravel will check the request data array to see if a value exists under the 'subscriber' key. If no such value is found, we resort to the closure to obtain a default. Inside the closure, we call:

$this->route('subscriber')
public function route($param = null, $default = null)
{
    $route = call_user_func($this->getRouteResolver());

    if (is_null($route) || is_null($param)) {
        return $route;
    }

    return $route->parameter($param, $default);
}

As you can see, this method will attempt to resolve the route that is attached to our request. If a route is resolved, it'll check for any route parameters that match the given key. In our case, we're looking for a parameter called subscriber. As we established earlier, our URI does have a subscriber parameter (/subscribers/{subscriber}), so whatever value is passed in that segment of the URI is what will be returned.

If you want to get a better visualisation of this, try doing dd($this) in the SubscriberRequest class and take a look at the routeResolver 👍

tl;dr
subscriber is a route parameter and the value is pulled from there.

@joshuafranks joshuafranks requested a review from dljfield June 11, 2020 12:00
@joshuafranks joshuafranks merged commit b48cd8b into master Jun 11, 2020
@joshuafranks joshuafranks deleted the bugfix/postgres-unique-validation branch June 11, 2020 12:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants