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

Drag'n'Drop Analysis #37

Merged
merged 5 commits into from Jul 10, 2013
Merged

Drag'n'Drop Analysis #37

merged 5 commits into from Jul 10, 2013

Conversation

kain88-de
Copy link
Member

https://bugs.launchpad.net/mixxx/+bug/1197634

This branch enables to drag'n'drop songs on the analysis view and it is also possible to analysie entire crates/playlists from the submenu. In addition I also renamed all the prepare stuff to analyse so that it is easier to see what these classes are actually used for.

Max Linke added 4 commits July 6, 2013 17:56
If you are new to the code the name prepare for everything that is
related to the analysisview can be confusing. Renaming them will make
it easier to see what the classes are being used for
this path also does some minor code refactoring
@@ -86,6 +82,23 @@ QString PlaylistDAO::getPlaylistName(int playlistId)
return name;
}

QList<int> PlaylistDAO::getTrackIds(int playlistId) {
QSqlQuery query(m_database);
query.prepare("SELECT track_id from PlaylistTracks WHERE playlist_id = :id");
Copy link
Member

Choose a reason for hiding this comment

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

I would do a SELECT DISTINCT because there could be duplicate tracks (even though analyzing twice is a no-op).

@kain88-de
Copy link
Member Author

I made the changes rryan proposed

@daschuer
Copy link
Member

daschuer commented Jul 9, 2013

Thank you @kain88-de! I have just tested your changes and they work fine.
But there is still a the problem of no user feedback about the analysis queue. The user does not instantly get a feedback when he uses the new context menu entry or the drop to analysis. After changing the view fast enough to analyzer he might can spot the rest of the issued analysis. But the Filtered view will not change.
I am not sure if a atomatic view change to analyzis well be good ore worse ..

Anyhow, your branch is a fine bugfix for the original issue, so lets merge!

@rryan
Copy link
Member

rryan commented Jul 9, 2013

The reason all this code is called prepare is because it's unfinished and all it did in V1 of the view was analyze tracks.

@asantoni and I envisioned it being a place to go to be a one-stop shop for analyzing, cleaning, and categorizing your library. This would include things like dragging tracks to a crate delegate (the dead code you deleted) for rapid tagging of music, fixing artist/album/genre tags (e.g. normalizing from "DNB", "DrumNBass", "Drum and Bass" to "Drum & Bass") etc.

I'd rather we keep it named "Prepare" so that someday when we work on adding those features we won't have to rename it back but since you've already done the work of renaming it we can wait until we add those features to rename it again.

@rryan
Copy link
Member

rryan commented Jul 9, 2013

Thanks for the changes -- LGTM

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.

None yet

3 participants