-
Notifications
You must be signed in to change notification settings - Fork 916
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
[Workspace] Fix workspace name duplication check #6776
[Workspace] Fix workspace name duplication check #6776
Conversation
Signed-off-by: Hailong Cui <[email protected]>
❌ Changelog Entry Missing HyphenChangelog entries must begin with a hyphen (-). |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6776 +/- ##
=======================================
Coverage 67.55% 67.55%
=======================================
Files 3428 3428
Lines 67340 67340
Branches 10994 10994
=======================================
Hits 45494 45494
Misses 19176 19176
Partials 2670 2670
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
link check error not related to this change, it will fixed by #6771
|
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.
LGTM
@@ -95,7 +95,7 @@ export class WorkspaceClient implements IWorkspaceClientImpl { | |||
const existingWorkspaceRes = await this.getScopedClientWithoutPermission(requestDetail)?.find( | |||
{ | |||
type: WORKSPACE_TYPE, | |||
search: attributes.name, | |||
search: `"${attributes.name}"`, |
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.
does this search for a workspace including this exact phrase or one that is an exact match?
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.
It depends on the field type. For my case, since workspace.name
filed type is keyword, it will convert to TermQuery
that's exact match. To make the PR description more clear, it add an example query with profile enabled.
* fix workspace name duplication check Signed-off-by: Hailong Cui <[email protected]> * Changeset file for PR #6776 created/updated --------- Signed-off-by: Hailong Cui <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com> (cherry picked from commit f74eb24) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* fix workspace name duplication check * Changeset file for PR #6776 created/updated --------- (cherry picked from commit f74eb24) Signed-off-by: Hailong Cui <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
…6776) * fix workspace name duplication check Signed-off-by: Hailong Cui <[email protected]> * Changeset file for PR opensearch-project#6776 created/updated --------- Signed-off-by: Hailong Cui <[email protected]> Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
Description
enclose workspace name with double quotes when doing duplication check, so that it will not interpret by simply query syntax and do match it with exact match (workspace.name field type is
keyword
)It will convert to
TermQuery
that's exact match.Issues Resolved
#6480
Screenshot
Testing the changes
Changelog
Check List
yarn test:jest
yarn test:jest_integration
- [ ] New functionality has been documented.