/polls/voters.json devuelve usuarios duplicados en solicitudes paginadas

Al llamar al endpoint /polls/voters.json, utilizando paginación, hemos descubierto que la primera llamada devuelve 25 usuarios como se esperaba, pero la segunda página devuelve 26 usuarios, uno de los cuales ya se devolvió en la primera llamada. Esto es consistente siempre que haya más de 25 usuarios.

El problema está aquí en el plugin de encuestas para Discourse:
https://github.com/discourse/poll/blob/main/plugins/poll/lib/poll.rb#L223

El desplazamiento (offset) se calcula asumiendo que PostgreSQL es exclusivo con un límite de la fila usando BETWEEN, cuando en realidad es inclusivo.

La primera consulta devuelve 25, porque las filas calculadas en realidad comienzan en 1, no en 0.

Mi solución propuesta es tan simple como:

   params = {
      offset: offset + 1,
      offset_plus_limit: offset + limit,
      option_digest: opts[:option_id].presence,
    }

O, una solución más elegante podría ser usar LIMIT y OFFSET de PostgreSQL.

    params = {
      limit: limit,
      offset: offset,
      option_digest: opts[:option_id].presence,
    }
          WHERE pv.poll_id IN (:poll_ids)
                /* where */
        ) v
        ORDER BY digest, CASE WHEN rank = 'Abstain' THEN 1 ELSE CAST(rank AS integer) END, username
        LIMIT :limit OFFSET :offset
      SQL
2 Me gusta

Buena observación, pondré un pr-welcome en esto en caso de que alguien quiera intentar solucionarlo.

Necesitamos confirmar que no rompemos cosas existentes en las encuestas en caso de que el frontend dependa incorrectamente de suposiciones erróneas aquí.

1 me gusta

¡Gracias por la rápida respuesta!
Intentaré configurar un entorno, no quería hacer un PR sin probar. Cualquiera siéntase libre de adelantarse. No soy un desarrollador de Ruby en absoluto. Tampoco sé sobre cambiar especificaciones o pruebas.

Irónicamente, en la interfaz de usuario hay código que vuelca los resultados en un conjunto para evitar este error.

2 Me gusta

Para cualquier otra persona que esté viendo, hemos implementado una corrección de interfaz de usuario similar para nuestro software de rifas aquí.

Un saludo a @lukecyca de VHS por identificar el área del problema.

También hay un informe relacionado sobre la carga de más votantes en las encuestas de enero.

1 me gusta

Gran hallazgo.

Es incluso más gracioso que eso. Envié un par de PRs muy grandes hace un tiempo para modernizar el frontend y añadir Ranked Choice.

La enorme escala de esos PRs definitivamente contribuyó a que me pasara por alto y las correcciones del backend realmente no estaban en el ámbito (aunque en el proceso de hacer esos PRs, de hecho, resolví muchos problemas que no eran estrictamente para que esos PRs los arreglaran).

Admito haber seguido ese precedente del frontend que se estableció en 2018 para ranked choice sin pensarlo mucho :sweat_smile:. (Que ustedes también siguieron, aunque a sabiendas :laughing:).

Es asombroso cómo las soluciones alternativas pueden persistir sin que se identifique el problema subyacente.

3 Me gusta

Así que pensé en intentarlo como mi primera contribución, pero parece que podría ser un poco más complicado de lo que esperaba. :wink:

Tengo un PR de borrador: FIX: do not return duplicates from /polls/voters.json by clechasseur · Pull Request #1 · clechasseur/discourse · GitHub

En este PR, primero agregué pruebas que reproducen el problema, luego apliqué la solución simple de Rob y las pruebas pasaron.

También probé la solución más elegante (la que habría preferido), pero aunque evita los votantes duplicados, también cambia qué votante se devuelve en cada página, incluida la primera, lo que podría considerarse un cambio importante (dependiendo de cómo lo maneje el frontend; aún no lo he mirado).

Sin embargo, dando un paso atrás, me pregunto cuál es el verdadero significado del parámetro limit cuando se llama a este endpoint: en realidad no limita el número de votantes totales devueltos, sino solo el número de votantes devueltos por cada opción de encuesta. Puede ver este efecto en la prueba que agregué para la encuesta de opción múltiple aquí: la primera página se limita efectivamente a 2 votantes por opción, pero en total se devuelven tres votantes diferentes (distribuidos entre las opciones). Al cambiar a la solución elegante (es decir, usar LIMIT :limit OFFSET :offset), el limit se aplica al número total de votos y no de votantes. No estoy 100% seguro de que sea mejor o más intuitivo.

De todos modos, soy nuevo en esto, así que quizás lo estoy pensando demasiado. La solución simple elimina los votantes duplicados y no causa demasiados estragos, por lo que podría ser el camino a seguir. Esperaré comentarios antes de enviar un PR al repositorio principal.

Aparte, creo que hay otro error en esta parte del código. La consulta para cargar los votantes se ordena por digest, rank y username, pero al ordenar por rank, utiliza esta condición:

CASE WHEN rank = 'Abstain' THEN 1 ELSE CAST(rank AS integer) END

Sin embargo, 'Abstain' en realidad corresponde al rango 0, no al 1; el rango 1 también puede devolverse como '1'. Esto potencialmente hace que el orden sea no determinista entre consultas, lo que significa que, dependiendo del número de votantes y el valor de limit utilizado, podría ser posible omitir votantes al realizar consultas paginadas. En mis nuevas pruebas, tuve que ordenar los votantes devueltos para evitar la naturaleza no determinista. (Dado que no es determinista, supongo que no es fácil de reproducir en una prueba, pero puedo intentarlo…).

1 me gusta

Seguí el enlace anterior a un informe anterior, que incluye un enlace a un informe anterior anterior que incluye algo que también parece apuntar a esta posibilidad:

Sin embargo, eso fue hace mucho tiempo, por lo que el código podría haber sido totalmente diferente en ese entonces. (Intenté retroceder en el tiempo con Git pero el historial se detuvo en 2021, así que supongo que el código se movió en algún momento).

Después de algo de experimentación, no pude crear una prueba que causara que el orden no determinista causara un problema con los votantes devueltos. Creo que esto se debe a la forma en que se elabora la consulta y a cómo se crean los votos de la encuesta, pero no estoy 100% seguro.

He creado una PR con mi prueba y las correcciones que propongo: FIX: Evitar devolver votantes duplicados del endpoint /polls/voters.json por clechasseur · Pull Request #34433 · discourse/discourse

3 Me gusta

Muchas gracias por la corrección y tu paciencia durante nuestra revisión.

Lo he aprobado y fusionado. :partying_face:

3 Me gusta

Este tema se cerró automáticamente después de 3 días. Ya no se permiten nuevas respuestas.