[Regression on closed #585] ReputationService.history orders computedAt ASC again, so the default page returns the oldest 50 snapshots

Aperta Adatta ai principianti
  • Qualcosa non va?
#732 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
2/5
Tempo stimato
1-3 ore
Idoneità per principianti
79/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
typescript
Ambito
backend-api-design

Direzione di ricerca

Inizia da src/reputation/reputation.service.ts in history, poi controlla il punto di ingresso del controller in src/reputation/reputation.controller.ts:71 e i test di reputazione esistenti. Verifica come la query del repository ordina e pagina gli snapshot, inclusi quelli con timestamp computedAt uguali. Il lavoro è completo quando la pagina predefinita restituisce gli snapshot più recenti in un ordine deterministico e un test copre l’ordinamento; l’issue suggerisce anche di restituire metadati di paginazione.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug

Location — src/reputation/reputation.service.ts:88-99 (history), reached from src/reputation/reputation.controller.ts:71 (GET /reputation/:userId/history).

Problem — The ordering closed as #585 is back:

return this.snapshotRepo.find({
  where: { userId },
  order: { computedAt: 'ASC' },
  take: limit,
  skip: offset,
});

reputation_snapshots is an append-only table (a new row per recompute), so ASC + skip: 0 returns the first snapshots ever written. With the default limit of 50 (REPUTATION_HISTORY_DEFAULT_LIMIT), a contributor who has accumulated more than 50 snapshots gets a page that contains none of their recent reputation and never changes again, no matter how much their stats move. The computedAt sort is also not tie-broken: computedAt is a @CreateDateColumn timestamp, and several recomputes in the same tick (the reputation.recompute event handler at line 78 can fire in bursts) order arbitrarily between pages, so offset paging can repeat or skip rows.

Why it matters — The history endpoint exists to chart reputation over time; its default response is the least useful 50 rows in the table and is indistinguishable from stale data to a client. To reach current data a caller must know the total row count and compute the final offset, which the endpoint does not return. For long-standing contributors (the users a reputation system is for) the endpoint is effectively broken.

Reproduction / failure path

  1. Pick a user and generate 60 snapshots (call POST /reputation/:userId/recompute repeatedly with changing stats, or emit reputation.recompute 60 times).
  2. GET /reputation/:userId/history with no query params.
  3. The response's last element is snapshot #50, not #60; the 10 most recent snapshots are unreachable without an explicit offset.

Suggested fix

  • Change the order to { computedAt: 'DESC', id: 'DESC' } so the default page is the most recent snapshots and equal timestamps resolve deterministically.
  • If clients need chronological order for charting, let them reverse the returned page (or accept an explicit order=asc|desc query param defaulting to desc) rather than making the un-paginated oldest-first behaviour the default.
  • Return a total count (or a nextOffset) alongside the rows so a client can page without guessing.
  • Add a spec asserting the order passed to snapshotRepo.find is DESC with an id tie-break; #585's fix shipped without one, which is why it regressed.
Lingua principale
TypeScript
Stelle
2
Fork
90
Merge medio
31m
PR unite (30g)
20

Preparare l'ambiente

  • Include un Dockerfile o un file Docker Compose
  • Nessun modello di pull request
  • Nessuna guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di MergeFi/backend

Tutte le issue di MergeFi/backend

Issue simili

Altre issue su TypeScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.