Skip to content

Commit ce20316

Browse files
committed
ui: fixed displaying rows of the previous query
- When the query of a view changed (filtering the view by a word, paginating, etc) while the scrollbar was not at the top or bottom of the view, the viewport kept displaying the rows of the previous query. Selections and menu actions operated on the new result set, so they didn't match the rows displayed. - Added a regression test for the above. ref: #1291
1 parent b9a6a4b commit ce20316

2 files changed

Lines changed: 58 additions & 12 deletions

File tree

ui/opensnitch/customwidgets/generictableview.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ class GenericTableModel(QStandardItemModel):
3636
prevQueryStr = ''
3737
# modified query object
3838
realQuery = QSqlQuery()
39+
# set when the query changes, to force the next viewport refresh.
40+
# Otherwise the view would keep displaying the rows of the previous
41+
# query when the scrollbar is not at the top/bottom of the view.
42+
forceNextRefresh = False
3943

4044
items = []
4145
lastItems = []
@@ -170,6 +174,7 @@ def setQuery(self, q, db, binds=None, limit=None, offset=None):
170174

171175
if self.prevQueryStr != self.origQueryStr:
172176
self.realQuery = tmpQuery
177+
self.forceNextRefresh = True
173178

174179
self.update_row_count()
175180
self.update_col_count()
@@ -202,6 +207,9 @@ def refreshViewport(self, scrollValue, maxRowsInViewport, force=False):
202207
force var will force a refresh if the scrollbar is at the top or bottom of the
203208
viewport, otherwise skip it to allow rows analyzing without refreshing.
204209
"""
210+
if self.forceNextRefresh:
211+
force = True
212+
self.forceNextRefresh = False
205213
if not force:
206214
return
207215

ui/tests/customwidgets/test_generictableview.py

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,27 +30,26 @@ def rule_name(num):
3030
return "rule-{0:03d}".format(num)
3131

3232

33-
def insert_test_rules(db):
33+
def insert_test_rules(db, names=None):
34+
if names is None:
35+
names = [rule_name(i) for i in range(NUM_RULES)]
3436
db.clean("rules")
35-
for i in range(NUM_RULES):
37+
for i, name in enumerate(names):
38+
rule_time = "2026-06-06 10:{0:02d}:{1:02d}".format(i // 60, i % 60)
3639
db.insert(
3740
"rules",
3841
"(time, node, name, enabled, precedence, action, duration, " \
3942
"operator_type, operator_sensitive, operator_operand, operator_data, " \
4043
"description, nolog, created)",
4144
(
42-
"2026-06-06 10:00:0{0}".format(i), TEST_NODE, rule_name(i), "True",
45+
rule_time, TEST_NODE, name, "True",
4346
"False", "allow", "always", "simple", "False", "process.path",
44-
"/bin/app-{0}".format(i), "", "False", "2026-06-06 10:00:0{0}".format(i)
47+
"/bin/app-{0}".format(i), "", "False", rule_time
4548
)
4649
)
4750

4851

49-
@pytest.fixture
50-
def rules_view(qtbot):
51-
db = Database.instance()
52-
insert_test_rules(db)
53-
52+
def build_rules_view(qtbot):
5453
container = QtWidgets.QWidget()
5554
layout = QtWidgets.QHBoxLayout(container)
5655
view = GenericTableView(container)
@@ -62,15 +61,32 @@ def rules_view(qtbot):
6261
view.setVerticalScrollBar(scrollbar)
6362
view.setTrackingColumn(COL_NAME)
6463
view.setModel(model)
65-
model.setQuery(RULES_QUERY, db.get_db())
64+
model.setQuery(RULES_QUERY, Database.instance().get_db())
6665

6766
qtbot.addWidget(container)
6867
container.resize(600, 400)
6968
container.show()
7069
qtbot.waitExposed(container)
7170
view.refresh()
72-
# yield keeps the fixture frame (and so the container) referenced for
73-
# the duration of the test; qtbot only holds a weak reference
71+
return container, view
72+
73+
74+
@pytest.fixture
75+
def rules_view(qtbot):
76+
insert_test_rules(Database.instance())
77+
container, view = build_rules_view(qtbot)
78+
# the fixture frame keeps the container referenced for the duration
79+
# of the test; qtbot only holds a weak reference
80+
yield view
81+
82+
83+
@pytest.fixture
84+
def mixed_rules_view(qtbot):
85+
"""Two name groups with enough rules to scroll the view."""
86+
names = ["app-{0:03d}".format(i) for i in range(30)]
87+
names += ["term-{0:03d}".format(i) for i in range(30)]
88+
insert_test_rules(Database.instance(), names)
89+
container, view = build_rules_view(qtbot)
7490
yield view
7591

7692

@@ -152,6 +168,28 @@ def test_selected_rows_returns_clicked_rule(rules_view, qtbot):
152168
assert selected_db_rows[0][COL_NAME] == rule_name(1)
153169

154170

171+
def test_query_change_refreshes_viewport_while_scrolled(mixed_rules_view, qtbot):
172+
"""Regression: changing the query (e.g. typing a filter) while the
173+
scrollbar was not at the top or bottom of the view kept displaying the
174+
rows of the previous query, so the visible rows didn't match the data
175+
that selections and menu actions operated on."""
176+
view = mixed_rules_view
177+
model = view.model()
178+
179+
view.vScrollBar.setValue(10)
180+
displayed = [row[COL_NAME] for row in model.items]
181+
assert len(displayed) > 0
182+
assert all(name.startswith("app-") for name in displayed)
183+
184+
filtered_query = "SELECT time, node, name, enabled FROM rules " \
185+
"WHERE name LIKE 'term-%' ORDER BY name ASC"
186+
model.setQuery(filtered_query, Database.instance().get_db())
187+
188+
displayed = [row[COL_NAME] for row in model.items]
189+
assert len(displayed) > 0
190+
assert all(name.startswith("term-") for name in displayed)
191+
192+
155193
def test_right_press_does_not_arm_drag_selection(rules_view, qtbot):
156194
"""Regression: a right-button press armed the drag-selection logic
157195
(mousePressed), interfering with refresh skipping and row tracking."""

0 commit comments

Comments
 (0)