Skip to content

Add extended promotional filter#319

Open
1aday wants to merge 5 commits into
tscircuit:mainfrom
1aday:codex/extended-promotional-filter-bounty
Open

Add extended promotional filter#319
1aday wants to merge 5 commits into
tscircuit:mainfrom
1aday:codex/extended-promotional-filter-bounty

Conversation

@1aday
Copy link
Copy Markdown

@1aday 1aday commented May 20, 2026

/claim #92

Summary

  • derive is_extended_promotional from preferred = 1 and basic = 0
  • expose/filter it on /components/list and /api/search
  • pass the same filter/field through the D1/cf-proxy components/search paths
  • add filter UI and focused tests

Verification

  • bun test tests/lib/is-extended-promotional.test.ts
  • bunx tsc --noEmit
  • bun run format:check
  • cd cf-proxy && bun test test/is-extended-promotional.test.ts test/render.test.ts
  • cd cf-proxy && bunx tsc --noEmit

Note: DB-backed route tests are blocked locally by an existing Kysely fixture state issue (driver has already been destroyed) before assertions run.

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: fb58b2045f

ℹ️ 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 lib/util/is-extended-promotional.ts Outdated
export const isExtendedPromotional = (component: {
basic?: number | boolean | null
preferred?: number | boolean | null
}): boolean => Boolean(component.preferred) && !Boolean(component.basic)
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 Compare promo flags explicitly before marking extended promo

is_extended_promotional is defined elsewhere in this commit as preferred = 1 AND basic = 0 (see the new SQL filters), but this helper uses truthiness and will return true for cases like { preferred: 1, basic: null }. That creates inconsistent API output versus filter behavior (items can be labeled extended promotional but never match is_extended_promotional=true). Use explicit equality checks for preferred and basic to keep derived fields and filtering aligned; the same logic is duplicated in cf-proxy/src/is-extended-promotional.ts.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in bca5a8e: both extended-promotional helpers now use preferred === 1 && basic === 0, matching the SQL filters exactly. I also added regression coverage for { preferred: 1, basic: null } and boolean inputs in both the root and cf-proxy helper tests.

Verification:

  • bun test tests/lib/is-extended-promotional.test.ts
  • cd cf-proxy && bun test test/is-extended-promotional.test.ts test/render.test.ts
  • bun run format:check
  • bunx tsc --noEmit
  • cd cf-proxy && bunx tsc --noEmit
  • git diff --check

@1aday
Copy link
Copy Markdown
Author

1aday commented May 20, 2026

Fixed the CI failure in 116ddf5. The test workflow now installs cf-proxy dependencies before running the root Bun test suite, so cf-proxy imports like kysely-d1 are available on a clean runner. I also fixed the DB test/setup lifecycle that the full suite exposed: derived-table setup now clears the destroyed singleton, and the DB-path test uses an explicit path instead of mutating JLCSEARCH_DB_PATH process-wide.\n\nVerification after building the real local database:\n- bun test (214 pass, 0 fail)\n- bun run format:check\n- bunx tsc --noEmit\n- cd cf-proxy && bunx tsc --noEmit\n- git diff --check\n\nThe fresh GitHub test check is running through setup again now; format-check and type-check are green on the new head.

@1aday
Copy link
Copy Markdown
Author

1aday commented May 20, 2026

Remote CI is green on the latest head 116ddf5: test, format-check, and type-check all completed successfully. The PR merge state is now clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant