Skip to content

feat(mobile): filter inbox reports by priority (port #2176)#2497

Open
Gilbert09 wants to merge 2 commits into
mainfrom
posthog-code/mobile-inbox-priority-filter
Open

feat(mobile): filter inbox reports by priority (port #2176)#2497
Gilbert09 wants to merge 2 commits into
mainfrom
posthog-code/mobile-inbox-priority-filter

Conversation

@Gilbert09
Copy link
Copy Markdown
Member

Ports the desktop filter inbox signal reports by priority feature (#2176) to the PostHog Code mobile app.

What changed

  • Store (inboxFilterStore.ts): added priorityFilter: SignalReportPriority[] (empty = no filter), with togglePriority and setPriorityFilter actions. Persisted and cleared in resetFilters, matching the existing statusFilter / sourceProductFilter / suggestedReviewerFilter patterns.
  • Param builder (utils.ts): added buildPriorityFilterParam, returning a deduped comma-separated string or undefined when empty (equivalent to desktop's helper, sibling to buildSuggestedReviewerFilterParam).
  • Query wiring (useInboxReports.ts + api.ts + types.ts): the selected priorities flow into the reports query as a priority request param.
  • UI (FilterSheet.tsx): added a Priority section with P0–P4 toggle rows and colored dots (P0 red, P1/P2 amber-orange, P3/P4 gray), reusing the existing OptionRow / SectionHeader primitives and theme colors.

Tests

  • inboxFilterStore.test.ts: toggle on/off, accumulation, dedupe on setPriorityFilter, and clear-on-reset.
  • utils.test.ts: buildPriorityFilterParam for empty (undefined), join, and dedupe.

Mobile inbox tests, biome lint, and typecheck all pass.

No desktop (apps/code) code was modified.

Ports the desktop "filter inbox signal reports by priority" feature to the
mobile app.

- Add `priorityFilter` (P0–P4) state to the inbox filter store, with
  `togglePriority` / `setPriorityFilter` actions; persisted and cleared on
  reset, mirroring the existing status/source/reviewer filters.
- Add a `buildPriorityFilterParam` helper that returns a deduped
  comma-separated string, or `undefined` when empty.
- Pass the selected priorities to the signal reports query as a `priority`
  param via `useInboxReports` and the API client.
- Add a Priority section (P0–P4 toggle rows with colored dots) to the filter
  sheet.
- Unit tests for the store actions and the param builder.

Generated-By: PostHog Code
Task-Id: bf3907c8-72ee-420f-97e7-8f3e176f6eaa
@Gilbert09 Gilbert09 requested a review from a team June 5, 2026 10:45
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 5, 2026

Comments Outside Diff (1)

  1. apps/mobile/src/features/inbox/utils.ts, line 178-238 (link)

    P1 Priority filter omitted from analytics has_active_filters

    InboxViewedFilterState has no priorityFilter field, so buildInboxViewedProperties never sets has_active_filters: true when the user's only active filter is a priority selection. The call-site in inbox.tsx also doesn't pass priorityFilter (and couldn't — it's not in the interface). Every INBOX_VIEWED event fired while only a priority filter is active will report has_active_filters: false, silently under-counting filtered sessions in analytics.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/mobile/src/features/inbox/utils.ts
    Line: 178-238
    
    Comment:
    **Priority filter omitted from analytics `has_active_filters`**
    
    `InboxViewedFilterState` has no `priorityFilter` field, so `buildInboxViewedProperties` never sets `has_active_filters: true` when the user's only active filter is a priority selection. The call-site in `inbox.tsx` also doesn't pass `priorityFilter` (and couldn't — it's not in the interface). Every `INBOX_VIEWED` event fired while only a priority filter is active will report `has_active_filters: false`, silently under-counting filtered sessions in analytics.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
apps/mobile/src/features/inbox/utils.ts:178-238
**Priority filter omitted from analytics `has_active_filters`**

`InboxViewedFilterState` has no `priorityFilter` field, so `buildInboxViewedProperties` never sets `has_active_filters: true` when the user's only active filter is a priority selection. The call-site in `inbox.tsx` also doesn't pass `priorityFilter` (and couldn't — it's not in the interface). Every `INBOX_VIEWED` event fired while only a priority filter is active will report `has_active_filters: false`, silently under-counting filtered sessions in analytics.

### Issue 2 of 2
apps/mobile/src/features/inbox/utils.test.ts:233-245
The three `buildPriorityFilterParam` cases are pure input/output pairs that fit naturally into `it.each`, consistent with how the sibling `toSuggestedReviewerWriteContent` and `reviewerMatchesAvailable` tests are written in the same file.

```suggestion
describe("buildPriorityFilterParam", () => {
  it.each([
    { name: "returns undefined for an empty selection", input: [], expected: undefined },
    { name: "joins selected priorities with commas", input: ["P0", "P2"] as const, expected: "P0,P2" },
    { name: "dedupes repeated priorities", input: ["P1", "P1", "P3"] as const, expected: "P1,P3" },
  ])("$name", ({ input, expected }) => {
    expect(buildPriorityFilterParam(input)).toBe(expected);
  });
});
```

Reviews (1): Last reviewed commit: "feat(mobile): filter inbox reports by pr..." | Re-trigger Greptile

Comment on lines +233 to +245
describe("buildPriorityFilterParam", () => {
it("returns undefined for an empty selection", () => {
expect(buildPriorityFilterParam([])).toBeUndefined();
});

it("joins selected priorities with commas", () => {
expect(buildPriorityFilterParam(["P0", "P2"])).toBe("P0,P2");
});

it("dedupes repeated priorities", () => {
expect(buildPriorityFilterParam(["P1", "P1", "P3"])).toBe("P1,P3");
});
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 The three buildPriorityFilterParam cases are pure input/output pairs that fit naturally into it.each, consistent with how the sibling toSuggestedReviewerWriteContent and reviewerMatchesAvailable tests are written in the same file.

Suggested change
describe("buildPriorityFilterParam", () => {
it("returns undefined for an empty selection", () => {
expect(buildPriorityFilterParam([])).toBeUndefined();
});
it("joins selected priorities with commas", () => {
expect(buildPriorityFilterParam(["P0", "P2"])).toBe("P0,P2");
});
it("dedupes repeated priorities", () => {
expect(buildPriorityFilterParam(["P1", "P1", "P3"])).toBe("P1,P3");
});
});
describe("buildPriorityFilterParam", () => {
it.each([
{ name: "returns undefined for an empty selection", input: [], expected: undefined },
{ name: "joins selected priorities with commas", input: ["P0", "P2"] as const, expected: "P0,P2" },
{ name: "dedupes repeated priorities", input: ["P1", "P1", "P3"] as const, expected: "P1,P3" },
])("$name", ({ input, expected }) => {
expect(buildPriorityFilterParam(input)).toBe(expected);
});
});
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/mobile/src/features/inbox/utils.test.ts
Line: 233-245

Comment:
The three `buildPriorityFilterParam` cases are pure input/output pairs that fit naturally into `it.each`, consistent with how the sibling `toSuggestedReviewerWriteContent` and `reviewerMatchesAvailable` tests are written in the same file.

```suggestion
describe("buildPriorityFilterParam", () => {
  it.each([
    { name: "returns undefined for an empty selection", input: [], expected: undefined },
    { name: "joins selected priorities with commas", input: ["P0", "P2"] as const, expected: "P0,P2" },
    { name: "dedupes repeated priorities", input: ["P1", "P1", "P3"] as const, expected: "P1,P3" },
  ])("$name", ({ input, expected }) => {
    expect(buildPriorityFilterParam(input)).toBe(expected);
  });
});
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Include `priorityFilter` in `buildInboxViewedProperties` so the
`has_active_filters` analytics flag is true when a priority filter is the only
active filter; previously priority-only filtering was reported as no active
filters. Thread `priorityFilter` through the inbox screen's INBOX_VIEWED call
site and add coverage for the priority-only case.

Also fold the `buildPriorityFilterParam` cases into `it.each`, matching the
sibling table-driven tests in the file.

Generated-By: PostHog Code
Task-Id: bf3907c8-72ee-420f-97e7-8f3e176f6eaa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant