[DONE] Fix the bug in the playlist application #1000

SebastienCozeDev · 2023-10-30T12:10:51Z

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].

Bug fixes

Fix the playlist sorting.
Fix the filters on the page listing the playlists.
Fix the auto-play.
Fix the error 500 for AnonymousUser in playlist.
Fix enriched videos in playlists.

gitguardian · 2023-11-09T13:58:25Z

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.

^{_{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!}}

ptitloup · 2023-11-09T15:05:50Z

Ok pour moi niveau fonctionnel - RAS

AymericJak

Pas testé, mais quelques remarques à apporter :)

pod/enrichment/templates/enrichment/edit_enrichment.html

pod/playlist/utils.py

pod/video/models.py

pod/video/templates/videos/video_aside.html

Badatos

quelques remarques ci-dessous

.gitguardian.yml

pod/enrichment/templates/enrichment/edit_enrichment.html

Badatos · 2023-11-09T14:56:00Z

pod/enrichment/templates/enrichment/video_enrichment_more_script.html

@@ -0,0 +1,6 @@
+{% load i18n %}


pas besoin d'i18n a priori sur cette page ^^

On utilise directement ce template pour remplacer la page en AJAX. C'est fait dans la vue get_video de l'app playlist.

pod/enrichment/templates/enrichment/video_enrichment_page_aside.html

pod/enrichment/tests/test_views.py

pod/video/templates/videos/video_aside.html

…card_aside.html'

…aside

pod/enrichment/templates/enrichment/video_enrichment_page_aside.html

ptitloup · 2023-11-10T09:47:14Z

pod/video/templates/videos/video_page_aside.html

-  <div class="card-body card-text text-center">
-    {% include "videos/link_video.html" with hide_favorite_link=True %}
-  </div>
+<div class="card" id="card-enrichment-informations" style="display: none;">


On pourrait regarder implement si enrichment est dans l'URL courant, fait un truc du genre :

{% if "enrichment" in request.URL %} {% include information %}{% endif %}

pod/enrichment/templates/enrichment/edit_enrichment.html

pod/enrichment/templates/enrichment/group_enrichment.html

pod/enrichment/tests/test_views.py

pod/enrichment/views.py

pod/playlist/utils.py

pod/video/models.py

pod/video/templates/videos/video_edit.html

SebastienCozeDev · 2023-11-10T10:06:33Z

Est-ce possible de tester en le retirant ?

C'est effectivement fonctionnel pour ce template

On pourrait regarder implement si enrichment est dans l'URL courant, fait un truc du genre :
{% if "enrichment" in request.URL %} {% include information %}{% endif %}

On a besoin de la div pour pouvoir refresh à chaque vidéo :

const idElements = [
    'card-manage-video',
    'card-takenote',
    'card-share',
    'card-disciplines',
    'card-types',
]
for (let id of idElements) {
    if (document.getElementById(id)) refreshElementWithDocumentFragment(`#${id}`, pageAside);
}

@ptitloup

pod/enrichment/tests/test_views.py

ptitloup · 2023-11-10T10:39:36Z

pod/video/templates/videos/video_page_aside.html

-  <div class="card-body card-text text-center">
-    {% include "videos/link_video.html" with hide_favorite_link=True %}
-  </div>
+<div class="card" id="card-enrichment-informations" style="display: none;">


as-tu tester ce point ?

AymericJak

Good !

AymericJak · 2023-11-10T10:45:35Z

Ah non, un test ne passe pas.

Corrige comme suit pour le type de retour :

from typing import Union

def function() -> Union[str, None]:

SebastienCozeDev · 2023-11-10T11:24:20Z

as-tu tester ce point ?

Oui

@ptitloup

SebastienCozeDev · 2023-11-10T12:44:00Z

On pourrait regarder implement si enrichment est dans l'URL courant, fait un truc du genre :
{% if "enrichment" in request.URL %} {% include information %}{% endif %}

On a besoin de la div des deux côtés. Sinon, on aura aucun moyen de l'afficher pour les vidéos enriches lors de la lecture de la playlist.

@ptitloup

ptitloup

ok pour moi, le point sur l'inclusion des templates en cas de vidéos enrichies reste un mystère mais qui n'enleve en rien la quelité de cette contribution. Merci bcp

* Fix the playlists sorting * 🐛 Fix the autoplay * 🐛 Fix enriched videos in playlists * 🎨 Fix flake8 * 🐛 Fix the error 500 for AnonymousUser in playlist * 🐛 Fix enriched videos in playlists * 🐛 Fix the numberPlaylists condition * 🔒 Fix GitGuardian * 🎨 Add line at the end file * 🎨 Replace 'var' by 'let' when it is possible * 🎨 Rename illegible identifier * Add aria-hidden attribute * 📝 Add JSDocs * 📝 Add PyDocs * 🎨 Improve condition * Create a real request error * Rename 'enrichmentinformations_card.html to 'enrichment_informations_card_aside.html' * 🎨 Fix flake8 * 🐛 Replace enrichmentinformations_card by erichment_informations_card_aside * Remove unused load * Re-add aria-hidden attributes * Update the quotation marks * 📝 Update PyDoc * 🐛 Fix coverage

Fix the playlists sorting

4014f9d

SebastienCozeDev added the bug Something isn't working label Oct 30, 2023

SebastienCozeDev self-assigned this Oct 30, 2023

This was linked to issues Oct 30, 2023

Listes de lecture : tri #996

Closed

Vidéos enrichies dans les listes de lecture #998

Closed

Listes de lecture : lecture automatique #997

Closed

SebastienCozeDev added 4 commits October 30, 2023 15:00

🐛 Fix the autoplay

02634b2

🐛 Fix enriched videos in playlists

247e22d

🎨 Fix flake8

2637772

🐛 Fix the error 500 for AnonymousUser in playlist

f28fab4

SebastienCozeDev force-pushed the SebastienCozeDev/fix_playlist_app branch from ed1d9f4 to f28fab4 Compare November 7, 2023 10:09

🐛 Fix enriched videos in playlists

b4fba49

SebastienCozeDev marked this pull request as ready for review November 9, 2023 12:57

SebastienCozeDev added 2 commits November 9, 2023 14:02

Merge develop

1fb0d94

🐛 Fix the numberPlaylists condition

7afe706

SebastienCozeDev force-pushed the SebastienCozeDev/fix_playlist_app branch 3 times, most recently from 27b76bf to 7afe706 Compare November 9, 2023 14:24

🔒 Fix GitGuardian

22ced78

SebastienCozeDev changed the title ~~[WIP] Fix the bug in the playlist application~~ [DONE] Fix the bug in the playlist application Nov 9, 2023

🎨 Add line at the end file

f33f86f

SebastienCozeDev requested review from Badatos, ptitloup and AymericJak November 9, 2023 14:44

AymericJak suggested changes Nov 9, 2023

View reviewed changes

Badatos requested changes Nov 9, 2023

View reviewed changes

🎨 Replace 'var' by 'let' when it is possible

e54530b

SebastienCozeDev changed the title ~~[DONE] Fix the bug in the playlist application~~ [WIP] Fix the bug in the playlist application Nov 9, 2023

SebastienCozeDev added 4 commits November 10, 2023 10:02

Create a real request error

2e0d2b6

Rename 'enrichmentinformations_card.html to 'enrichment_informations_…

a0d7212

…card_aside.html'

🎨 Fix flake8

d8a6e29

🐛 Replace enrichmentinformations_card by erichment_informations_card_…

702a1aa

…aside

SebastienCozeDev changed the title ~~[WIP] Fix the bug in the playlist application~~ [DONE] Fix the bug in the playlist application Nov 10, 2023

SebastienCozeDev requested review from AymericJak, ptitloup and Badatos November 10, 2023 09:29

ptitloup requested changes Nov 10, 2023

View reviewed changes

AymericJak suggested changes Nov 10, 2023

View reviewed changes

SebastienCozeDev added 3 commits November 10, 2023 11:07

Remove unused load

de7ec67

Re-add aria-hidden attributes

64ac6aa

Update the quotation marks

d640ebc

Badatos reviewed Nov 10, 2023

View reviewed changes

pod/enrichment/tests/test_views.py Show resolved Hide resolved

📝 Update PyDoc

c640fdc

SebastienCozeDev requested review from Badatos, AymericJak and ptitloup November 10, 2023 10:37

ptitloup requested changes Nov 10, 2023

View reviewed changes

AymericJak approved these changes Nov 10, 2023

View reviewed changes

🐛 Fix coverage

d7f23f0

SebastienCozeDev requested a review from ptitloup November 10, 2023 12:51

ptitloup approved these changes Nov 10, 2023

View reviewed changes

Badatos approved these changes Nov 10, 2023

View reviewed changes

ptitloup merged commit 3040f59 into EsupPortail:develop Nov 10, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DONE] Fix the bug in the playlist application #1000

[DONE] Fix the bug in the playlist application #1000

SebastienCozeDev commented Oct 30, 2023 •

edited

Loading

gitguardian bot commented Nov 9, 2023 •

edited

Loading

ptitloup commented Nov 9, 2023

AymericJak left a comment •

edited

Loading

Badatos left a comment

Badatos Nov 9, 2023

SebastienCozeDev Nov 10, 2023

ptitloup Nov 10, 2023

SebastienCozeDev commented Nov 10, 2023 •

edited

Loading

ptitloup Nov 10, 2023

AymericJak left a comment

AymericJak commented Nov 10, 2023

SebastienCozeDev commented Nov 10, 2023 •

edited

Loading

SebastienCozeDev commented Nov 10, 2023

ptitloup left a comment

[DONE] Fix the bug in the playlist application #1000

[DONE] Fix the bug in the playlist application #1000

Conversation

SebastienCozeDev commented Oct 30, 2023 • edited Loading

Bug fixes

gitguardian bot commented Nov 9, 2023 • edited Loading

️✅ There are no secrets present in this pull request anymore.

ptitloup commented Nov 9, 2023

AymericJak left a comment • edited Loading

Choose a reason for hiding this comment

Badatos left a comment

Choose a reason for hiding this comment

Badatos Nov 9, 2023

Choose a reason for hiding this comment

SebastienCozeDev Nov 10, 2023

Choose a reason for hiding this comment

ptitloup Nov 10, 2023

Choose a reason for hiding this comment

SebastienCozeDev commented Nov 10, 2023 • edited Loading

ptitloup Nov 10, 2023

Choose a reason for hiding this comment

AymericJak left a comment

Choose a reason for hiding this comment

AymericJak commented Nov 10, 2023

SebastienCozeDev commented Nov 10, 2023 • edited Loading

SebastienCozeDev commented Nov 10, 2023

ptitloup left a comment

Choose a reason for hiding this comment

SebastienCozeDev commented Oct 30, 2023 •

edited

Loading

gitguardian bot commented Nov 9, 2023 •

edited

Loading

AymericJak left a comment •

edited

Loading

SebastienCozeDev commented Nov 10, 2023 •

edited

Loading

SebastienCozeDev commented Nov 10, 2023 •

edited

Loading