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

YouTube + Music #401

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

YouTube + Music #401

wants to merge 15 commits into from

Conversation

Titiftw
Copy link

@Titiftw Titiftw commented Sep 25, 2024

Hi,
I found your project and I really enjoyed it. However, it was lacking few functionalities that I tried to add myself. Note that I am not a Python nor a Svelte expert but if it may help you, it includes Music upload and YouTube (that one might require some fanciness on the link input).
I'm missing software for the translations though.

If you think this can help you, feel free to use it. If not, then please discard that PR!

Kind regards,
Jason

JasonDavidI8C and others added 2 commits September 25, 2024 17:08
View video

Wrong if else

Issue iframe

Edit on the displayed video

Small fixes

Hide title, better button location

Trying to hide title as showinfo no longer works

Display video

Reverted

Music upload

File input not declared

Removed compressing on musics

Issue uploading music

Trying to display size & type

Images not there

Trying stuffs

Trying with different object

Reverted

Added type, corrected filename

Display differnt image if music

Extra }

Updated user storage used

Issue in loop

Management for music

Set music from Library

Test blo

Audio player

Wrong link

Trying this

Back to list

Listing stuffs which are deleted

Issue filtering

Wrong path set

Issue when adding a music to a quizz not being linked to it

Wrong var used

Debug

Debug

Debug

Blo

Debug

Magic

Reverted

Fix in calculate hash

I'm banana

Remove storage path

Calculate hash is bugged

Blo

Blo

Debug

Blo

Blo

Blo

Blo

Blo

Blo

Blo

Blo

Blo

Blo

Blo

Blo

Blo

Blo

Blo

Log

Log

Blo

Log

Log

Trying

Save item

Blo

Blo

Always delete button

Delete button always showed

Blo

Banana

Added player on admin

Fix player

UI fixes

Removed hardcoded blocorp
@Titiftw Titiftw marked this pull request as draft September 25, 2024 15:13
classquiz/db/models.py Outdated Show resolved Hide resolved
classquiz/helpers/__init__.py Outdated Show resolved Hide resolved
Comment on lines -130 to -135
for image in images_to_delete:
if image is not None:
try:
await storage.delete([image])
except DeletionFailedError:
pass
Copy link
Owner

Choose a reason for hiding this comment

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

Why remove that?

Copy link
Author

Choose a reason for hiding this comment

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

I don't see "images_to_delete" being populated. Am I wrong? It seems it's always set to []

Copy link
Owner

Choose a reason for hiding this comment

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

It should find the images that were in the quiz before it was saved and deletes them, but I'll check that

classquiz/worker/storage.py Outdated Show resolved Hide resolved
frontend/package-lock.json Outdated Show resolved Hide resolved
loading="lazy"
alt={image.alt_text || $t('file_dashboard.not_available')}
/>
{#if file.mime_type === 'music/mp3'}
Copy link
Owner

Choose a reason for hiding this comment

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

mime type oversight

Copy link
Author

Choose a reason for hiding this comment

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

What do you mean ?

alt={image.alt_text || $t('file_dashboard.not_available')}
/>
{#if file.mime_type === 'music/mp3'}
<svg
Copy link
Owner

Choose a reason for hiding this comment

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

svg source?

Copy link
Author

Choose a reason for hiding this comment

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

frontend/src/routes/edit/musics/+page.svelte Outdated Show resolved Hide resolved
});
});
xhr.open('POST', `/api/v1/storage/raw/${file.name}`, true);
xhr.setRequestHeader('Content-Type', 'music/mp3');
Copy link
Owner

Choose a reason for hiding this comment

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

Not always mp3

Copy link
Author

Choose a reason for hiding this comment

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

You're right but I don't have any other option (checking file extension?)

let file: File;
let file_data: undefined | Blob = undefined;

const compress_music = async () => {
Copy link
Owner

Choose a reason for hiding this comment

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

What's this purpose?

Copy link
Author

Choose a reason for hiding this comment

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

None, but I didn't manage and spend time to remove the way it used to work. So I kept the compression function if compression could be added.

@mawoka-myblock
Copy link
Owner

Thank you soo much for this PR! Really appreciate it! I've just looked through the code roughly and put some notes down. Please address them or comment on them and mark them as resolved afterwards. Another thing I haven't seen: Where's the YouTube player? maybe I just missed it. I'll also have to test this code locally. Again, many thanks for such a huge PR!

@Titiftw
Copy link
Author

Titiftw commented Sep 25, 2024

Owner

The YouTube player is done using iframe:
CleanShot 2024-09-25 at 17 35 47@2x
Once the link is added to the card, it gets displayed on it:
CleanShot 2024-09-25 at 17 37 29@2x
Might still be missing from the Admin card as I didn't test it fully as I realized that the YouTube video title would be showed to every body... Not the greatest idea for Blind tests.

@Titiftw
Copy link
Author

Titiftw commented Sep 25, 2024

@mawoka-myblock I'm having issue "unlinking" images/musics from a quiz. If I remove a music or an image from a quiz and it becomes unused in any quiz, the call "/storage/list" still shows it as part of a quiz which prevents the file delete button to appear. It seems that the storage worker line await new_quiz.storageitems.remove(item) always fails as the item UUID doesn't exist.
Any idea what's wrong?

@mawoka-myblock
Copy link
Owner

I'm gonna have to look into that when I got time. Expect the weekend in 3 days

@Titiftw Titiftw marked this pull request as ready for review September 25, 2024 19:45
@mawoka-myblock
Copy link
Owner

I haven't forgotten you yet!

@Titiftw
Copy link
Author

Titiftw commented Oct 7, 2024

No worries @mawoka-myblock, take your time! In the meantime, I also worked on another feature to fix several points which were blocking my tests:

  • When playing, there was a bug when a player would reconnect more than once which could potentially happen. I fixed it in my feature branch (now the player can refresh any time the game, as long as it is not answering a question).
  • When playing, the total score showed on the player screen was wrong if the player reconnected.
  • When playing, a player could give X answers if it would click X times on the "Validate" button. I adjusted the disabled button logic in the frontend and also return an "already_replied" on the WebSocket.
  • When playing on a CHECK question, the player could select multiple answers. I made sure all the selected answers were removed when clicking on another answer.
  • I also made few styles fixes (annoying scrollbar when playing for instance).

I'll create another PR if you don't mind!

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