Skip to content

Commit 6b5ff7a

Browse files
WHOIM1205florentc
andcommitted
fix: store unedited packages as suggestion cache
`original_packages` was incorrectly set to the edited packages dict, losing the pre-edit baseline needed for undo and diff display. Add tests covering ignore, restore, and sequential ignore flows. Co-authored-by: Florent C. <florentc@users.noreply.github.com> Signed-off-by: WHOIM1205 <rathourprateek8@gmail.com>
1 parent 6e8ca2d commit 6b5ff7a

File tree

2 files changed

+132
-3
lines changed

2 files changed

+132
-3
lines changed

src/shared/listeners/cache_suggestions.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,9 +234,7 @@ def cache_new_suggestions(suggestion: CVEDerivationClusterProposal) -> None:
234234
title=relevant_piece["title"],
235235
description=relevant_piece["descriptions__value"],
236236
affected_products=affected_products,
237-
# FIXME(@fricklerhandwerk): It's probably because I don't understand the code involved too well, but it seems wrong that we don't keep the original packages here.
238-
# If it must be that way, document why.
239-
original_packages=packages,
237+
original_packages=original_packages,
240238
packages=packages,
241239
metrics=[to_dict(m) for m in prefetched_metrics],
242240
categorized_maintainers=categorize_maintainers(packages, maintainer_overlays),

src/webview/tests/test_packages.py

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,134 @@ def test_ignore_restore_package(
9393
expect(active_packages.get_by_text("package1")).to_be_visible()
9494
expect(active_packages.get_by_text("package2")).to_be_visible()
9595
expect(ignored_packages).not_to_be_visible()
96+
97+
98+
def test_ignore_multiple_packages(
99+
live_server: LiveServer,
100+
as_staff: Page,
101+
make_cached_suggestion: Callable[..., CVEDerivationClusterProposal],
102+
make_drv: Callable[..., NixDerivation],
103+
) -> None:
104+
"""Ignoring packages one by one keeps all of them in the ignored section."""
105+
drv1 = make_drv(pname="alpha")
106+
drv2 = make_drv(pname="bravo")
107+
drv3 = make_drv(pname="charlie")
108+
suggestion = make_cached_suggestion(
109+
drvs={
110+
drv1: ProvenanceFlags.PACKAGE_NAME_MATCH,
111+
drv2: ProvenanceFlags.PACKAGE_NAME_MATCH,
112+
drv3: ProvenanceFlags.PACKAGE_NAME_MATCH,
113+
},
114+
)
115+
116+
as_staff.goto(
117+
live_server.url
118+
+ reverse("webview:suggestion:detail", kwargs={"suggestion_id": suggestion.pk})
119+
)
120+
121+
active = as_staff.locator(f"#suggestion-{suggestion.pk}-active-packages")
122+
ignored = as_staff.locator(f"#suggestion-{suggestion.pk}-ignored-packages")
123+
container = as_staff.locator(f"#suggestion-{suggestion.pk}")
124+
125+
# Ignore alpha
126+
active.locator(".package-alpha").get_by_role("button", name="Ignore").click()
127+
container.get_by_text(re.compile("Ignored packages")).click()
128+
expect(ignored.get_by_text("alpha")).to_be_visible()
129+
expect(active.get_by_text("bravo")).to_be_visible()
130+
expect(active.get_by_text("charlie")).to_be_visible()
131+
132+
# Ignore bravo
133+
active.locator(".package-bravo").get_by_role("button", name="Ignore").click()
134+
ignored.click()
135+
expect(ignored.get_by_text("alpha")).to_be_visible()
136+
expect(ignored.get_by_text("bravo")).to_be_visible()
137+
expect(active.get_by_text("charlie")).to_be_visible()
138+
139+
# Only charlie remains active
140+
expect(active.get_by_text("alpha")).not_to_be_visible()
141+
expect(active.get_by_text("bravo")).not_to_be_visible()
142+
143+
144+
def test_restore_one_of_multiple_ignored_packages(
145+
live_server: LiveServer,
146+
as_staff: Page,
147+
make_cached_suggestion: Callable[..., CVEDerivationClusterProposal],
148+
make_drv: Callable[..., NixDerivation],
149+
) -> None:
150+
"""Restoring one ignored package does not affect other ignored packages."""
151+
drv1 = make_drv(pname="alpha")
152+
drv2 = make_drv(pname="bravo")
153+
drv3 = make_drv(pname="charlie")
154+
suggestion = make_cached_suggestion(
155+
drvs={
156+
drv1: ProvenanceFlags.PACKAGE_NAME_MATCH,
157+
drv2: ProvenanceFlags.PACKAGE_NAME_MATCH,
158+
drv3: ProvenanceFlags.PACKAGE_NAME_MATCH,
159+
},
160+
)
161+
162+
as_staff.goto(
163+
live_server.url
164+
+ reverse("webview:suggestion:detail", kwargs={"suggestion_id": suggestion.pk})
165+
)
166+
167+
active = as_staff.locator(f"#suggestion-{suggestion.pk}-active-packages")
168+
ignored = as_staff.locator(f"#suggestion-{suggestion.pk}-ignored-packages")
169+
container = as_staff.locator(f"#suggestion-{suggestion.pk}")
170+
171+
# Ignore both alpha and bravo
172+
active.locator(".package-alpha").get_by_role("button", name="Ignore").click()
173+
container.get_by_text(re.compile("Ignored packages")).click()
174+
active.locator(".package-bravo").get_by_role("button", name="Ignore").click()
175+
176+
# Restore only alpha
177+
ignored.click()
178+
ignored.locator(".package-alpha").get_by_role("button", name="Restore").click()
179+
180+
ignored.click()
181+
expect(active.get_by_text("alpha")).to_be_visible()
182+
expect(active.get_by_text("charlie")).to_be_visible()
183+
expect(ignored.get_by_text("bravo")).to_be_visible()
184+
expect(active.get_by_text("bravo")).not_to_be_visible()
185+
186+
187+
def test_ignored_packages_persist_across_page_load(
188+
live_server: LiveServer,
189+
as_staff: Page,
190+
make_cached_suggestion: Callable[..., CVEDerivationClusterProposal],
191+
make_drv: Callable[..., NixDerivation],
192+
) -> None:
193+
"""Ignored packages remain ignored after navigating away and back."""
194+
drv1 = make_drv(pname="alpha")
195+
drv2 = make_drv(pname="bravo")
196+
suggestion = make_cached_suggestion(
197+
drvs={
198+
drv1: ProvenanceFlags.PACKAGE_NAME_MATCH,
199+
drv2: ProvenanceFlags.PACKAGE_NAME_MATCH,
200+
},
201+
)
202+
203+
detail_url = live_server.url + reverse(
204+
"webview:suggestion:detail", kwargs={"suggestion_id": suggestion.pk}
205+
)
206+
as_staff.goto(detail_url)
207+
208+
active = as_staff.locator(f"#suggestion-{suggestion.pk}-active-packages")
209+
210+
# Ignore bravo
211+
active.locator(".package-bravo").get_by_role("button", name="Ignore").click()
212+
213+
# Navigate away and back
214+
as_staff.goto(detail_url)
215+
216+
active = as_staff.locator(f"#suggestion-{suggestion.pk}-active-packages")
217+
ignored = as_staff.locator(f"#suggestion-{suggestion.pk}-ignored-packages")
218+
219+
expect(active.get_by_text("alpha")).to_be_visible()
220+
expect(active.get_by_text("bravo")).not_to_be_visible()
221+
222+
# Open ignored section and verify bravo is there
223+
as_staff.locator(f"#suggestion-{suggestion.pk}").get_by_text(
224+
re.compile("Ignored packages")
225+
).click()
226+
expect(ignored.get_by_text("bravo")).to_be_visible()

0 commit comments

Comments
 (0)