Skip to content

[Review Stack] #857 on #860#6

Open
akira69 wants to merge 1 commit intoreview/860-on-846from
review/857-on-860
Open

[Review Stack] #857 on #860#6
akira69 wants to merge 1 commit intoreview/860-on-846from
review/857-on-860

Conversation

@akira69
Copy link
Copy Markdown
Owner

@akira69 akira69 commented Mar 13, 2026

This is a fork-only stacked review PR for upstream Donkie/Spoolman#857.

Use this PR for true stacked review diff only.

Chain:

  • Base upstream foundation: #846
  • Prior stacked review layer: review/860-on-846
  • This review branch: review/857-on-860

Notes:

  • The code/content matches the current canonical feat/manufacturer-logo-upstream-split branch tree.
  • The canonical upstream PR branch remains flat against master so the upstream PR stays clean.
  • This review PR exists only to render the stacked diff cleanly.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b555eaa996

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread client/src/pages/vendors/list.tsx Outdated
Comment on lines +200 to +203
// The logo filter is resolved against the currently loaded rows only. Do not forward
// it into Refine's server-side filter state, or older persisted values can blank the list.
delete forwardedFilters.logo;
tableProps.onChange(pagination, forwardedFilters, sorter, extra);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Forward logo filter to server-side vendor search

useTable is configured for server-side pagination, but this handler removes logo before forwarding filters, so the "Has logo/No logo" filter only runs via onFilter on the currently loaded page rows. When vendors span multiple pages, matches on other pages are never requested and pagination totals remain unfiltered, so the new logo filter gives incomplete results; pass the logo filter through to the backend query so it applies to the full dataset.

Useful? React with 👍 / 👎.

@akira69 akira69 force-pushed the review/857-on-860 branch from b555eaa to 837aafe Compare March 15, 2026 14:36
@akira69 akira69 force-pushed the review/860-on-846 branch 2 times, most recently from 23efa57 to 6df5e78 Compare March 17, 2026 15:15
@akira69 akira69 force-pushed the review/857-on-860 branch from 06c9825 to d127692 Compare March 17, 2026 15:15
@akira69 akira69 force-pushed the review/857-on-860 branch from fda598b to 741d993 Compare March 18, 2026 22:44
@akira69 akira69 force-pushed the review/860-on-846 branch from 6df5e78 to 723bb32 Compare March 18, 2026 22:44
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