Skip to content

Commit c70a9ac

Browse files
Shalom-Karrclaude
andcommitted
Notifications filter: full-page reload + diagnostic logs
The pushState approach didn't stick in the CI screenshot — the URL still reported as no-query and the dropdown stayed on "All types". Likely an interaction with Discourse's location service. Falling back to window.location.assign(newUrl) does an unconditional full navigation; on reload, model() reads the fresh ?type= from window.location and the filtered list renders. Slightly heavier UX but bulletproof for the click case. For the still-failing direct-URL cases (tests 2 & 3), the failure screenshots show TYPE = "new reply" (so selectedValue does read the URL) yet both Liked and Replied rows remain visible — the server didn't filter. Added one-line warn logs at the model() boundary and at the controller patch's index branch so the next CI cycle prints exactly what the route built and what the server received. Will remove once the cause is pinned down. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017kMMnEotb32Equr761bQkf
1 parent 9199f3d commit c70a9ac

3 files changed

Lines changed: 26 additions & 11 deletions

File tree

assets/javascripts/discourse/components/notifications-type-filter.gjs

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -73,29 +73,29 @@ export default class NotificationsTypeFilter extends Component {
7373
}
7474

7575
@action
76-
async onChange(value) {
76+
onChange(value) {
7777
// `router.transitionTo(path + ?type=...)` is a NO-OP here because
7878
// Ember treats the new URL as the same route — `type` isn't declared
7979
// as a controller queryParam (an Ember classic-reopen limitation
8080
// noted at length in the initializer), so the queryParam diff is
8181
// empty and the transition silently aborts WITHOUT touching the
82-
// URL. Confirmed via the failing system spec: the URL stayed at the
83-
// original path after picking a type from the dropdown.
82+
// URL. A pushState+router.refresh dance had a similar problem —
83+
// the Capybara assertion saw the URL revert.
8484
//
85-
// `window.history.pushState` bypasses Ember's router entirely and
86-
// updates the address-bar URL unconditionally. `router.refresh()`
87-
// then re-runs the route's model(), which reads `?type=...` straight
88-
// from window.location — so the list re-fetches with the new
89-
// filter. Same shape on the way back to All (the `type` key is
90-
// removed first).
85+
// Falling back to a full navigation via `window.location.assign`
86+
// sidesteps the whole Ember queryParam dance. The page reloads,
87+
// route model() reads the fresh `?type=` from window.location, and
88+
// the filtered list renders. Slightly heavier than an in-app
89+
// transition, but the alternative is reaching into Discourse's
90+
// queryParam plumbing — and the filter pick happens infrequently
91+
// enough that a single reload is a fine UX trade.
9192
const url = new URL(window.location.href);
9293
if (value === ALL) {
9394
url.searchParams.delete("type");
9495
} else {
9596
url.searchParams.set("type", value);
9697
}
97-
window.history.pushState({}, "", url.pathname + url.search);
98-
await this.router.refresh();
98+
window.location.assign(url.pathname + url.search);
9999
}
100100

101101
<template>

assets/javascripts/discourse/initializers/notifications-type-filter.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,21 @@ export default {
5858
this.get("currentUser.username") !== username &&
5959
!this.get("currentUser.admin")
6060
) {
61+
// eslint-disable-next-line no-console
62+
console.warn("[notif-filter] early exit: not own profile or admin");
6163
return;
6264
}
6365
const args = { username, filter: params.filter, limit: 60 };
6466
const urlType = new URLSearchParams(window.location.search).get(
6567
"type"
6668
);
69+
// eslint-disable-next-line no-console
70+
console.warn(
71+
"[notif-filter] model() — urlType=",
72+
urlType,
73+
"location=",
74+
window.location.href
75+
);
6776
if (urlType && urlType !== "all") {
6877
if (urlType === "mod_notes") {
6978
// Staff-only path — server patch handles routing into a
@@ -76,6 +85,8 @@ export default {
7685
args.filter_by_types = urlType.toLowerCase();
7786
}
7887
}
88+
// eslint-disable-next-line no-console
89+
console.warn("[notif-filter] store.find args=", JSON.stringify(args));
7990
return this.store.find("notification", args);
8091
},
8192
});

sub_plugins/mod_categories.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1408,6 +1408,10 @@ def index
14081408
# core controller a value it recognizes.
14091409
canonical_type =
14101410
::Notification.types.keys.find { |k| k.to_s.casecmp(requested_type).zero? }
1411+
::Rails.logger.warn(
1412+
"[notif-filter] NotificationsController#index requested_type=#{requested_type.inspect} " \
1413+
"canonical=#{canonical_type.inspect} filter_by_types(before)=#{params[:filter_by_types].inspect}",
1414+
)
14111415
params[:filter_by_types] = canonical_type.to_s if canonical_type
14121416
params.delete(:type)
14131417
super

0 commit comments

Comments
 (0)