[Feature] Enable Unique Checks via Ajax

[OwenMelbz]

summary
Enables fields to be marked as unique, giving the user visual feedback as they type, with an optional hint

preview

usage

$this->crud->addField([
            'label' => 'Product Slug',
            'name' => 'slug',
            'unique' => true, //enables unicity checks when typing
            'unique_hint' => 'slug' //(optional) the name of the field to display for the hint, if empty no hint will show
        ]);

[tabacitu]

I like this, I like this a lot, the interface is perfect, I think.

But since is not a "core" function, I'm wondering if this is the best way to implement this (because there's no going back if we do). Here's what I think the problems are and my proposed solutions:

Problem 1. We have logic from one field type inside general-purpose files
(inc/field_attributes.blade.php and form_content.blade.php)

This would make the CRUD more difficult to understand and overwrite for users that don't use that functionality (which will probably be a majority). I think a better approach to this would be a separate field type, unique_string. That would:

• keep the text field pretty simple, as it is now;
• make us contain all the unique_string logic inside one file (unique_string.blade.php);

Problem 2. CrudController got really really big and includes both core and non-core methods

As Laravel suggests (the blue notice):

I think the best approach for this would be to:

• rename unicity to checkUniqueString();
• separate the checkIfUnique() method inside a trait, something likesrc/PanelFeatureTraits/CheckUniqueString.php;

That would both allow the user to overwrite the method in their EntityCrudController AND separate a non-core functionality from CrudController. I think we'll end up using this procedure a lot, including for already-posted functionalities (like listRevisions() , restoreRevision(), reorder(), saveReorder(), showDetailsRow(), search()). So our src/PanelFeatureTraits folder will look something like:

• AjaxTable.php
• CheckUniqueString.php
• Reorder.php
• Revisions.php
• ShowDetailsRow.php

I think this would not constitute a breaking change if we use the traits by default in CrudController. I don't know if we should keep doing this in v4, though.

Problem 3. We register yet another route for all CRUDs, whether or not it's used
I think we should include the route in CRUD::resource(); only after we have a way to add a "clean" routes list, like we talked about in the SoftDeletes issue, with CRUD::routes('car', 'CarCrudController')->withCheckUniqueString() or CRUD::routes('car', 'CarCrudController')->with(['trash']);;

To sum up and see if this is a good idea, the instructions for the unique_string field would be:

1. Include use CheckUniqueString; on your EntityCrudController;
2. Add your field definition;
3. Add the route for the AJAX check;

What do you think? Does this make for a cleaner framework? I don't think the added steps for this functionality are that much of a drag. Only a few CRUD panels would need this.

[OwenMelbz]

1. My concern with this was the moment I started using it for the main thing I wanted it for, it fell over. I wanted unique emails on "create user" which was using the type => email and by making a whole new field type, I could no longer use it - this is why I opted for a new attribute that can be added to fields which meant all the template files would not need updating. If we had "unique_string" then you lose the ability to check all the other field types like email/number/url etc
2. I've got no problem with abstracting this into a trait :)
3. Sure, I think as things are expanding as you're suggesting using the ->with() chaining will be good, I wonder if we can get this function to setup the ajax endpoint route for the user rather than adding another route in manually?

[tabacitu]

Problem 5. Are there use cases for a more general functionality?
This only checks if the string is unique. Will/would developers need to check for something else with ajax? Because then it would make sense for the functionality to be broader, on more/all field types, and allow them to do their own checks.

We could do that by passing a model method to the field. So you'd add a field where, as the user types, a certain method on the model is checked. That method would either return true or false.

So the field could be:

$this->crud->addField([
            'label' => 'Product Slug',
            'name' => 'slug',
            'type' => 'text',
            'ajaxCheck' => 'slugExists',
            'ajaxCheckErrorMessage' => 'This slug already exists',
        ]);

and it would send an AJAX POST request to admin/product/ajaxCheck/slugExists, which would in turn call that method on the model ($model->slugExists('your-string-here')) and return true/false.

But the question is: are there actual usecases or is it overoptimization?

[OwenMelbz]

Problem 5 eh? Jumping 4? :D

I did actually think about making it more generic so you pass the method/route to check, which would be a nice feature, however I found unique checks were as far as I'd needed to go, with things like usernames, urls, emails, access codes, api keys etc etc.

