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
5 changes: 3 additions & 2 deletions src/library/analysisfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,9 @@ void AnalysisFeature::cleanupAnalyzer() {
bool AnalysisFeature::dropAccept(QList<QUrl> urls, QObject* pSource) {
Q_UNUSED(pSource);
QList<QFileInfo> files = DragAndDropHelper::supportedTracksFromUrls(urls, false, true);
// Adds track, does not insert duplicates, handles unremoving logic.
QList<TrackId> trackIds = m_pTrackCollection->getAndEnsureTrackIds(files, true);
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(files,
TrackDAO::ResolveTrackIdOption::UnhideHidden
| TrackDAO::ResolveTrackIdOption::AddMissing);
analyzeTracks(trackIds);
return trackIds.size() > 0;
}
Expand Down
10 changes: 6 additions & 4 deletions src/library/autodj/autodjfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,14 @@ bool AutoDJFeature::dropAccept(QList<QUrl> urls, QObject* pSource) {

// 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. getAndEnsureTrackIds(), does not insert duplicates and handles
// unremove logic.
// Auto DJ playlist.
// pSource != nullptr it is a drop from inside Mixxx and indicates all
// tracks already in the DB
bool addMissingTracks = (pSource == nullptr);
QList<TrackId> trackIds = m_pTrackCollection->getAndEnsureTrackIds(files, addMissingTracks);
TrackDAO::ResolveTrackIdOptions options = TrackDAO::ResolveTrackIdOption::UnhideHidden;
if (pSource == nullptr) {
options |= TrackDAO::ResolveTrackIdOption::AddMissing;
}
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(files, options);
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
10 changes: 6 additions & 4 deletions src/library/crate/cratefeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -210,12 +210,14 @@ bool CrateFeature::dropAcceptChild(const QModelIndex& index, QList<QUrl> urls,

// 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. getAndEnsureTrackIds(), does not insert duplicates and handles
// unremove logic.
// playlist.
// pSource != nullptr it is a drop from inside Mixxx and indicates all
// tracks already in the DB
bool addMissingTracks = (pSource == nullptr);
QList<TrackId> trackIds = m_pTrackCollection->getAndEnsureTrackIds(files, addMissingTracks);
TrackDAO::ResolveTrackIdOptions options = TrackDAO::ResolveTrackIdOption::UnhideHidden;
if (pSource == nullptr) {
options |= TrackDAO::ResolveTrackIdOption::AddMissing;
}
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(files, options);
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
4 changes: 3 additions & 1 deletion src/library/crate/cratetablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ int CrateTableModel::addTracks(const QModelIndex& index,
fileInfoList.append(fileInfo);
}

QList<TrackId> trackIds = m_pTrackCollection->getAndEnsureTrackIds(fileInfoList, true);
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(fileInfoList,
TrackDAO::ResolveTrackIdOption::UnhideHidden
| TrackDAO::ResolveTrackIdOption::AddMissing);
if (m_pTrackCollection->addCrateTracks(m_selectedCrate, trackIds)) {
select();
return trackIds.size();
Expand Down
6 changes: 3 additions & 3 deletions src/library/dao/trackdao.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ TrackId TrackDAO::getTrackId(const QString& absoluteFilePath) {
return trackId;
}

QList<TrackId> TrackDAO::getTrackIds(const QList<QFileInfo>& files,
bool addMissingTracks) {
QList<TrackId> TrackDAO::resolveTrackIds(const QList<QFileInfo>& files,
ResolveTrackIdOptions options) {
QList<TrackId> trackIds;
trackIds.reserve(files.size());

Expand Down Expand Up @@ -164,7 +164,7 @@ QList<TrackId> TrackDAO::getTrackIds(const QList<QFileInfo>& files,
LOG_FAILED_QUERY(query);
}

if (addMissingTracks) {
if (options & ResolveTrackIdOption::AddMissing) {
// Prepare to add tracks to the database.
// This also begins an SQL transaction.
addTracksPrepare();
Expand Down
13 changes: 12 additions & 1 deletion src/library/dao/trackdao.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ class LibraryHashDAO;
class TrackDAO : public QObject, public virtual DAO, public virtual GlobalTrackCacheRelocator {
Q_OBJECT
public:

enum class ResolveTrackIdOption : int {
ResolveOnly = 0,
UnhideHidden = 1,
AddMissing = 2
};
Q_DECLARE_FLAGS(ResolveTrackIdOptions, ResolveTrackIdOption)

// The 'config object' is necessary because users decide ID3 tags get
// synchronized on track metadata change
TrackDAO(
Expand All @@ -40,7 +48,7 @@ class TrackDAO : public QObject, public virtual DAO, public virtual GlobalTrackC
void finish();

TrackId getTrackId(const QString& absoluteFilePath);
QList<TrackId> getTrackIds(const QList<QFileInfo>& files, bool addMissingTracks);
QList<TrackId> resolveTrackIds(const QList<QFileInfo>& files, ResolveTrackIdOptions options);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add the default option = ResolveTrackIdOption::ResolveOnly? Would make sense here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ping

Copy link
Contributor

Choose a reason for hiding this comment

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

It is not obvious that options contains a mask of flags. Maybe a renaming could help (.h/.cpp)?
ResolveTrackIdOptions options -> ResolveTrackIdFlags flags

QList<TrackId> getTrackIds(const QDir& dir);

// WARNING: Only call this from the main thread instance of TrackDAO.
Expand Down Expand Up @@ -157,4 +165,7 @@ class TrackDAO : public QObject, public virtual DAO, public virtual GlobalTrackC
DISALLOW_COPY_AND_ASSIGN(TrackDAO);
};


Q_DECLARE_OPERATORS_FOR_FLAGS(TrackDAO::ResolveTrackIdOptions)

#endif //TRACKDAO_H
4 changes: 3 additions & 1 deletion src/library/librarytablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ int LibraryTableModel::addTracks(const QModelIndex& index,
foreach (QString fileLocation, locations) {
fileInfoList.append(QFileInfo(fileLocation));
}
QList<TrackId> trackIds = m_pTrackCollection->getAndEnsureTrackIds(fileInfoList, true);
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(fileInfoList,
TrackDAO::ResolveTrackIdOption::UnhideHidden
| TrackDAO::ResolveTrackIdOption::AddMissing);
select();
return trackIds.size();
}
Expand Down
6 changes: 3 additions & 3 deletions src/library/mixxxlibraryfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,9 +182,9 @@ bool MixxxLibraryFeature::dropAccept(QList<QUrl> urls, QObject* pSource) {
return false;
} else {
QList<QFileInfo> files = DragAndDropHelper::supportedTracksFromUrls(urls, false, true);

// Adds track, does not insert duplicates, handles unremoving logic.
QList<TrackId> trackIds = m_pTrackCollection->getAndEnsureTrackIds(files, true);
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(files,
TrackDAO::ResolveTrackIdOption::UnhideHidden
| TrackDAO::ResolveTrackIdOption::AddMissing);
return trackIds.size() > 0;
}
}
Expand Down
10 changes: 6 additions & 4 deletions src/library/playlistfeature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,14 @@ bool PlaylistFeature::dropAcceptChild(const QModelIndex& index, QList<QUrl> urls

// 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. getAndEnsureTrackIds(), does not insert duplicates and handles
// unremove logic.
// playlist.
// pSource != nullptr it is a drop from inside Mixxx and indicates all
// tracks already in the DB
bool addMissingTracks = (pSource == nullptr);
QList<TrackId> trackIds = m_pTrackCollection->getAndEnsureTrackIds(files, addMissingTracks);
TrackDAO::ResolveTrackIdOptions options = TrackDAO::ResolveTrackIdOption::UnhideHidden;
if (pSource == nullptr) {
options |= TrackDAO::ResolveTrackIdOption::AddMissing;
}
QList<TrackId> trackIds = m_pTrackCollection->resolveTrackIds(files, options);
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
4 changes: 3 additions & 1 deletion src/library/playlisttablemodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,9 @@ int PlaylistTableModel::addTracks(const QModelIndex& index,
fileInfoList.append(fileInfo);
}

QList<TrackId> trackIds = m_pTrackCollection->getAndEnsureTrackIds(fileInfoList, true);
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
10 changes: 6 additions & 4 deletions src/library/trackcollection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,12 @@ void TrackCollection::relocateDirectory(QString oldDir, QString newDir) {
GlobalTrackCacheLocker().relocateCachedTracks(&m_trackDao);
}

QList<TrackId> TrackCollection::getAndEnsureTrackIds(
const QList<QFileInfo>& files, bool addMissingTracks) {
QList<TrackId> trackIds = m_trackDao.getTrackIds(files, addMissingTracks);
unhideTracks(trackIds);
QList<TrackId> TrackCollection::resolveTrackIds(
const QList<QFileInfo>& files, TrackDAO::ResolveTrackIdOptions options) {
QList<TrackId> trackIds = m_trackDao.resolveTrackIds(files, options);
if (options & TrackDAO::ResolveTrackIdOption::UnhideHidden) {
unhideTracks(trackIds);
}
return trackIds;
}

Expand Down
2 changes: 1 addition & 1 deletion src/library/trackcollection.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ 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> getAndEnsureTrackIds(const QList<QFileInfo>& files, bool addMissingTracks);
QList<TrackId> resolveTrackIds(const QList<QFileInfo>& files, TrackDAO::ResolveTrackIdOptions options);

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