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

When Pushing a Database, If Pre-entered Database Name Is Changed, Click When OK Clicked Is Ignored #1136

Open
chrisjlocke opened this Issue Sep 13, 2017 · 11 comments

Comments

Projects
None yet
3 participants
@chrisjlocke
Copy link
Contributor

chrisjlocke commented Sep 13, 2017

Details for the issue

When you enter an invalid database name to push to dbhub.io, DB4S tries to identify the branch, but fails due to the invalid URL.

image

The OK button isn't cancelled, so the user can continue. Obviously this doesn't work, but still throws up 'a weird error' (from the users perspective)

image

The same is with the branch name - this currently allows non-URL friendly characters, resulting in the above 'Ow - what are you DOING?!!!' error.

image

Useful extra information

I'm opening this issue because:

  • DB4S is crashing
  • DB4S has a bug
  • DB4S needs a tweak and a cuddle
  • DB4S has another problem

I'm using DB4S on:

  • Windows: ( version: ___ )
  • Linux: ( distro: ___ )
  • Mac OS: ( version: ___ )
  • Other: ___

I'm using DB4S version:

  • 3.10.Nightly
  • 3.10.0-beta*
  • 3.9.1
  • Other: ___

I have also:

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Sep 13, 2017

Hmmm, from the server side that "Internal Server Error" should probably be looked at. That generally means the request is getting further along in the system than it should. Though it's also fairly likely it's just returning a generic error instead of something more appropriate to whatever's happening. I'll check on the server side in a bit, just to make sure it's all ok there.

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Oct 3, 2017

DB4S needs a tweak and a cuddle

Like this? https://www.youtube.com/watch?v=9oiEzmgA5Ek 😄

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Oct 3, 2017

Looked into this on the server side. The invalid values were being caught in the correct place (initial validation checks) server side, but the returned error code was 500 ("Internal server error") instead of 400 ("Bad request"). That part's now fixed in the server side code, and is running on the db4s-beta server.

We'll still need to figure out some way to check the fields in DB4S before sending them to the server.

Maybe have DB4S run the same regular expression checks that the server side does?

MKleusberg added a commit that referenced this issue Oct 5, 2017

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Oct 5, 2017

Thanks, @chrisjlocke, good point 😄 I've just added the regular expressions to the code. Can you maybe do a quick check if I missed anything?

@chrisjlocke

This comment has been minimized.

Copy link
Contributor Author

chrisjlocke commented Oct 6, 2017

If you enter ## in the 'database name' and click to 'Commit message', an error is thrown up (first image in the orignal post).
From a newbie users point of view, this isn't indicative that the name they've given is invalid. Its obviously obvious, and trial and error would confirm that, but it would be nice to remove that bad attempt.
Also, if I type/change the pre-entered database name and click OK, the click is 'eaten' so it doesn't register. I can try and screencast that if you require?

@chrisjlocke

This comment has been minimized.

Copy link
Contributor Author

chrisjlocke commented Oct 6, 2017

Submitted a database and got this 'error'

image

This was using the latest nightly. Upgrade what? The application, or remove the remotedbs.db again?

@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Oct 6, 2017

I think I found out what's causing the upgrade required bug. I'll open a new issue for it now, then investigate the other issues you've mentioned 😄

MKleusberg added a commit that referenced this issue Oct 6, 2017

dbhub: Better validation in push dialog
This makes it more obvious to the user where the error in the input is
by just now allowing invalid characters.

It also prevents an annoying error message from popping up when entering
invalid database names and changing focus to another field afterwards.

See issue #1136.
@MKleusberg

This comment has been minimized.

Copy link
Member

MKleusberg commented Oct 6, 2017

Thanks, I believe the annoying error message is fixed now as well. I've changed it so you just can't enter invalid characters any longer.

The other issue with the 'eaten' click doesn't seem to happen for me. So yeah, maybe the screen recording would help here 😄

@chrisjlocke

This comment has been minimized.

Copy link
Contributor Author

chrisjlocke commented Oct 6, 2017

https://screencast-o-matic.com/watch/cb616VIgZi

Its not entirely obvious, but at 0:06 when the OK button is clicked, the Cancel button gets 'highlighted', and then the OK button is clicked again.
If the database exists (ie, the same name is entered) this does not occur.
If the 'commit message' box gets the focus after entering a name, again this 'click eaten' doesn't occur.

The OK button is again clicked at 0:23 which doesn't register (but the Cancel button gets highlighted)

@justinclift

This comment has been minimized.

Copy link
Member

justinclift commented Oct 11, 2017

As a data point, that "OK button not clickable at first" problem happens for me using Fedora 25 too. I generally get around it by clicking in the branch name field, after which the OK button is clickable.

Haven't looked into it more yet, nor tried on OSX. I'll probably look at it some time this week if it annoys me enough. 😄

@chrisjlocke

This comment has been minimized.

Copy link
Contributor Author

chrisjlocke commented Nov 9, 2017

I'll change the issue title, as the original issue has been fixed, but the 'click doesn't work on first click' is quite annoying, so its best this issue is left open.

@chrisjlocke chrisjlocke changed the title When Pushing Database, Invalid Filenames Can Be Submitted When Pushing a Database, If Pre-entered Database Name Is Changed, Click When OK Clicked Is Ignored Nov 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment