Skip to content

fix(search): remove prequalified results#7130

Closed
rmelisson wants to merge 1 commit intodevfrom
remi/search-nopq
Closed

fix(search): remove prequalified results#7130
rmelisson wants to merge 1 commit intodevfrom
remi/search-nopq

Conversation

@rmelisson
Copy link
Copy Markdown
Contributor

  • pour tester les résultats de recherche sans les PQ

@revu-bot revu-bot bot requested a review from revu-bot March 3, 2026 13:12
@rmelisson rmelisson temporarily deployed to build-preproduction March 3, 2026 13:12 — with GitHub Actions Inactive
Copy link
Copy Markdown
Collaborator

@revu-bot revu-bot left a comment

Choose a reason for hiding this comment

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

PR Summary: Remove Prequalified Results

This PR disables prequalified (PQ) results in two places — the search page (app/recherche/page.tsx) and the presearch() controller method — and adds a new app/api/search/route.ts file.

File Lines Severity Issue
src/api/modules/search/controller.ts 15 CRITICAL get() still enables PQ by default; inconsistent with PR intent
src/api/modules/search/controller.ts 15 IMPORTANT q as string cast without null guard — runtime crash on missing param
app/api/search/route.ts 1–7 IMPORTANT New route has no tests and no docstring on the exported handler

Overall: The change is partially applied — presearch() and the page are updated, but the get() method in the same controller still defaults PQ to true when no pq query param is provided. This is likely a bug given the stated goal of the PR.

Comment on lines +1 to +8
import { SearchController } from "src/api";

export const dynamic = "force-dynamic";

export async function GET(request: Request) {
const controller = new SearchController(request);
return controller.get();
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[IMPORTANT] New public route has no tests and no docstring

  • Per the coding guidelines, public functions must have concise docstrings explaining purpose and return values.
  • The new GET handler is a public API entry point with no test coverage. The guidelines require 100% coverage for critical code — an API route qualifies.
  • At minimum, tests should cover:
    • Happy path: valid ?q= returns 200 with results
    • Missing q param returns 400
    • ?pq=true vs default behaviour

Suggested docstring addition:

Suggested change
import { SearchController } from "src/api";
export const dynamic = "force-dynamic";
export async function GET(request: Request) {
const controller = new SearchController(request);
return controller.get();
}
import { SearchController } from "src/api";
export const dynamic = "force-dynamic";
/**
* GET /api/search
*
* Searches documents matching the given query.
* Query params: `q` (required) search term; `pq` (optional, default off) enable prequalified results.
* Returns a JSON response with matching documents and classification.
*/
export async function GET(request: Request) {
const controller = new SearchController(request);
return controller.get();
}

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Mar 3, 2026

@tokenbureau
Copy link
Copy Markdown

tokenbureau bot commented Apr 1, 2026

@m-maillot
Copy link
Copy Markdown
Contributor

remplacé par : #7222 et rebase sur la branche dev

@m-maillot m-maillot closed this Apr 8, 2026
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.

3 participants