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

[BUG] Check for duplicate data source id before fetching the data sources in index pattern selector #6498

Merged
merged 4 commits into from
Apr 17, 2024

Conversation

BionIT
Copy link
Collaborator

@BionIT BionIT commented Apr 16, 2024

Description

This change adds check for duplicate data source id before fetching the data sources in index pattern selector

Issues Resolved

Screenshot

duplicate.mp4

Testing the changes

The following steps were performed in the recording above:

  1. revert the change, go to maps plugin, and add a layer, see the bulk get payload which shows duplicate data source id
  2. apply the change, go to maps plugin, and add a layer, see the bulk get payload which shows unique data source id

Changelog

  • skip

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Signed-off-by: Lu Yu <nluyu@amazon.com>
Signed-off-by: Lu Yu <nluyu@amazon.com>
Copy link
Member

@ruanyl ruanyl left a comment

Choose a reason for hiding this comment

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

Approved with a minor comment

Comment on lines +178 to +183
if (
dataSourceReference &&
!this.state.dataSourceIdToTitle.has(dataSourceReference.id) &&
!dataSourceIdSet.has(dataSourceReference.id)
) {
dataSourceIdSet.add(dataSourceReference.id);
Copy link
Member

Choose a reason for hiding this comment

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

I think we can simplify the logic a bit like this, then we don't need to add an extra variable dataSourceIdSet

Suggested change
if (
dataSourceReference &&
!this.state.dataSourceIdToTitle.has(dataSourceReference.id) &&
!dataSourceIdSet.has(dataSourceReference.id)
) {
dataSourceIdSet.add(dataSourceReference.id);
const dataSourcesToFetch = new Set<string>();
savedObjects.map((indexPatternSavedObject: SimpleSavedObject<any>) => {
const dataSourceReference = getDataSourceReference(indexPatternSavedObject.references);
if (dataSourceReference && !this.state.dataSourceIdToTitle.has(dataSourceReference.id)) {
dataSourcesToFetch.add(dataSourceReference.id);
}
});
const dataSourceIdToTitleToUpdate = new Map();
if (dataSourcesToFetch.size > 0) {
const resp = await savedObjectsClient.bulkGet(
[...dataSourcesToFetch].map((id) => ({ type: 'data-source', id }))
);

@BionIT BionIT merged commit d2d410b into opensearch-project:main Apr 17, 2024
72 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 17, 2024
…rces in index pattern selector (#6498)

* remove duplicate entries

Signed-off-by: Lu Yu <nluyu@amazon.com>

* add test

Signed-off-by: Lu Yu <nluyu@amazon.com>

---------

Signed-off-by: Lu Yu <nluyu@amazon.com>
(cherry picked from commit d2d410b)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
zhongnansu pushed a commit that referenced this pull request Apr 17, 2024
…rces in index pattern selector (#6498) (#6508)

* remove duplicate entries



* add test



---------


(cherry picked from commit d2d410b)

Signed-off-by: Lu Yu <nluyu@amazon.com>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
all-star-contributor backport 2.x multiple datasource multiple datasource project Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry v2.14.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants