Fix for tagging backend #7240
Merged
Fix for tagging backend #7240
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
betodealmeida
added
!deprecated-label:bug
betodealmeida
commented
Apr 8, 2019
| @@ -123,7 +123,7 @@ def _add_owners(cls, session, target): | |||
| tagged_object = TaggedObject( | |||
| tag_id=tag.id, | |||
| object_id=target.id, | |||
| object_type=ObjectTypes.chart, | |||
| object_type=cls.object_type, | |||
Codecov Report
@@ Coverage Diff @@
## lyftga #7240 +/- ##
==========================================
+ Coverage 64.49% 64.53% +0.04%
==========================================
Files 425 425
Lines 20879 20861 -18
Branches 2280 2280
==========================================
- Hits 13466 13463 -3
+ Misses 7287 7272 -15
Partials 126 126
Continue to review full report at Codecov.
|
|
LGTM |
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
This PR has bug fixes for the tagging backend, and a few improvements on the SQLAlchemy queries used by it.
The tags model has a clowny bug 🤡where the type of tags was being set incorrectly in the parent class (see
superset/models/tags.pybelow). This should fix all the weird problems that were happening with tags (new charts starting with tags, and wrong objects showing up in the tag search).For the SQLAlchemy queries, I rewrote them to use explicit joins (to avoid accidents with cross joins), and also made the logic more clear.
TEST PLAN
I created charts and dashboards, tagged them, and verified that everything is working as expected.
ADDITIONAL INFORMATION
REVIEWERS
@xtinec (this might cause merge conflicts with our private cherry, LMK if you need help fixing it)
@khtruong
@datability-io