-
Notifications
You must be signed in to change notification settings - Fork 45
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
User select in UI feature #1385
Conversation
app/api/chemotion/admin_user_api.rb
Outdated
desc 'Find top (5) matched user by name and by type' | ||
params do | ||
requires :name, type: String, desc: 'user name' | ||
optional :type, type: Array[String], desc: 'user types', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/RedundantArrayConstructor: Remove the redundant Array
constructor.
app/api/chemotion/user_api.rb
Outdated
@@ -5,14 +5,14 @@ class UserAPI < Grape::API | |||
desc 'Find top 3 matched user names' | |||
params do | |||
requires :name, type: String | |||
optional :type, type: Array[String], desc: 'user types', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/RedundantArrayConstructor: Remove the redundant Array
constructor.
it 'returns a response with an array of people' do | ||
expect(parsed_json_response['users']).to include(person_obj) | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RSpec/EmptyLineAfterExampleGroup: Add an empty line after describe
.
adapt corresponding non admin endpoint to use user simple entity
remove deprecated fetcher
df27243
to
a8c3fec
Compare
LCOV of commit
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me except for one maybe bug. Lets have a talk about it.
7a2d5bf
to
0c09a07
Compare
LCOV of commit
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
@@ -186,7 +186,7 @@ export default class AdminFetcher { | |||
} | |||
|
|||
static fetchUsersByNameType(name, type, limit = 5) { | |||
return fetch(`/api/v1/admin_user/listUsers/byname.json?type=${type}&name=${name}&limit=${limit}`, { | |||
return fetch(`/api/v1/admin_user/listUsers/byname.json?${new URLSearchParams({ name, type, limit })}`, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, if naming the variable as the parameter makes it may more easier
* remove duplicated admin api endpoint also adapt equivalent non admin endpoint to use user simple entity * extract react select option formatting to utility function * remove deprecated fetcher
* remove duplicated admin api endpoint also adapt equivalent non admin endpoint to use user simple entity * extract react select option formatting to utility function * remove deprecated fetcher
* remove duplicated admin api endpoint also adapt equivalent non admin endpoint to use user simple entity * extract react select option formatting to utility function * remove deprecated fetcher
Unify user-find-by-name api endpoint:
resolve #1384