search

dbudwin / RoboHome-Web

RoboHome-Web is the codebase that represents the frontend of the RoboHome project. The web interface provides a way to create users, add and manage devices, and an additional way to control devices. :robot: :house_with_garden:
GNU General Public License v3.0
8 stars 17 forks source link

Disallow duplicated device name for the same user #164

Closed kailichiang closed 1 year ago

kailichiang commented 5 years ago

Description

Disallow and show danger message when

Motivation and Context

Disallow duplicated device name for the same user #162

How Has This Been Tested?

Add a new test case for adding a device with an existing name.

Screenshots (if appropriate):

Types of changes

Checklist:

dbudwin commented 5 years ago

I pulled this code down and played with it. It does work to prevent duplicate device names. However, I think it would be a better user experience if the user was given a chance to correct the device name instead of just seeing an error message. For example, when a user is creating a device named "Test" and they click "Add Device," instead of the modal closing and the flash message being displayed, they should get a validation error within the modal where they can change the device name to something else without losing the rest of their settings.

kailichiang commented 5 years ago

About "better experience," there are several possible solutions.

  1. Use ajax to validate input value before really creating or updating. This is gonna be great user experience. But it needs lots of js code, a new API, and so on.
  2. After backend validation and redirection, a user sees error message. When clicking the same button, the modal shows with old values. This is gonna be the simplest.
  3. After backend validation, a user redirect back with modal opening (if any error) with old values. This needs a logic or mechanism to know why a user back to determine opening modal or not, showing add-modal or edit-modal etc.

Solution 1 and 3 probably needs lots engineering, I'm not sure. You may have more experience.

dbudwin commented 5 years ago

I agree, I think option 1 would have the best user experience. I don't have a ton of JS experience (I try to avoid it as much as I can), but I figured there has to be a JS library out there that could help. Doing some searching I found Parsley. There was also a recent question on StackOverflow trying to solve a problem very similar to this one. The same user as that StackOverflow question also got some guidance when asked on Parsley's GitHub repository too.

In brief, it looks like we would need to use Parsley's Remote Validator to hit a new API endpoint that would return a list of all the device names that a user has. So, if I understand correctly, we shouldn't need much more than a small JS function which uses Parsley and a simple API endpoint. This project already has an API implemented for querying all the devices that a user has for example (see index of /API/DevicesController.php).

Thoughts?

kailichiang commented 5 years ago

That's awesome and look interesting! I can try to do it. But where do you wanna show validation error messages, below the input?

dbudwin commented 5 years ago

I think below the input makes sense. That seems to be the common way to use that library and their examples online often do that. It would also make it easier in the future if I needed more validation on some of those fields.

dbudwin commented 5 years ago

Are you still considering getting client-side validation working for this issue?

kailichiang commented 5 years ago

I’d like to do client-side validation. As you suggested in another PR, before starting, I’m better to delete my original fork and fork again. This PR probably will be ignored. But I’m kinda busy recently, so I don’t start yet.

The client-side validation can be also documented to the original issue, therefore anyone interested is welcome to do this.