Vkontakte clementine internet service

[davidsansome, 2011-11-18 11:36:51]

Looks pretty good!  I've added some little comments below.  If you can get the
authentication stuff working with QTextBrowser (and manual network requests I
suppose) then that would be perfect, otherwise maybe it would be possible to
reverse engineer the login process and make a POST yourself manually.

http://codereview.appspot.com/5405044/diff/1/src/internet/vkontakteservice.cpp
File src/internet/vkontakteservice.cpp (right):

http://codereview.appspot.com/5405044/diff/1/src/internet/vkontakteservice.cp...
src/internet/vkontakteservice.cpp:76: if (item->text()==tr("My own tracks"))
It might be better to set a value in Role_Type and compare that instead of
relying on the text.  Or even just do if (item == my_tracks_)

http://codereview.appspot.com/5405044/diff/1/src/internet/vkontakteservice.cp...
src/internet/vkontakteservice.cpp:86: expire_date_ =
s.value("expire_date",0).toInt();
toDateTime()

http://codereview.appspot.com/5405044/diff/1/src/internet/vkontakteservice.cp...
src/internet/vkontakteservice.cpp:89: logged_in_ = (time(NULL)<expire_date_ &&
!user_id_.isEmpty() );
QDateTime::currentDateTime() < expire_date_

http://codereview.appspot.com/5405044/diff/1/src/internet/vkontakteservice.cp...
src/internet/vkontakteservice.cpp:124: qLog(Error) << reply->errorString();
Is it possible for the user to try again if this happens?  The tree item will
already be marked as lazy-loaded so it won't call PopulateTracksForUserAsync
again if the user clicks on it a second time.  Maybe add an option to the
context menu to refresh the list?

http://codereview.appspot.com/5405044/diff/1/src/internet/vkontakteservice.cp...
src/internet/vkontakteservice.cpp:132: doc.setContent(reply->readAll());
You can give it the QIODevice directly which might be more efficient:
doc.setContent(reply)

http://codereview.appspot.com/5405044/diff/1/src/internet/vkontakteservice.cp...
src/internet/vkontakteservice.cpp:151: QStandardItem* row = new
QStandardItem(song.artist()+" - "+song.title());
song.PrettyTitleWithArtist()

http://codereview.appspot.com/5405044/diff/1/src/internet/vkontakteservice.cp...
src/internet/vkontakteservice.cpp:161: case 4:
Can you make these constants since they're shared by PopulateFriendsFinished
too?

http://codereview.appspot.com/5405044/diff/1/src/internet/vkontakteservice.cp...
src/internet/vkontakteservice.cpp:299: s.setValue("user_id",QString());
s.remove("user_id") might be better, then you don't have to duplicate the
default values.

http://codereview.appspot.com/5405044/diff/1/src/internet/vkontakteservice.h
File src/internet/vkontakteservice.h (right):

http://codereview.appspot.com/5405044/diff/1/src/internet/vkontakteservice.h#...
src/internet/vkontakteservice.h:59: void PopulateTracksForUserAsync(QString
user_id, QStandardItem* root);
Pass all these user_ids as const QString&

http://codereview.appspot.com/5405044/diff/1/src/internet/vkontakteservice.h#...
src/internet/vkontakteservice.h:74: time_t expire_date_;
Use a QDateTime instead, and then .toTime_t() and QDateTime::fromTime_t()