Skip to content
This repository was archived by the owner on Mar 13, 2025. It is now read-only.

skill & skillsProvider removal #126

Open
wants to merge 5 commits into
base: feature/removing_skill_model
Choose a base branch
from
Open

skill & skillsProvider removal #126

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8991850
skill & skillsProvider removal
Sande3p Oct 5, 2021
68089aa
fix #127
xxcxy Oct 8, 2021
55b93a1
add unit test /users/{userId}/skills #128
xxcxy Oct 11, 2021
caf8700
fix db migration
xxcxy Oct 12, 2021
85aaf18
fix search user by skill name
xxcxy Oct 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fix search user by skill name
  • Loading branch information
@xxcxy
xxcxy committed Oct 20, 2021
commit 85aaf18daf0c2106f2203ccbbe7dd381091a6594
12 changes: 12 additions & 0 deletions src/common/es-helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ const USER_ORGANIZATION = {
}

const USER_FILTER_TO_MODEL = {
skill: {
esDocumentQuery: 'skills.skillId.keyword'
},
achievement: {
name: 'achievement',
isAttribute: false,
Expand Down Expand Up @@ -836,6 +839,7 @@ function hasNonAlphaNumeric (text) {
}

async function setUserSearchClausesToEsQuery (boolClause, keyword) {
Copy link

Choose a reason for hiding this comment

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

@Sande3p I don't see any update in the code where setUserSearchClausesToEsQuery is used. I mean if we have removed searching skills in this method's implementation, we must be doing that where this function is being called, right? Am I missing something here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, I don't think we need to do anything in searchUsers, the keyword be used to match user records. since we remove skill model, we just remove the matching skill name and reserve other matchings.

Copy link

Choose a reason for hiding this comment

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

What I meant is searchUsers method would not support searching by skills name now, if we don't add some other way to provide filtering for user based on skills names. I want to be sure that we are not removing any feature from the calling code. We need to remove skills model but we should not remove any feature from the calling code.

Copy link
Collaborator

Choose a reason for hiding this comment

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

fixed

const skillIds = (await helper.getSkillsByName(keyword)).map(skill => skill.id)
boolClause.should.push({
query_string: {
fields: ['firstName', 'lastName', 'handle'],
Expand All @@ -861,6 +865,14 @@ async function setUserSearchClausesToEsQuery (boolClause, keyword) {
}
})

if (skillIds.length > 0) {
boolClause.should.push({
terms: {
[USER_FILTER_TO_MODEL.skill.esDocumentQuery]: skillIds
}
})
}

boolClause.minimum_should_match = 1
}

Expand Down