Skip to content

Code Review Comments (Josh Queja) #32

@jqueja

Description

@jqueja

Certainly! Here's your feedback with numbering and bolding:

  1. Song Removal in songs.py:

    • The remove_song function in songs.py currently removes a song from the available options but not from playlists that added it before. Consider adding functionality to remove it from playlists as well to maintain consistency.
  2. Validating Links in songs.py:

    • In the add_song function, which takes a link as a parameter, it's crucial to implement a validation mechanism to ensure that the link is valid and leads to the intended platform. This can help prevent issues with incorrect or malicious links.
  3. Removing the 'platform' Column in users.py:

    • Eliminating the 'platform' column in users.py could align with the product mission. Since certain songs are exclusive to specific platforms, consider handling platform-specific logic elsewhere to enhance simplicity.
  4. user_playlist_position Purpose:

    • Clarify the purpose of the user_playlist_position table in your database schema. Understanding its role is essential for maintaining and evolving the codebase.
  5. Security Concerns in create_user:

    • Express concern about security when creating a user, as the password is visible in the request URL. Suggest exploring secure methods like using HTTPS and ensuring sensitive information is not exposed in the URL.
  6. Password Hashing in create_user:

    • Consider storing passwords as hashes instead of using a salt. This enhances security by protecting user passwords even if the database is compromised.
  7. Valid User Requirement for Adding Songs:

    • Clarify whether being a valid user is a prerequisite for adding songs to the playlist. If so, enforce appropriate user validation checks in the code.
  8. Handling Playlists on User Deletion:

    • Discuss the decision to either delete or save playlists associated with a user when they delete their profile. The choice depends on your product's user experience goals.
  9. Error Handling and 'one_or_none()':

    • Implement error checking, especially in places where the code may fail. The use of 'one_or_none()' is a good practice, but ensure it's complemented by robust error handling mechanisms.
  10. Documentation on Playlist Platform Consistency:

    • Consider adding documentation explaining that if songs in a playlist are from different platforms, there might be skipping or handling based on the primary platform.
  11. Readable Queries and Testing:

    • Great readable queries that have the ability to be tested and decoupled.
  12. Positive Outlook:

    • Overall great code and great vision for the project. This is just the start of the project and I can’t wait til yall get everything set and ready :)

Your attention to these considerations will contribute to the continued improvement and success of your project. Best of luck with its development!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions