Skip to content

fix(rest,ui): 500 on service pool detail and duplicated Advanced tab outside English - #52

Merged
dkmstr merged 5 commits into
masterfrom
master-rest-detail-position-and-gui-tab-i18n
Aug 7, 2026
Merged

fix(rest,ui): 500 on service pool detail and duplicated Advanced tab outside English#52
dkmstr merged 5 commits into
masterfrom
master-rest-detail-position-and-gui-tab-i18n

Conversation

@aschumann-virtualcable

Copy link
Copy Markdown
Contributor

Dos arreglos de la GUI/REST detectados el 2026-08-06, más un test que impide que el primero vuelva.

1. 500 al abrir un service pool concreto — fix(rest): implement get_item_position in remaining DetailHandlers (323eac7db)

4ef7cf2ff convirtió get_item_position en @abstractmethod y quitó el cuerpo de la base. Seis
DetailHandler se quedaron sin implementarlo, así que abrir el detalle de un service pool contestaba
500 y dejaba tres tablas vacías (entre ellas Acciones programadas).

Implementado en cada handler, sin tocar la base.

2. Pestaña Advanced duplicada fuera de inglés — 2827f49a2 + 3dd907fdf

GuiBuilder no traducía el nombre del tab, mientras los campos de módulo llegaban ya traducidos por
gui_description(). Resultado: en un RDP (directo y túnel) con el admin en español salían dos pestañas
Advanced / Avanzado. Se traduce el tab en la salida del builder y el helper se saca del GuiBuilder.

Se descartó la alternativa de no traducir ningún tab (diff de 2 líneas): mandaría a inglés las 9 del
enum y las custom de módulos (Security, Ldap info, SMTP Server, Certificates…), que hoy sí salen
traducidas.

3. Red de seguridad — test(rest): assert handlers implement their abstract contract (d3ffc54e3)

tests/REST/_meta/test_handler_abstracts.py recorre recursivamente el árbol de subclases de
DetailHandler y ModelHandler y falla listando cualquier clase con __abstractmethods__ no vacío.
No mira get_item_position en concreto: cubre el contrato entero, así que muerde con cualquier
@abstractmethod que se añada a las bases en el futuro. Lleva guarda: si el árbol sale vacío, falla en
vez de pasar en verde sin comprobar nada.

RED comprobado sobre master (worktree temporal):

rama    → PENDING: {}                                      2 passed
master  → 6 clases con ['get_item_position']               1 failed, 1 passed
          MetaAssignedService · ServersServers · Changelog
          Groups · Publications · Transports

Las 6 son exactamente las del punto 1. 18 subclases recorridas.

Estado

  • 6 ficheros, +163 / −1 (108 de test).
  • Merge de master (c22d550c2, ort, sin conflictos) → 0 commits por detrás.
  • Suite: 306 passed, 4 skipped, 5 subtests (tests/REST + test_ui_builder.py).

Duda para la review

Changelog.get_item_position es la única que no devuelve un int: hace
raise exceptions.rest.InvalidMethodError("Changelog does not support position"). El raise sale del
propio 4ef7cf2ff, pero un return -1 encaja mejor con que todos devuelvan número. Es una línea y el
test de abstractos pasa igual con las dos — decidme cuál preferís.

Limitaciones conocidas (no bloquean, quedan anotadas)

  1. Los campos de módulo llegan a build() ya traducidos y se les aplica gettext otra vez. Hoy es
    idempotente, pero se rompería si un texto traducido coincidiera con un msgid.
  2. services_pools.py:539 usa .new_tab(gettext("Availability")), traduciendo en la definición mientras
    el resto usa marcadores.
  3. La solución de fondo es mandar el tab como id estable + label traducido y que el SPA agrupe por id en
    vez de por el literal visible. Toca frontend → otra rama.

janpenitent and others added 5 commits August 6, 2026 12:56
Commit 4ef7cf2 made DetailHandler.get_item_position abstract, but six
subclasses did not implement it. Since DetailHandler is an ABC, those
handlers became non-instantiable, raising TypeError on every request and
surfacing as "Error 500: Unexpected error" with empty tables in the
service pool detail tabs (groups, transports, publications, changelog),
the server group detail and the meta pool assigned services.

Implemented using the existing calc_item_position helper. Changelog
entries are keyed by revision instead of uuid, so position does not apply
there and raises InvalidMethodError, matching the base class contract.
REST-side fields (stock fields and GuiBuilder.new_tab) emitted the raw
translation marker 'Advanced', while module fields were already translated
by gui_description(). On non-english locales the admin UI rendered two
separate advanced tabs.

Tabs are now translated on build(), on copies, so the static stock field
definitions are not modified.
It does not use self nor belongs to the builder public API.
A ModelHandler/DetailHandler subclass missing an @AbstractMethod is not
caught at import time: it raises TypeError when the dispatcher instantiates
it, surfacing as "Error 500: Unexpected error" with an empty table. That is
how the six handlers fixed on 323eac7 broke.

Walks the whole subclass tree after the dispatcher registers the handler
modules, and fails listing any pending abstract method.
@github-actions
github-actions Bot requested a review from dkmstr August 7, 2026 12:51
@dkmstr
dkmstr merged commit 14de189 into master Aug 7, 2026
5 checks passed
@aschumann-virtualcable
aschumann-virtualcable deleted the master-rest-detail-position-and-gui-tab-i18n branch August 7, 2026 14:56
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