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 5861 Fix: fix import api always display overwritten after importing saved objects, after this fix, the first time import will display new and after that will display overwritten if import the same objects #5871

Merged
merged 9 commits into from
Feb 16, 2024

Conversation

yujin-emma
Copy link
Contributor

@yujin-emma yujin-emma commented Feb 14, 2024

Description

Adjust the order of checkConflicts, checkConflictForDataSource and checkOriginConflict.
The root cause of issue 5861 is when check conflict with data source, if there is no data source conflict, the object will be added in pendingOverwrites without checking wether the object found or not, which is told by checkConflict results, in errors array

We do not need to mutate the pendingOverwrites in checkConflictForDataSource since checkConflicts already handled

Issues Resolved

Fixes #5861

Screenshot

Testing the changes

bug-fix-5861

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: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Copy link

codecov bot commented Feb 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (26fc902) 66.98% compared to head (489c273) 66.98%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5871      +/-   ##
==========================================
- Coverage   66.98%   66.98%   -0.01%     
==========================================
  Files        3304     3304              
  Lines       63572    63569       -3     
  Branches    10153    10153              
==========================================
- Hits        42585    42582       -3     
  Misses      18520    18520              
  Partials     2467     2467              
Flag Coverage Δ
Linux_1 35.21% <ø> (ø)
Linux_2 55.11% <100.00%> (-0.01%) ⬇️
Linux_3 43.55% <0.00%> (+<0.01%) ⬆️
Linux_4 35.20% <0.00%> (+<0.01%) ⬆️
Windows_1 35.24% <ø> (ø)
Windows_2 55.07% <100.00%> (-0.01%) ⬇️
Windows_3 43.55% <0.00%> (+<0.01%) ⬆️
Windows_4 35.20% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
@Flyingliuhub
Copy link
Member

@yujin-emma can we have better bug title, like as the detail which you are trying to address, not the bug number. Thanks

@yujin-emma yujin-emma changed the title Bug 5861 Bug 5861 Fix: fix import api always display overwritten after importing saved objects, after this fix, the first time import will display new and after that will display overwritten if import the same objects Feb 14, 2024
@@ -38,7 +38,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Discover] Enhanced the data source selector with added sorting functionality ([#5609](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/5609))
- [Multiple Datasource] Add datasource picker component and use it in devtools and tutorial page when multiple datasource is enabled ([#5756](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5756))
- [Multiple Datasource] Add datasource picker to import saved object flyout when multiple data source is enabled ([#5781](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5781))
- [Multiple Datasource] Add interfaces to register add-on authentication method from plug-in module ([#5851](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5851))
- [Multiple Datasource] Add interfaces to register add-on authentication method from plug-in module ([#5851](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5851))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, this change comes from rebasing the main, this change is from this commit: yujin-emma@e08bf30

@yujin-emma
Copy link
Contributor Author

Found a test failure not related to the code change in this PR: #5876

@yujin-emma
Copy link
Contributor Author

Checks on feature branch only failed with the function test failure issue: #5876, already filed another issue with this test, the left failure should be fine with rerun

Signed-off-by: Yu Jin <112784385+yujin-emma@users.noreply.github.com>
@ZilongX ZilongX added the multiple datasource multiple datasource project label Feb 16, 2024
ZilongX
ZilongX previously approved these changes Feb 16, 2024
AMoo-Miki
AMoo-Miki previously approved these changes Feb 16, 2024
Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
@yujin-emma yujin-emma dismissed stale reviews from AMoo-Miki and ZilongX via 489c273 February 16, 2024 21:10
@ZilongX ZilongX merged commit d8aefae into opensearch-project:main Feb 16, 2024
64 of 67 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 16, 2024
…ting saved objects, after this fix, the first time import will display `new` and after that will display `overwritten` if import the same objects (#5871)

* bug fix 5861

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* clean unused parameters in test

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* bug fix 5861

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* clean unused parameters in test

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* fix failed UT

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* update CHANGELOG.md

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* Update changelog message

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

---------

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: Yu Jin <112784385+yujin-emma@users.noreply.github.com>
(cherry picked from commit d8aefae)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
Flyingliuhub pushed a commit that referenced this pull request Feb 20, 2024
…ting saved objects, after this fix, the first time import will display `new` and after that will display `overwritten` if import the same objects (#5871) (#5891)

* bug fix 5861

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* clean unused parameters in test

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* bug fix 5861

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* clean unused parameters in test

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* fix failed UT

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* update CHANGELOG.md

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

* Update changelog message

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>

---------

Signed-off-by: yujin-emma <yujin.emma.work@gmail.com>
Signed-off-by: Yu Jin <112784385+yujin-emma@users.noreply.github.com>
(cherry picked from commit d8aefae)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

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
Projects
None yet
4 participants