api: Fix search by name#6032
Conversation
Pearl1594
requested review from
DaanHoogland,
nvazquez,
sureshanaparti and
weizhouapache
|
@blueorangutan package |
|
@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2691 |
|
@blueorangutan test |
|
@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
| } | ||
| @Override | ||
| public List<ProjectRoleVO> findByName(String name, Long projectId) { | ||
| public List<ProjectRoleVO> findByName(String name, Long projectId, String keyword) { |
There was a problem hiding this comment.
this looks to accept both name and keyword and search for name adhering to both. Is that intended?
There was a problem hiding this comment.
Name would take precedence @DaanHoogland , so it's effectively not going to use the keyword passes. I can actually remove it.
There was a problem hiding this comment.
@DaanHoogland
yes, in most list apis, keyword is acceptable with other parameters like name, type, etc.
for this particular api, I personally think it is not necessary to accept keyword
but, it is ok to me.
| sc.setParameters("project_id", projectId); | ||
| } | ||
| if (StringUtils.isNotEmpty(keyword)) { | ||
| SearchCriteria<ProjectRoleVO> ssc = createSearchCriteria(); |
There was a problem hiding this comment.
@Pearl1594
why not use
sc.setParameters("keyword", keyword);
|
Trillian test result (tid-3416)
|
yadvr
left a comment
yadvr
left a comment
There was a problem hiding this comment.
Lgtm, we already have keyword param for fuzzy search
6 participants
Description
This PR aims at restricting using name field in the list APIs to do exact matching as the APIs have a keyword field too. This is an extension of #5782 - specifically addressing the following comment: #5782 (comment)
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?