Review Request 112533: add ratings in the bodega-client
Giorgos Tsiapaliokas
terietor at gmail.com
Mon Sep 16 19:00:58 UTC 2013
> On Sept. 12, 2013, 12:50 p.m., Aaron J. Seigo wrote:
> > lib/bodega/participantratingsjobmodel.cpp, line 75
> > <http://git.reviewboard.kde.org/r/112533/diff/3/?file=189090#file189090line75>
> >
> > this method should:
> >
> > * perform a check for the existence of session; otherwise creating a ParticipantRatingsJobModel and then calling reload will cause a crash
> >
> > * check if the session is authenticated; if it is not, then it should connect to the session's authenticated(bool) signal and return. sth like:
> >
> > if (!session->isAuthenticated()) {
> > connect(session, SIGNAL(authenticated(bool)),
> > q, SLOT(fetchParticipantRatings()),
> > Qt::UniqueConnection);
> > return;
> > }
> >
> > disconnect(session, SIGNAL(authenticated(bool)),
> > q, SLOT(fetchParticipantRatings()));
> >
something is wrong here
I see that this issue exists in the the
ParticipantRatingsJobModel::setSession but there is no
such `ParticipantRatingsJob *job = session->participantRatings();` code.
> On Sept. 12, 2013, 12:50 p.m., Aaron J. Seigo wrote:
> > lib/bodega/participantratingsjobmodel.cpp, lines 322-323
> > <http://git.reviewboard.kde.org/r/112533/diff/3/?file=189090#file189090line322>
> >
> > why would it automatically fetch the list on authentication? it should only fetch the ratings on authentication if ratings have been requested ... i think this can just go.
I will fix it together with the above
- Giorgos
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112533/#review39876
-----------------------------------------------------------
On Sept. 14, 2013, 10:35 a.m., Giorgos Tsiapaliokas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112533/
> -----------------------------------------------------------
>
> (Updated Sept. 14, 2013, 10:35 a.m.)
>
>
> Review request for Bodega.
>
>
> Description
> -------
>
> This patch implements the ratings feature
>
>
> Diffs
> -----
>
> activeclient/package/contents/ui/SettingsPage.qml fb46eca
> activeclient/package/contents/ui/settings/ParticipantRatings.qml PRE-CREATION
> activeclient/package/contents/ui/storebrowser/AssetColumn.qml a678f16
> activeclient/package/contents/ui/storebrowser/Ratings.js PRE-CREATION
> activeclient/package/contents/ui/storebrowser/RatingsColumn.qml PRE-CREATION
> activeclient/src/bodegastore.h 1e5aac5
> activeclient/src/bodegastore.cpp ba9dc27
> lib/bodega/CMakeLists.txt 8d382a7
> lib/bodega/assetjob.h 5aab88c
> lib/bodega/assetjob.cpp 5f539cb
> lib/bodega/assetoperations.h 7ce7900
> lib/bodega/assetoperations.cpp 9f9c2d5
> lib/bodega/assetratingsjob.h PRE-CREATION
> lib/bodega/assetratingsjob.cpp PRE-CREATION
> lib/bodega/assetratingsjobmodel.h PRE-CREATION
> lib/bodega/assetratingsjobmodel.cpp PRE-CREATION
> lib/bodega/globals.h 5ab45da
> lib/bodega/participantratingsjob.h PRE-CREATION
> lib/bodega/participantratingsjob.cpp PRE-CREATION
> lib/bodega/participantratingsjobmodel.h PRE-CREATION
> lib/bodega/participantratingsjobmodel.cpp PRE-CREATION
> lib/bodega/ratingattributesjob.h PRE-CREATION
> lib/bodega/ratingattributesjob.cpp PRE-CREATION
> lib/bodega/ratingsmodel_p.h PRE-CREATION
> lib/bodega/ratingsmodel_p.cpp PRE-CREATION
> lib/bodega/session.h d27d284
> lib/bodega/session.cpp a7c7e94
> lib/bodega/session_p.h ebefb4f
>
> Diff: http://git.reviewboard.kde.org/r/112533/diff/
>
>
> Testing
> -------
>
> check the attached images
>
>
> File Attachments
> ----------------
>
>
> http://git.reviewboard.kde.org/media/uploaded/files/2013/09/05/ratings-main.png
>
> http://git.reviewboard.kde.org/media/uploaded/files/2013/09/05/participantratings.png
>
> http://git.reviewboard.kde.org/media/uploaded/files/2013/09/06/labelsnotcenter.png
>
> http://git.reviewboard.kde.org/media/uploaded/files/2013/09/06/delete2.png
>
> http://git.reviewboard.kde.org/media/uploaded/files/2013/09/06/brokenParticipant.diff
>
> http://git.reviewboard.kde.org/media/uploaded/files/2013/09/06/participantbroken.png
>
>
> Thanks,
>
> Giorgos Tsiapaliokas
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/active/attachments/20130916/1088839a/attachment.html>
More information about the Active
mailing list