Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modul de presencia setmanal #51

Merged
merged 4 commits into from Jul 25, 2019
Merged

Conversation

xeviterr
Copy link
Collaborator

Mòdul que ja fa temps que fem servir a Montilivi per veure com està la presencia al llarg de la setmana.
És útil per fer-se una idea de com van els alumnes.

També es pot passar llista, però crec que ningú ho fa servir, més que per modificar alguna assistència concreta. Aquest punt s'hauria de unificar amb el codi de presència per no tenir funcionalitat duplicada, el problema és que no sé on posar el codi compartit entre el mòdul presència i el mòdul presenciaSetmanal.

image

@ctrl-alt-d ctrl-alt-d self-requested a review July 15, 2019 17:06
Copy link
Owner

@ctrl-alt-d ctrl-alt-d left a comment

Choose a reason for hiding this comment

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

Wow! Bestial, i amb paràmetre de custom:

CUSTOM_MODUL_PRESENCIA_SETMANAL_ACTIU = True

Per mi merge! Mirar comentaris.

@ctrl-alt-d ctrl-alt-d requested a review from juaky July 15, 2019 17:07
@ctrl-alt-d
Copy link
Owner

He provat el mòdu i, quan he triat el grup, m'ha sortit un error:

Selection_582

The above exception (column "2019-07-15" does not exist LINE 1: ...horaria.id AND presencia_impartir.dia_impartir >= "2019-07-1... ^ ) was the direct cause of the following exception:

/opt/djau2019/aula/apps/presenciaSetmanal/views.py in detallgrup
    for assistencia in assistencies: 

Crec que és degut a aquesta select:

str('SELECT alumnes_alumne.id as id_alumne, presencia_impartir.id as id_hora, ' +
            'presencia_controlassistencia.estat_id as estat_id, ' +
            'presencia_controlassistencia.id, alumnes_alumne.nom as nom_alumne, alumnes_alumne.cognoms as cognoms_alumne ' +
            'FROM presencia_controlassistencia, alumnes_alumne, presencia_impartir, ' +
            'horaris_horari, horaris_diadelasetmana, horaris_franjahoraria ' +
            'WHERE alumnes_alumne.id = alumne_id AND ' +
            'presencia_controlassistencia.impartir_id = presencia_impartir.id AND ' +
            'presencia_impartir.horari_id = horaris_horari.id ' +
            'AND horaris_horari.dia_de_la_setmana_id = horaris_diadelasetmana.id AND ' +
            'horaris_horari.hora_id = horaris_franjahoraria.id AND ' +
            'presencia_impartir.dia_impartir >= "' + dillunsSetmana.isoformat() + '" AND ' +
            'presencia_impartir.dia_impartir <= "' + divendresSetmana.isoformat() + '" AND ' +
            'horaris_horari.grup_id = ' + grup_id + ' ' +
            'ORDER BY alumnes_alumne.cognoms, horaris_diadelasetmana.n_dia_ca, horaris_franjahoraria.hora_inici')

Abans de fer el merge hauriem de transformar les selects en query api, per temes de portabilitat i de seguretat.

@ctrl-alt-d ctrl-alt-d self-requested a review July 15, 2019 22:23
Copy link
Owner

@ctrl-alt-d ctrl-alt-d left a comment

Choose a reason for hiding this comment

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

  • Canviar els sql per django query api.
  • Proposta: Hauria de ser un submenú d'aula (i no un menú principal)

@xeviterr
Copy link
Collaborator Author

xeviterr commented Jul 16, 2019 via email

@xeviterr
Copy link
Collaborator Author

xeviterr commented Jul 16, 2019 via email

@xeviterr
Copy link
Collaborator Author

He passat la consulta a query API, a veure si ara funciona amb postres.

Proposta: Hauria de ser un submenú d'aula (i no un menú principal), això queda pendent.... a veure si demà....

@xeviterr
Copy link
Collaborator Author

Mira m'he animat, també ho he fet.

@xeviterr
Copy link
Collaborator Author

S'hauria de mirar la funció de dins views.py:

def modificaEstatControlAssistencia(request, codiEstat, idAlumne, idImpartir)

Basicament el que fa la funció és passar llista.

He create el codi a base de mirar com ho fa el mòdul de presència, però al final duplico certes parts de codi, potser seria més interessant poder compartir el codi de passar llista entre el mòdul de presència i el de presència setmanal.

No m'atreveixo a tocar el mòdul de presència i tampoc sé on posar el codi compartit.

S'accepten propostes.

@ctrl-alt-d
Copy link
Owner

Proposta: Hauria de ser un submenú d'aula (i no un menú principal), això queda pendent.... a veure si demà....

Perfecte!

Selection_583

He passat la consulta a query API, a veure si ara funciona amb postgres.

Molt rebé. Crec que ja no necessites convertDateToDjangoDate ni tampoc el .isoformat(). Les dates són dates.

potser seria més interessant poder compartir el codi de passar llista entre el mòdul de presència i el de presència setmanal.

També tenim el "Justificator" per als tutors, que potser s'assembla més, que passar llista, a la pantalla que has fet.

Per anar al "Justificator" has d'entrar amb un professor que sigui tutor i anar l'opció "Justificar", quan et pregunta l'alumne, deixes l'opció "Justificador", d'aquesta manera apareixen tots els alumnes:

Selection_584

Crec que és bo que ho integrem per poder unificar el codi.

@xeviterr
Copy link
Collaborator Author

xeviterr commented Jul 17, 2019 via email

@xeviterr
Copy link
Collaborator Author

De fet el justificador només modifica un control assistència.

El mòdul de presència setmanal permet passar llista i per tant modifica el profe de l'objecte a impartir. Seria més similar al mòdul presència.

Hi ha trossos de codi compartits, provo de unificar i em dones el vist i plau o ho intentes tu....

o no fem res de moment....

@ctrl-alt-d
Copy link
Owner

De fet el justificador només modifica un control assistència. El mòdul de presència setmanal permet passar llista i per tant modifica el profe de l'objecte a impartir. Seria més similar al mòdul presència.

És cert, només s'assemblen en la UI / UX.

Hi ha trossos de codi compartits, provo de unificar i em dones el vist i plau o ho intentes tu.... o no fem res de moment....

Nota previa: Ideal que les regles de negoci estiguin associades als models i no als processos ( views ). És la manera de garantir que s'aplicaran els mateixos criteris a tot arreu.

Per mi ok a que facis PR amb aquesta millora. Jo no ho puc fer ara :( les hores que hi dedico són més per donar suport que per fer manteniment/millores. En @juaky durant el curs sí que farà aquestes tasques crec.

@ctrl-alt-d ctrl-alt-d self-requested a review July 18, 2019 08:28
@ctrl-alt-d
Copy link
Owner

Per mi tot està correcte, les vistes requereixen que l'usuari pertanyi al grup de professors, es pot deshabilitar mitjançant paràmetre custom (no afecta instal·lacions que no necessitin la pantalla), no cal esperar més, faig el merge. Gràcies per l'aportació!

@ctrl-alt-d ctrl-alt-d merged commit 6fd910b into master Jul 25, 2019
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.

None yet

2 participants