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

[DONE] New 3.4.0 fixes #981

Merged
merged 5 commits into from
Oct 13, 2023

Conversation

LoicBonavent
Copy link
Collaborator

@LoicBonavent LoicBonavent commented Oct 12, 2023

Fixes 3 bugs for 3.4.0 version:

  • Bugfix for create a thumbnail.
    To check, execute a command like this: "python manage.py create_thumbnail id"

  • Fixed bug preventing change of pages or number of lines in statistics interface
    To check, on a statistics page (/stats_view/), try to change the number of lines or page. Without this bugfix, you can see an "data.filter is not a function" error in the browser console and an icon Loading, and no changes.

  • Bugfix for issue [BUG] Erreur champs owner lors de l'ajout d'une vidéo #948 (An error appears in the form for adding a video, for a super admin, for the owner attribute).
    To check, with a super admin user, upload a video file, wait for encoding to finish (while still on the editing page) and save. Without this bugfix, you can see an "owner - this field is required" error.

Copy link
Collaborator

@Badatos Badatos left a comment

Choose a reason for hiding this comment

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

Merci pour ces corrections !
Y'a que pour la correction JS que je mets une remarque.

pod/video/static/js/video_stats_view.js Show resolved Hide resolved
pod/video/static/js/video_stats_view.js Outdated Show resolved Hide resolved
Copy link
Contributor

@AymericJak AymericJak left a comment

Choose a reason for hiding this comment

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

Le code me semble correct !

Il me reste à tester en local. Je ferai ça un peu plus tard :)

@AymericJak AymericJak added the bug Something isn't working label Oct 12, 2023
Copy link
Contributor

@AymericJak AymericJak left a comment

Choose a reason for hiding this comment

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

  • L'upload des vidéos en mode super admin est maintenant bien corrigé !
  • Quand je vais sur la page des statistiques, je constate une autre erreur dans la console :
    image
  • Pour ce qui est du reste, c'est tout bon pour moi :)

@SebastienCozeDev SebastienCozeDev self-requested a review October 13, 2023 08:18
@LoicBonavent
Copy link
Collaborator Author

Sur le coup, je n'ai pas touché au fichier webpush.js. Il concerne les notifications : je ne vois pas trop son utilité pour les stats...

@AymericJak
Copy link
Contributor

Yess, je sais bien que tu n'y a pas touché, mais comme tu fais du bug fix de la 3.4.0, je trouve ça intéressant de le corriger :)

Je suis du même avis, je ne vois pas trop l'utilité des notifications pour les stats :)

@LoicBonavent
Copy link
Collaborator Author

On voit que tu bosses avec Nicolas :)
Ok, je regarde cela cet après-midi

Copy link
Contributor

@SebastienCozeDev SebastienCozeDev left a comment

Choose a reason for hiding this comment

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

Merci pour cette PR !

J'ai pu remarquer un petit soucis avec le thème noir, le champ est en fond blanc alors qu'habituellement, il est sur fond foncé.

image

image

Il n'y a également pas de texte informatif sur le champ.

pod/video/static/js/video_stats_view.js Show resolved Hide resolved
pod/video/static/js/video_stats_view.js Show resolved Hide resolved
@AymericJak
Copy link
Contributor

En fait @SebastienCozeDev, pour le JQuery de la partie Stats, cela a été laissé ainsi puisqu'une PR sur les stats est en cours.

Autant éviter de faire 2 fois le même boulot, puisque JQuery a totalement été retiré dans la PR Stats.

@LoicBonavent
Copy link
Collaborator Author

On voit que tu bosses avec Nicolas :) Ok, je regarde cela cet après-midi

Concernant l'erreur Javascript non bloquante, il faudrait modifier le fichier webpush.js, qui est fourni par le module django-webpush, pour gérer le fait que subBtn peut être null. A voir si l'on veut par la suite contribuer à ce projet... Pour l'instant, je préfère ne pas toucher.

@LoicBonavent
Copy link
Collaborator Author

Merci pour cette PR !

J'ai pu remarquer un petit soucis avec le thème noir, le champ est en fond blanc alors qu'habituellement, il est sur fond foncé.

image

image

Il n'y a également pas de texte informatif sur le champ.

Le texte informatif provient du modèle; il y a pas mal de champs qui n'ont pas de texte informatif (disciplines, licence, date d'ajout...). Ce n'est pas trop utile quand c'est clair. Bref, je n'en ai pas ajouté pour Propriétaire.
Par contre, j'ai modifié le fichier dark.css pour tenir compte de l'autre point : cela venait du style de select2 pour le cas single et non multiple. J'en ai aussi profité pour modifier le fond lors de l'affichage des résultats d'une recherche dans un champ select2.
Je pense que c'est tout bon maintenant :)

Copy link
Contributor

@AymericJak AymericJak left a comment

Choose a reason for hiding this comment

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

Ok, merci à toi pour ta PR.

Bonne fin de journée, et bon week-end :)

@ptitloup ptitloup merged commit c79866f into EsupPortail:develop Oct 13, 2023
4 checks passed
vsabatie pushed a commit to vsabatie/Pod that referenced this pull request Nov 22, 2023
* Bugfix for create a thumbnail

* Fixed bug preventing change of pages or number of lines in interface

* Bugfix for issue EsupPortail#948 (An error appears in the form for adding a video, for a super admin, for the owner attribute)

* Adding comments and using getElementById

* Manage the background and color of select2 single field, for the dark mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants