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

Search IN(...) Clause doest not permit multiple values #16041

Closed
SjamonDaal opened this issue Mar 25, 2020 · 9 comments
Closed

Search IN(...) Clause doest not permit multiple values #16041

SjamonDaal opened this issue Mar 25, 2020 · 9 comments
Labels
Bug A problem or regression with an existing feature has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete
Projects
Milestone

Comments

@SjamonDaal
Copy link

Describe the bug

Not sure but it seems to be the same issue as in #10105. (Which has been resolved in 2015)

To Reproduce

Steps to reproduce the behavior:

  1. Go to my database and select a table
  2. Click on Search
  3. In my Column "ID" I select the Operator "IN(...)"
  4. In my Column "ID" I enter the value "1,2,3"
  5. And the error shows "Please enter a valid number."

Expected behavior

No error, hit search and get the result.

Screenshots

Screenshot 2020-03-25 at 10 53 41

Server configuration

  • Operating system: Ubuntu 18.04
  • Web server: Apache2
  • Database version: 5.7.29
  • PHP version: 7.4.4
  • phpMyAdmin version: 5.0.2

Client configuration

  • Browser: Google Chrome
  • Operating system: MacOS

Additional context

n/a

@williamdes
Copy link
Member

Thank you for the report, a recent change introduced it..

@williamdes williamdes added the Bug A problem or regression with an existing feature label Mar 25, 2020
@williamdes williamdes added this to Needs triage in issues via automation Mar 25, 2020
@williamdes williamdes moved this from Needs triage to Medium priority in issues Mar 25, 2020
@williamdes williamdes added this to the 5.0.3 milestone Mar 25, 2020
@anksh1997
Copy link
Contributor

I am pretty sure that this e92e51d commit introduced it.

Moreover I noticed

var fieldsValue = $('select#fieldID_' + searchIndex);

This select#filedID_ doesn't exists

I changed select#fieldID_ to input#fieldID_ because we have to check existence of that input
(please correct me if i am wrong)

My thoughts on solutions:

  • For verifyAfterSearchFieldChange we can change INT datatype to some other datatyp for multiple values if we have (we can do this in changeValueFieldType
    or
  • In verifyAfterSearchFieldChange we can bypass/add new verification for inputs having multiple prop

@williamdes
Copy link
Member

In verifyAfterSearchFieldChange we can bypass/add new verification for inputs having multiple prop

Seems to be the best solution

@anksh1997
Copy link
Contributor

Hi @williamdes,

I have fixed the issue but need a code review whenever you will get time (I haven't sent PR).

anksh1997@c2d4dba

the branch name is 'fix_multiple_not_working`

  • i have added one validator method for validating Multiple Int with regex (don't know where to put it)
  • also There was a problem with min and max attribute of that input#filedID_

I don't know if this is the optimised solution

@williamdes
Copy link
Member

Hi @asprazz

The mentioned commit seems interesting but some of the fix could be done on the back-end side
Also be sure to use QA_5_0 branch as base to make the fix

@anksh1997
Copy link
Contributor

anksh1997 commented Apr 13, 2020

Yeah I am targeting QA5_0

I am little bit confused
Please, guide me through this.

Edit: QA_5_0

@williamdes
Copy link
Member

@asprazz I think #15942 can help you manage any GIT issues
For the fix just create a branch from the latest upstream QA_5_0 and commit your fix to open a pull-request

Does that seem okay ?

@anksh1997
Copy link
Contributor

@asprazz I think #15942 can help you manage any GIT issues
For the fix just create a branch from the latest upstream QA_5_0 and commit your fix to open a pull-request

Does that seem okay ?

That was helpful

But I was talking about backend changes, I am confused about it.

@williamdes
Copy link
Member

@asprazz I think #15942 can help you manage any GIT issues
For the fix just create a branch from the latest upstream QA_5_0 and commit your fix to open a pull-request
Does that seem okay ?

That was helpful

But I was talking about backend changes, I am confused about it.

Maybe you can open a pull-request so se we can discuss about potential changes :)

@williamdes williamdes added the has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete label Apr 14, 2020
williamdes added a commit that referenced this issue Apr 18, 2020
…lues

Pull-request: #16077
Fixes: #16041

Signed-off-by: William Desportes <williamdes@wdes.fr>
williamdes added a commit that referenced this issue Apr 18, 2020
Signed-off-by: William Desportes <williamdes@wdes.fr>
issues automation moved this from Medium priority to Closed Apr 18, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A problem or regression with an existing feature has-pr An issue that has a pull request pending that may fix this issue. The pull request may be incomplete
Projects
issues
  
Closed
Development

No branches or pull requests

3 participants