Skip to content

Show if song is in playlist#137

Open
p-garence wants to merge 4 commits into
ghenry22:masterfrom
p-garence:show-if-song-is-in-playlist
Open

Show if song is in playlist#137
p-garence wants to merge 4 commits into
ghenry22:masterfrom
p-garence:show-if-song-is-in-playlist

Conversation

@p-garence
Copy link
Copy Markdown

Summary

This is a feature proposal.

The feature is to see which playlists a song already belongs to before adding it again. This helps users manage duplicates and organize their library more effectively.

This feature is OFF by default to minimize network overhead for users with large libraries.

Changes

  • I18n: Added translation entries for the new setting and display labels.
  • Library Store: Added fetchAllPlaylistsWithSongs to PlaylistLibraryStore. This method fetches full details for all playlists to build a local map of song IDs.
  • Settings: Added "Show if song is in playlist" option under Appearance and Layout.
  • UI:
    • Updated AddToPlaylistSheet to conditionally fetch detailed playlist data.
    • Implemented numberOfMatch to determine number of times a song is in a playlist.
    • Added visual indicators in the AddToPlaylist menu showing the occurrence count.
  • Test: Added a test for fetchAllPlaylistsWithSongs.

## Concerns

  • All the translations are in english. Should the translations be automatically translated or left for later translation ?

  • Did not fully understand the 80% + coverage requirements for the tests, is the added test enough ?

Testing

  • [ X] npx tsc --noEmit passes
  • [ X] npx jest --no-coverage passes
  • [ X] New/updated tests added (if applicable)
  • Tested on iOS
  • [ X] Tested on Android

Related Issues

Closes N/A (Feature Request)

@ghenry22
Copy link
Copy Markdown
Owner

you can run the unit tests with coverage to see the test coverage and identify any areas that are under the 80% thresholds. If under then you will need to expand test coverage.

Have you considered the handling off offlinemode in this implementation? A lot of this functionality depends on the server being available so we need to take into account when the app is in offlineMode and adjust the options and data that we show accordingly.

translations: you need to complete the base english translations, additional languages will be populated through crowdin afterwards. Adding the english base values will trigger CI to push the changes to crowdin which will then generate the keys for other languages.

Where is this visible in the UI? Can you provide some screenshots from your test environment to show visually any new UI surface, this will help with evaluation.

Thank you for contributing!

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.

2 participants