Maybe if it's implemented in such a way that allows the future possibilities it would be nice, just start with a generic

$this->crud->addField([
            'label' => 'Product Slug',
            'name' => 'slug',
            'type' => 'text',
            'ajaxCheck' => 'unique',
            'ajaxCheckErrorMessage' => 'This slug already exists',
        ]);

which intern hits /controller/ajaxCheck/{{$field['ajaxCheck'}} allowing the user to define a custom one, then we can build in 'unique' as the default

[tabacitu]

[now replying to your 3-point message above]

1. Hmm.. You're right. Maybe the solution I've mentioned above (general ajaxCheck) would be a broad-enough functionality to include in all/most fields then. Still need to figure out if it makes sense making it so general, though. That would also change the javascript functionality a bit, because we can't use the "check" end "edit" icons anymore, since the field can be any type. Or I guess we can place them on the <label>, but... that would potentially be ugly. Anyway, I don't think the icons are a big issue, after all the only important thing you need to communicate there is whether the check fails, which is obvious with the <div class="alert">. Otherwise the user doesn't need to know what's going on in the background (ajax checks).
2. Great.
3. That would be awesome, but I don't think we can achieve that. In Laravel routes are registered long before the request hits the Controller.

Eager to find out what you think about the general approach.

[tabacitu]

:-)) my bad. It was problem 4.

Yes, I agree, unique should come pre-built, because it will probably be the most used. And we can easily do that by having a method inside CrudTrait, because all models already use it:

public function columnValueExists($column, $value) {
   // check that column for that value
   // return true/false
}

I'm not sure this is a good name for it, but I think "unique" might be too common. Then someone might create a "unique()" method on his model and overwrite this one my mistake.

I think this has really really progressed. This has been a very good back-and-forth and the end result might be an excellent new feature. I do think we should let it sink in for a day or so, see if we think differently or come up with a better approach.

I guess pushing features into open-source projects is like unprotected sex: it's very gratifying, but you can't take it back if you don't like the outcome :-))

[OwenMelbz]

1. I think for the time being "string" based checks will be okay, so we can still keep the icon indicators within the text fields, as even things like emails etc will still show the same field, what other fields did you think would be an issue?

3. I can't see the documentation anywhere for ->with() but if thats getting called from the route config, then surely that runs at the same time as setting up the routes? thus being able to register more? Might be worth a try - if not can you just register /admin/ajaxChecks ? then contain more post data?

4. Sure, any thing like that ColumnValueUnique/Exists - I think we'd need to consider what the user is expected to return, for example ColumnValueExists return true/false - then means we need to figure out what that model is and return it to the view for the user to see it in the info box, so we need to make sure its easy enough for users to customise the response to the view

[OwenMelbz]

@tabacitu I've pushed some changes, not entirely sure how to implement the with however I've given it a whirl to see how close I could get! Additionally I'd added the changes you mentioned i think

[OwenMelbz]

@tabacitu this is also helpful :D

https://twitter.com/taylorotwell/status/784467722872631296

[jrpub]

Definitely a must-have feature, especially the broader version with the Model function.
Typical use case: a form with an URL field, with an ajax check to see if the it has been already entered (not a strict equality: http://domain.com/uri == https://domain.com/uri for instance)

Is the merge of this feature still planned?

[lloy0076]

Spacing around ifs and functions are dubious throughout.

[lloy0076]

Documentation.

[lloy0076]

Documentation.

[lloy0076]

Should this weed out the undefineds?

[lloy0076]

Documentation.

[lloy0076]

Documentation.

[lloy0076]

it's

[lloy0076]

spaces

[lloy0076]

Spaces. Remove the new line.

[lloy0076]

Spaces.

[lloy0076]

spaces

[tabacitu]

@OwenMelbz - I think I'm going to close this, unfortunately. It's a cool functionality, but it breaks away from how CRUD does stuff usually, and I think that's a bad thing.

I think we'll come back to this when Create/Edit forms will be AJAX, in the next major version - it will
then make sense for validation to happen as-you-type, since that's how all fields will work. Until then, this field would be too different to be included in the core, and it would require those controllers which again - I think are too niche to be in the core.

I do think this PR brought up some serious architecture issues, though, thank you for that, we should seriously keep it in mind in the next major version.

Cheers!