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] WebTV features #1233

Merged
merged 29 commits into from
Jan 22, 2025
Merged

[DONE] WebTV features #1233

merged 29 commits into from
Jan 22, 2025

Conversation

Naihl
Copy link
Collaborator

@Naihl Naihl commented Dec 4, 2024

Before sending your pull request, make sure the following are done

  • You have read our contribution guidelines.
  • Your PR targets the develop branch.
  • The title of your PR starts with [WIP] or [DONE].

Speakers

  • Make "first name" optional
  • Separate homonyms and add multi-role options (roles selection for the current video)
  • Change in the way to display the participants and their roles.

Duplicate

  • Add the form duplication feature

@Badatos
Copy link
Collaborator

Badatos commented Dec 5, 2024

Merci Pour cette première PR
Il faudra résoudre les conflits sur les fichiers de traduction (.po/mo)
Pour générer les fichiers .po à partir du code source, lance la commande make lang édite ensuite les fichiers .po avec un outil comme poEdit (qui compile automatiquement en .mo) ou avec un simple éditeur de texte, et en lancant ensuite la commande django compilemessages

Ensuite, dès que la PR sera finalisée et prête à une revue de code, modifie le préfixe "WIP" (Work In Progress) en DONE

@Badatos
Copy link
Collaborator

Badatos commented Dec 5, 2024

Je vois que ta PR s'intitule "WebTV features". Comptes-tu mettre d'autres fonctionnalité que la duplication dans cette PR ?
Je pense préférable de granulariser au maximum, et de ne mettre que la duplication dans cette PR, et faire d'autres PR pour chaque fonctionnalité.

@Naihl
Copy link
Collaborator Author

Naihl commented Dec 5, 2024

Je vois que ta PR s'intitule "WebTV features". Comptes-tu mettre d'autres fonctionnalité que la duplication dans cette PR ? Je pense préférable de granulariser au maximum, et de ne mettre que la duplication dans cette PR, et faire d'autres PR pour chaque fonctionnalité.

Je l'ai appelée ainsi car elle possède également une modification sur les intervenants que j'avais réalisé en même temps que la duplication avant d'avoir accès aux PR Pod. Les prochaines contiendront une fonctionnalité unique

@Naihl Naihl changed the title [WIP] WebTV features [DONE] WebTV features Dec 6, 2024
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.

Plus que 2 petites remarques, et les conflits à résoudre sur les fichiers .po/.mo

@LoicBonavent LoicBonavent self-requested a review December 12, 2024 15:20
Copy link
Collaborator

@LoicBonavent LoicBonavent left a comment

Choose a reason for hiding this comment

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

Bonjour,
Un grand merci pour cette PR.
Côté code, c'est nickel pour ma part, mais j'ai quelques retours sur mes tests :

  • Serait-il possible d'ajouter une confirmation lors d'un clic sur l'action de dupliquer ?
  • La duplication ne semble pas copier les propriétaires additionnels. Je pense que c'est important de les conserver également.
  • Perso, je modifierai bien le libellé "Dupliquer la fiche" en "Dupliquer la vidéo"
  • Serait-il possible d'avoir, dans le nouveau titre, une traduction de Copy of (dans le slug, pas de soucis) ?
  • Pour mes tests dans un environnement de test local, la vidéo dupliquée ne s'encode pas, et j'obtiens le message "La vidéo est actuellement en attente d’encodage.". Cela vient de mon env de test ?

Encore merci

@Badatos
Copy link
Collaborator

Badatos commented Jan 6, 2025

Attention, il faut que tout le code passe par Flake8.

Voici la liste des problèmes signalés :

./pod/duplicate/views.py:9:1: E302 expected 2 blank lines, found 1
./pod/duplicate/views.py:54:14: E251 unexpected spaces around keyword / parameter equals
./pod/duplicate/views.py:54:16: E251 unexpected spaces around keyword / parameter equals
./pod/duplicate/views.py:55:13: E251 unexpected spaces around keyword / parameter equals
./pod/duplicate/views.py:55:15: E251 unexpected spaces around keyword / parameter equals
./pod/speaker/context_processors.py:9:1: E302 expected 2 blank lines, found 1
./pod/speaker/forms.py:9:1: F401 'pod.main.context_processors.WEBTV_MODE' imported but unused
./pod/speaker/views.py:17:1: F401 'pod.main.context_processors.WEBTV_MODE' imported but unused
./pod/speaker/views.py:199:8: F821 undefined name 'REQUIRED_SPEAKER_FIRSTNAME'

@Badatos
Copy link
Collaborator

Badatos commented Jan 6, 2025

@Badatos Une idée de comment résoudre les conflits de fichiers de traduction ? J'ai pourtant réalisé les commandes de compilation mais je n'arrive pas à résoudre ces conflits

oui : le plus simple est de procéder ainsi :

  1. résoudre d'abord tous les conflits en prenant les fichiers .po/.mo du dépot
  2. Ensuite, regénérer les fichiers .po via make lang
  3. Appliquer d'éventuelles modifications perdues lors du conflit

@Badatos
Copy link
Collaborator

Badatos commented Jan 6, 2025

Attention, les tests unitaires génèrent 4 erreurs (voir ici : https://github.com/EsupPortail/Esup-Pod/actions/runs/12633328698/job/35198720218 )

Exemple d'erreur : "AttributeError: 'Settings' object has no attribute 'REQUIRED_SPEAKER_FIRSTNAME'" dans le `test_speaker_management_superuser_get

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.

Reste quelques changements à apporter aux traductions

@Naihl
Copy link
Collaborator Author

Naihl commented Jan 10, 2025

J'ai mis à jour la traduction, par contre les conflits de fichiers refont surface..

@Badatos
Copy link
Collaborator

Badatos commented Jan 10, 2025

J'ai mis à jour la traduction, par contre les conflits de fichiers refont surface..

commence toujours par résoudre les conflits avant de faire des modifs, ca évite de devoir refaire les choses encore après la résolution ^^

@Badatos
Copy link
Collaborator

Badatos commented Jan 22, 2025

@LoicBonavent , est-ce que tu peux retester avec les dernière modifications de @Naihl stp ?

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 toutes les corrections ;)

Copy link
Collaborator

@LoicBonavent LoicBonavent left a comment

Choose a reason for hiding this comment

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

Super, merci beaucoup pour tout ce travail !

@Badatos Badatos merged commit 98c2e1d into EsupPortail:develop Jan 22, 2025
3 checks passed
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