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

#239: Only allow audio files for song uploading #240

Conversation

perig99
Copy link
Contributor

@perig99 perig99 commented Oct 3, 2024

Description

  • Updated the file upload logic to validate audio files more accurately.
  • Updated unit test for the AddSongPlayListAccordion component to validate file input handling.

Commit type

  • feat: Only allow audio files for song uploading

Issue

Issue #239

Solution

This pull request resolves the problem by improving the validation and handling of file uploads, specifically for audio files:

  • by changing attribute to "audio/*"
  • by handling unsupported files in handleChangeFile

Copy link
Owner

@AntonioMrtz AntonioMrtz left a comment

Choose a reason for hiding this comment

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

The code is great :) . Thanks for sharing your time and following our conventions and workflows. I did write a few minor improvements that can be done. Let me know if you need anything. Backend pipeline is failing because of an enviroment value only provided in the main repo so don't worry about it.

@AntonioMrtz AntonioMrtz linked an issue Oct 4, 2024 that may be closed by this pull request
Copy link
Owner

@AntonioMrtz AntonioMrtz left a comment

Choose a reason for hiding this comment

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

LGTM :)

@AntonioMrtz
Copy link
Owner

AntonioMrtz commented Oct 5, 2024

Hi @perig99 , code looks good 👍. Thanks for sharing your time and efforts to contribute to the project. I hope too see you soon in the project, are you looking for other issues to contribute? After I merge I will add you in README and CONTRIBUTORS :) . If you find this project interesting/helpful, I'd appreciate it if you could star the repo!

@AntonioMrtz AntonioMrtz merged commit f3d28ac into AntonioMrtz:master Oct 5, 2024
@perig99
Copy link
Contributor Author

perig99 commented Oct 9, 2024

Hi @AntonioMrtz,
thanks for opportunity to take a part on this project. It was my first time experience to contribute on project like this and it was great experience for me. I am focusing on my Master thesis right now. So once, when I will have more free time, I would like to help. :)

@AntonioMrtz
Copy link
Owner

AntonioMrtz commented Oct 9, 2024

Hi @perig99 I wish you the best on your master thesis! . Feel free to drop by the project whenever you like. We'll be waiting :)

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.

Only allow audio files for song uploading
2 participants