UNIMOODLE / moodle-mod_hybridteaching

P1
Other
2 stars 0 forks source link

refactor: attendance_render en classes/output/attendance_render.php debe user table_sql adecuadamente #114

Open moodleulpgc opened 8 months ago

moodleulpgc commented 8 months ago

Esta clase NO es un renderer, no hereda de plugin_renderer. El modelo de moodle lla a establecer estructuras de datos (como estas tablas) en clases que contienen datos. QUe luego se visualizan con el renderer usámdo méntodos render_xxxxxxx(nombre-dela-clase) que aceptan una instancia de la case como argumento. class attendance_render extends \table_sql implements dynamic_table

Las table_sql se utilizan para contener listados que se pueden obtener por una query SQL definida en la propia clase. Aquí no se usa eso, sino que se obtiene el array de items separadamente. Por ejemplo

$attendancelist = $attendancecontroller->load_attendance($page, $perpage, $params, $extrasql,
        $operator, $sort, $dir, $view, $sessionid, $fname, $lname);
$attendanceassist = $attendancecontroller->load_attendance_assistance($params, $extrasql, $operator);
$attendancecount = $attendancecontroller->count_attendance( $fname, $lname, $selectedsession, $params $operator);

Para eso, no es necesario ni merece la pena una clase tabla_sql, basta flexible_table. Ninguna de estas dos clases usa propiamente los métodos de flexibletable col{columnname} para especificar el contenido de cada columna.

Tampoco se usan los métodos estandar de estas clases para la gestión de sus propiedades, por ejemplo

y muchos otros, no se usan en absoluto, lo he hace inútil heredar de flexible_table y se re-escribe código que ya estaría en flexible_table / sql_table.

El resultado es una spagetificación de código que lo hace muy poco entendible y muy poco mantenible a largo palzo, ni adaptable a futuras versiones.

Por otra parte, los diferentes métodos print_ tienen incluso varios retornos distintos según la página.
Para conseguir eso lo más conveniente sería definir una clase de tabla base (customizada para hybridteaching), y después varias clases especializadas, que heredan de la base y añaden/modifican los detalles específicos para cada tipo de tabla.

mcalvoisyccom commented 6 months ago

Esta issue es una mejora no incluida actualmente en las especificaciones, no lo asumimos en este proyecto.

moodleulpgc commented 6 months ago

No estoy de acuerdo, en todas las imágenes (y los textos) se especifica que estas tablas deben ser instancias de flex_table, ordenables, exportables, con las características de

Lo que este issue plantea es que se usa la clase por el nombre pero SIN usar realmente sus métodos y características. Una mala implementación, lo siento.

tmas0 commented 16 hours ago

Buenas,

No se ha cambiado para que se use flexible_table, por tanto, interpreto que este ticket no está solucionado según se pide.

Un saludo