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 feature for provisioning instance and virtual machines #22996

Merged
merged 1 commit into from Apr 25, 2024

Conversation

jeffibm
Copy link
Member

@jeffibm jeffibm commented Apr 17, 2024

@Fryguy
Copy link
Member

Fryguy commented Apr 17, 2024

@kbrock Please review

@@ -32,6 +32,10 @@ def self.eligible_for_provisioning
where(:type => subclasses_supporting(:provisioning).map(&:name)).active
end

def self.search(name)
where('vms.name like ?', "%#{name}%")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this breaks when we search for something like - cent%

Started POST "/vm_cloud/vm_pre_prov/new?search_text=cent%&hide_deprecated_templates=true" for ::1 at 2024-04-17 19:40:06 +0530
[----] F, [2024-04-17T19:40:06.866328 #13781:ece0] FATAL -- :   
ActionController::BadRequest (Invalid query parameters: invalid %-encoding (cent%)):

Copy link
Member Author

@jeffibm jeffibm Apr 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Fryguy , I had made a small change here in the ui side which helps to handle the search with % in the string.

@jeffibm jeffibm force-pushed the search-provision-instance branch 3 times, most recently from b62601a to 71899d6 Compare April 18, 2024 05:29
@@ -32,6 +32,10 @@ def self.eligible_for_provisioning
where(:type => subclasses_supporting(:provisioning).map(&:name)).active
end

def self.filter_with_name(name)
where("vms.name ILIKE ?", "%#{name}%")
Copy link
Member

@kbrock kbrock Apr 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry to be that guy...

Can we do:

Suggested change
where("vms.name ILIKE ?", "%#{name}%")
where(arel_table[:name].matches("%#{name}%"))

The last 2 parameters defaulting to ("%#{name}%", nil, false) -- meaning that we want a case insensitive LIKE matching.

see also: https://rubydoc.info/docs/rails/6.0.2.1/Arel/Nodes/Matches

Copy link
Member Author

@jeffibm jeffibm Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @kbrock , I made the change and tested it. looks good..
image

@remo233
Copy link

remo233 commented Apr 19, 2024

Hi @jeffibm ,

I hope the checks are successfully, can you please guide me how to do this in manageiq petrosian ?

@jeffibm
Copy link
Member Author

jeffibm commented Apr 19, 2024

Hi @jeffibm ,

I hope the checks are successfully, can you please guide me how to do this in manageiq petrosian ?

I have no idea how do it now.

However, you will have to wait till these PRs are thoroughly reviewed, merged, and then backported to petrosian. This process usually takes time.

We will notify you once it's done.

@miq-bot
Copy link
Member

miq-bot commented Apr 24, 2024

Checked commit jeffibm@975b91c with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@kbrock kbrock merged commit e3217a6 into ManageIQ:master Apr 25, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants