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.

Sign up for GitHub

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

feat: custom favorite filter for dashboards, charts and saved queries #11083

Merged
merged 13 commits into from
Oct 1, 2020
Merged

feat: custom favorite filter for dashboards, charts and saved queries #11083

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
af0ecd1
feat: custom favorite filter for dashboards
dpgaspar Sep 27, 2020
9f06eb2
Merge branch 'master' into feat/api-favorites
dpgaspar Sep 27, 2020
59733d3
lint and sort
dpgaspar Sep 27, 2020
0f75be8
add favored for charts
dpgaspar Sep 27, 2020
30ef91c
fix tests and lint
dpgaspar Sep 27, 2020
8eb19e9
more tests and saved query filter
dpgaspar Sep 28, 2020
aede712
fix tests
dpgaspar Sep 28, 2020
91cd438
fix tests
dpgaspar Sep 28, 2020
3b7c4d0
lint
dpgaspar Sep 28, 2020
3355761
Merge branch 'master' into feat/api-favorites
dpgaspar Sep 28, 2020
832266c
lint and fix conflict
dpgaspar Sep 28, 2020
f5a84e5
remove unnecessary prop
dpgaspar Sep 29, 2020
2d38a86
separate tests
dpgaspar Oct 1, 2020
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
3 changes: 3 additions & 0 deletions superset/models/sql_lab.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ class SavedQuery(Model, AuditMixinNullable, ExtraJSONMixin):
backref=backref("saved_queries", cascade="all, delete-orphan"),
)

def __repr__(self) -> str:
return self.label

@property
def pop_tab_link(self) -> Markup:
return Markup(
Expand Down
13 changes: 9 additions & 4 deletions tests/queries/saved_queries/api_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
from tests.base_tests import SupersetTestCase


SAVED_QUERIES_FIXTURE_COUNT = 5
SAVED_QUERIES_FIXTURE_COUNT = 10


class TestSavedQueryApi(SupersetTestCase):
Expand Down Expand Up @@ -326,17 +326,22 @@ def test_get_saved_query_favorite_filter(self):
self.login(username="admin")
uri = f"api/v1/saved_query/?q={prison.dumps(arguments)}"
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 200)
data = json.loads(rv.data.decode("utf-8"))
self.assertEqual(data["count"], len(expected_models))
assert rv.status_code == 200
Copy link

Choose a reason for hiding this comment

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

why not use self.assert200(rv)?

Copy link
Member Author

@dpgaspar dpgaspar Oct 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Because it's used nowhere on the test codebase, not written in stone but with pytest now, we are slowly refactoring to use assert

assert len(expected_models) == data["count"]
Copy link

Choose a reason for hiding this comment

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

why did you change it to basic assert?

Copy link
Member Author

Choose a reason for hiding this comment

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

not written in stone but with pytest now, we are slowly refactoring to use assert


for i, expected_model in enumerate(expected_models):
assert expected_model.label == data["result"][i]["label"]

# Test not favorite saves queries
expected_models = (
db.session.query(SavedQuery)
.filter(and_(~SavedQuery.id.in_(users_favorite_query)))
.filter(
and_(
~SavedQuery.id.in_(users_favorite_query),
SavedQuery.created_by == admin,
)
)
.order_by(SavedQuery.label.asc())
.all()
)
Expand Down