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

Fix the track order during drag and drop Lp1829601 #2237

Merged
merged 13 commits into from
Oct 22, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add new functions resolveTrackIdsFromUrls() and resolveTrackIdsFromLo…
…cations() to remove duplicates
  • Loading branch information
daschuer committed Sep 28, 2019
commit 29167403b0e930dde61c2cbe99eab20ad5d2b833
7 changes: 2 additions & 5 deletions src/library/analysisfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,8 @@ void AnalysisFeature::cleanupAnalyzer() {
}

bool AnalysisFeature::dropAccept(QList<QUrl> urls, QObject* pSource) {
Q_UNUSED(pSource);
QList<QFileInfo> files = DragAndDropHelper::supportedTracksFromUrls(urls, false, true);
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(files,
TrackDAO::ResolveTrackIdOption::UnhideHidden
| TrackDAO::ResolveTrackIdOption::AddMissing);
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIdsFromUrls(urls,
!pSource);
analyzeTracks(trackIds);
return trackIds.size() > 0;
}
Expand Down
12 changes: 2 additions & 10 deletions src/library/autodj/autodjfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,21 +138,13 @@ void AutoDJFeature::activate() {
}

bool AutoDJFeature::dropAccept(QList<QUrl> urls, QObject* pSource) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The dropAccept()/dropAcceptChild() contain a lot of duplicated code (AutoDJ/Crates/Playlist). Is It possible to extract this code into a function?

QList<QFileInfo> files = DragAndDropHelper::supportedTracksFromUrls(urls, false, true);
if (!files.size()) {
return false;
}

// If a track is dropped onto the Auto DJ tree node, but the track isn't in the
// library, then add the track to the library before adding it to the
// Auto DJ playlist.
// pSource != nullptr it is a drop from inside Mixxx and indicates all
// tracks already in the DB
TrackDAO::ResolveTrackIdOptions options = TrackDAO::ResolveTrackIdOption::UnhideHidden;
if (pSource == nullptr) {
options |= TrackDAO::ResolveTrackIdOption::AddMissing;
}
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(files, options);
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIdsFromUrls(urls,
!pSource);
if (!trackIds.size()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

isEmpty()

Copy link
Contributor

Choose a reason for hiding this comment

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

ping

return false;
}
Expand Down
12 changes: 2 additions & 10 deletions src/library/crate/cratefeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,21 +203,13 @@ bool CrateFeature::dropAcceptChild(const QModelIndex& index, QList<QUrl> urls,
VERIFY_OR_DEBUG_ASSERT(crateId.isValid()) {
return false;
}
QList<QFileInfo> files = DragAndDropHelper::supportedTracksFromUrls(urls, false, true);
if (!files.size()) {
return false;
}

// If a track is dropped onto a crate's name, but the track isn't in the
// library, then add the track to the library before adding it to the
// playlist.
// pSource != nullptr it is a drop from inside Mixxx and indicates all
// tracks already in the DB
TrackDAO::ResolveTrackIdOptions options = TrackDAO::ResolveTrackIdOption::UnhideHidden;
if (pSource == nullptr) {
options |= TrackDAO::ResolveTrackIdOption::AddMissing;
}
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(files, options);
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIdsFromUrls(urls,
!pSource);
if (!trackIds.size()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

isEmpty()

Copy link
Contributor

Choose a reason for hiding this comment

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

ping

return false;
}
Expand Down
11 changes: 2 additions & 9 deletions src/library/crate/cratetablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,8 @@ int CrateTableModel::addTracks(const QModelIndex& index,
Q_UNUSED(index);
// If a track is dropped but it isn't in the library, then add it because
// the user probably dropped a file from outside Mixxx into this crate.
QList<QFileInfo> fileInfoList;
foreach(QString fileLocation, locations) {
QFileInfo fileInfo(fileLocation);
fileInfoList.append(fileInfo);
}

QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(fileInfoList,
TrackDAO::ResolveTrackIdOption::UnhideHidden
| TrackDAO::ResolveTrackIdOption::AddMissing);
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIdsFromLocations(
locations);
if (m_pTrackCollection->addCrateTracks(m_selectedCrate, trackIds)) {
select();
return trackIds.size();
Expand Down
9 changes: 2 additions & 7 deletions src/library/librarytablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,8 @@ void LibraryTableModel::setTableModel(int id) {
int LibraryTableModel::addTracks(const QModelIndex& index,
const QList<QString>& locations) {
Q_UNUSED(index);
QList<QFileInfo> fileInfoList;
foreach (QString fileLocation, locations) {
fileInfoList.append(QFileInfo(fileLocation));
}
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(fileInfoList,
TrackDAO::ResolveTrackIdOption::UnhideHidden
| TrackDAO::ResolveTrackIdOption::AddMissing);
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIdsFromLocations(
locations);
select();
return trackIds.size();
}
Expand Down
6 changes: 2 additions & 4 deletions src/library/mixxxlibraryfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,8 @@ bool MixxxLibraryFeature::dropAccept(QList<QUrl> urls, QObject* pSource) {
if (pSource) {
return false;
} else {
QList<QFileInfo> files = DragAndDropHelper::supportedTracksFromUrls(urls, false, true);
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(files,
TrackDAO::ResolveTrackIdOption::UnhideHidden
| TrackDAO::ResolveTrackIdOption::AddMissing);
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIdsFromUrls(
urls, true);
return trackIds.size() > 0;
}
}
Expand Down
13 changes: 2 additions & 11 deletions src/library/playlistfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,22 +106,13 @@ bool PlaylistFeature::dropAcceptChild(const QModelIndex& index, QList<QUrl> urls
VERIFY_OR_DEBUG_ASSERT(playlistId >= 0) {
return false;
}

QList<QFileInfo> files = DragAndDropHelper::supportedTracksFromUrls(urls, false, true);
if (!files.size()) {
return false;
}

// If a track is dropped onto a playlist's name, but the track isn't in the
// library, then add the track to the library before adding it to the
// playlist.
// pSource != nullptr it is a drop from inside Mixxx and indicates all
// tracks already in the DB
TrackDAO::ResolveTrackIdOptions options = TrackDAO::ResolveTrackIdOption::UnhideHidden;
if (pSource == nullptr) {
options |= TrackDAO::ResolveTrackIdOption::AddMissing;
}
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(files, options);
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIdsFromUrls(urls,
!pSource);
if (!trackIds.size()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

isEmpty()

Copy link
Contributor

Choose a reason for hiding this comment

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

ping

return false;
}
Expand Down
13 changes: 3 additions & 10 deletions src/library/playlisttablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ int PlaylistTableModel::addTracks(const QModelIndex& index,
return 0;
}

QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIdsFromLocations(
locations);

const int positionColumn = fieldIndex(ColumnCache::COLUMN_PLAYLISTTRACKSTABLE_POSITION);
int position = index.sibling(index.row(), positionColumn).data().toInt();

Expand All @@ -86,16 +89,6 @@ int PlaylistTableModel::addTracks(const QModelIndex& index,
position = rowCount() + 1;
}

QList<QFileInfo> fileInfoList;
foreach (QString fileLocation, locations) {
QFileInfo fileInfo(fileLocation);
fileInfoList.append(fileInfo);
}

QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(fileInfoList,
TrackDAO::ResolveTrackIdOption::UnhideHidden
| TrackDAO::ResolveTrackIdOption::AddMissing);

int tracksAdded = m_pTrackCollection->getPlaylistDAO().insertTracksIntoPlaylist(
trackIds, m_iPlaylistId, position);

Expand Down
31 changes: 31 additions & 0 deletions src/library/trackcollection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "util/db/sqltransaction.h"

#include "util/assert.h"
#include "util/dnd.h"


namespace {
Expand Down Expand Up @@ -96,6 +97,36 @@ QList<TrackId> TrackCollection::resolveTrackIds(
return trackIds;
}

QList<TrackId> TrackCollection::resolveTrackIdsFromUrls(
const QList<QUrl>& urls, bool addMissing) {
QList<QFileInfo> files = DragAndDropHelper::supportedTracksFromUrls(urls, false, true);
if (!files.size()) {
return QList<TrackId>();
}

TrackDAO::ResolveTrackIdOptions options =
TrackDAO::ResolveTrackIdOption::UnhideHidden;
if (addMissing) {
options |= TrackDAO::ResolveTrackIdOption::AddMissing;
}
return resolveTrackIds(files, options);
}

QList<TrackId> TrackCollection::resolveTrackIdsFromLocations(
const QList<QString>& locations) {
QList<QFileInfo> fileInfoList;
foreach(QString fileLocation, locations) {
QFileInfo fileInfo(fileLocation);
fileInfoList.append(fileInfo);
}
return resolveTrackIds(fileInfoList,
TrackDAO::ResolveTrackIdOption::UnhideHidden
| TrackDAO::ResolveTrackIdOption::AddMissing);
}

QList<TrackId> resolveTrackIdsFromUrls(const QList<QUrl>& urls,
TrackDAO::ResolveTrackIdOptions options);

bool TrackCollection::hideTracks(const QList<TrackId>& trackIds) {
DEBUG_ASSERT(QApplication::instance()->thread() == QThread::currentThread());

Expand Down
7 changes: 6 additions & 1 deletion src/library/trackcollection.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,12 @@ class TrackCollection : public QObject,

// This function returns a track ID of all file in the list not already visible,
// it adds and unhides the tracks as well.
QList<TrackId> resolveTrackIds(const QList<QFileInfo>& files, TrackDAO::ResolveTrackIdOptions options);
QList<TrackId> resolveTrackIds(const QList<QFileInfo> &files,
TrackDAO::ResolveTrackIdOptions options);
QList<TrackId> resolveTrackIdsFromUrls(const QList<QUrl>& urls,
bool addMissing);
QList<TrackId> resolveTrackIdsFromLocations(
const QList<QString>& locations);

bool hideTracks(const QList<TrackId>& trackIds);
bool unhideTracks(const QList<TrackId>& trackIds);
Expand Down