Gerrit: Factor out user parsing

This has several advantages: 1. It removes duplicate logic. 2. It is now clear where we use username and where full name. 3. Reviewers now appear with full names (broke by 9f697128c3). Change-Id: I5729abc9006cec0d0cb615b01c8ace5def60ce27 Reviewed-by: Tobias Hunger <tobias.hunger@qt.io>
2017-02-25 21:39:08 +02:00
parent 1e89c617f4
commit 1418d6ab1c
3 changed files with 40 additions and 34 deletions
--- a/src/plugins/git/gerrit/gerritmodel.cpp
+++ b/src/plugins/git/gerrit/gerritmodel.cpp
@@ -24,7 +24,6 @@
 ****************************************************************************/

 #include "gerritmodel.h"
-#include "gerritparameters.h"
 #include "../gitplugin.h"
 #include "../gitclient.h"

@@ -32,6 +31,7 @@
 #include <coreplugin/progressmanager/futureprogress.h>
 #include <vcsbase/vcsoutputwindow.h>

+#include <utils/algorithm.h>
 #include <utils/asconst.h>
 #include <utils/synchronousprocess.h>

@@ -60,7 +60,7 @@ namespace Internal {

 QDebug operator<<(QDebug d, const GerritApproval &a)
 {
-    d.nospace() << a.reviewer << " :" << a.approval << " ("
+    d.nospace() << a.reviewer.fullName << ": " << a.approval << " ("
                << a.type << ", " << a.description << ')';
    return d;
 }
@@ -68,7 +68,7 @@ QDebug operator<<(QDebug d, const GerritApproval &a)
 // Sort approvals by type and reviewer
 bool gerritApprovalLessThan(const GerritApproval &a1, const GerritApproval &a2)
 {
-    return a1.type.compare(a2.type) < 0 || a1.reviewer.compare(a2.reviewer) < 0;
+    return a1.type.compare(a2.type) < 0 || a1.reviewer.fullName.compare(a2.reviewer.fullName) < 0;
 }

 QDebug operator<<(QDebug d, const GerritPatchSet &p)
@@ -80,7 +80,7 @@ QDebug operator<<(QDebug d, const GerritPatchSet &p)

 QDebug operator<<(QDebug d, const GerritChange &c)
 {
-    d.nospace() << c.fullTitle() << " by " << c.email
+    d.nospace() << c.fullTitle() << " by " << c.owner.email
                << ' ' << c.lastUpdated << ' ' <<  c.currentPatchSet;
    return d;
 }
@@ -119,9 +119,9 @@ QString GerritPatchSet::approvalsToHtml() const
        } else {
            str << ", ";
        }
-        str << a.reviewer;
-        if (!a.email.isEmpty())
-            str << " <a href=\"mailto:" << a.email << "\">" << a.email << "</a>";
+        str << a.reviewer.fullName;
+        if (!a.reviewer.email.isEmpty())
+            str << " <a href=\"mailto:" << a.reviewer.email << "\">" << a.reviewer.email << "</a>";
        str << ": " << forcesign << a.approval << noforcesign;
    }
    str << "</tr>\n";
@@ -171,10 +171,9 @@ QString GerritPatchSet::approvalsColumn() const

 bool GerritPatchSet::hasApproval(const QString &userName) const
 {
-    for (const GerritApproval &a : approvals)
-        if (a.reviewer == userName)
-            return true;
-    return false;
+    return Utils::contains(approvals, [&userName](const GerritApproval &a) {
+        return a.reviewer.userName == userName;
+    });
 }

 int GerritPatchSet::approvalLevel() const
@@ -189,11 +188,11 @@ QString GerritChange::filterString() const
 {
    const QChar blank = ' ';
    QString result = QString::number(number) + blank + title + blank
-            + owner + blank + project + blank
+            + owner.fullName + blank + project + blank
            + branch + blank + status;
    for (const GerritApproval &a : currentPatchSet.approvals) {
        result += blank;
-        result += a.reviewer;
+        result += a.reviewer.fullName;
    }
    return result;
 }
@@ -452,8 +451,8 @@ QString GerritModel::toHtml(const QModelIndex& index) const
    str << "<html><head/><body><table>"
        << "<tr><td>" << subjectHeader << "</td><td>" << c->fullTitle() << "</td></tr>"
        << "<tr><td>" << numberHeader << "</td><td><a href=\"" << c->url << "\">" << c->number << "</a></td></tr>"
-        << "<tr><td>" << ownerHeader << "</td><td>" << c->owner << ' '
-        << "<a href=\"mailto:" << c->email << "\">" << c->email << "</a></td></tr>"
+        << "<tr><td>" << ownerHeader << "</td><td>" << c->owner.fullName << ' '
+        << "<a href=\"mailto:" << c->owner.email << "\">" << c->owner.email << "</a></td></tr>"
        << "<tr><td>" << projectHeader << "</td><td>" << c->project << " (" << c->branch << ")</td></tr>"
        << dependencyHtml(dependsOnHeader, c->dependsOnNumber, serverPrefix)
        << dependencyHtml(neededByHeader, c->neededByNumber, serverPrefix)
@@ -529,6 +528,16 @@ void GerritModel::setState(GerritModel::QueryState s)
    emit stateChanged();
 }

+// {"name":"Hans Mustermann","email":"hm@acme.com","username":"hansm"}
+static GerritUser parseGerritUser(const QJsonObject &object)
+{
+    GerritUser user;
+    user.userName = object.value("username").toString();
+    user.fullName = object.value("name").toString();
+    user.email = object.value("email").toString();
+    return user;
+}
+
 /* Parse gerrit query Json output.
 * See http://gerrit.googlecode.com/svn/documentation/2.1.5/cmd-query.html
 * Note: The url will be present only if  "canonicalWebUrl" is configured
@@ -554,9 +563,6 @@ static GerritChangePtr parseSshOutput(const QJsonObject &object)
    const QString branchKey = "branch";
    const QString numberKey = "number";
    const QString ownerKey = "owner";
-    const QString ownerNameKey = "name";
-    const QString ownerUserKey = "username";
-    const QString ownerEmailKey = "email";
    const QString statusKey = "status";
    const QString projectKey = "project";
    const QString titleKey = "subject";
@@ -580,9 +586,7 @@ static GerritChangePtr parseSshOutput(const QJsonObject &object)
    for (int a = 0; a < ac; ++a) {
        const QJsonObject ao = approvalsJ.at(a).toObject();
        GerritApproval approval;
-        const QJsonObject approverO = ao.value(approvalsByKey).toObject();
-        approval.reviewer = approverO.value(ownerUserKey).toString();
-        approval.email = approverO.value(ownerEmailKey).toString();
+        approval.reviewer = parseGerritUser(ao.value(approvalsByKey).toObject());
        approval.approval = ao.value(approvalsValueKey).toString().toInt();
        approval.type = ao.value(approvalsTypeKey).toString();
        approval.description = ao.value(approvalsDescriptionKey).toString();
@@ -595,10 +599,7 @@ static GerritChangePtr parseSshOutput(const QJsonObject &object)
    change->number = object.value(numberKey).toString().toInt();
    change->url = object.value(urlKey).toString();
    change->title = object.value(titleKey).toString();
-    const QJsonObject ownerJ = object.value(ownerKey).toObject();
-    change->owner = ownerJ.value(ownerNameKey).toString();
-    change->user = ownerJ.value(ownerUserKey).toString();
-    change->email = ownerJ.value(ownerEmailKey).toString();
+    change->owner = parseGerritUser(object.value(ownerKey).toObject());
    change->project = object.value(projectKey).toString();
    change->branch = object.value(branchKey).toString();
    change->status =  object.value(statusKey).toString();
@@ -689,7 +690,7 @@ QList<QStandardItem *> GerritModel::changeToRow(const GerritChangePtr &c) const
    }
    row[NumberColumn]->setData(c->number, Qt::DisplayRole);
    row[TitleColumn]->setText(c->fullTitle());
-    row[OwnerColumn]->setText(c->owner);
+    row[OwnerColumn]->setText(c->owner.fullName);
    // Shorten columns: Display time if it is today, else date
    const QString dateString = c->lastUpdated.date() == QDate::currentDate() ?
                c->lastUpdated.time().toString(Qt::SystemLocaleShortDate) :
@@ -705,7 +706,7 @@ QList<QStandardItem *> GerritModel::changeToRow(const GerritChangePtr &c) const
    row[ApprovalsColumn]->setText(c->currentPatchSet.approvalsColumn());
    // Mark changes awaiting action using a bold font.
    bool bold = false;
-    if (c->user == m_server->user) { // Owned changes: Review != 0,1. Submit or amend.
+    if (c->owner.userName == m_server->user) { // Owned changes: Review != 0,1. Submit or amend.
        const int level = c->currentPatchSet.approvalLevel();
        bold = level != 0 && level != 1;
    } else { // Changes pending for review: No review yet.
--- a/src/plugins/git/gerrit/gerritmodel.h
+++ b/src/plugins/git/gerrit/gerritmodel.h
@@ -25,6 +25,8 @@

 #pragma once

+#include "gerritparameters.h"
+
 #include <QStandardItemModel>
 #include <QSharedPointer>
 #include <QDateTime>
@@ -35,8 +37,6 @@ QT_END_NAMESPACE

 namespace Gerrit {
 namespace Internal {
-class GerritServer;
-class GerritParameters;
 class QueryContext;

 class GerritApproval {
@@ -45,8 +45,7 @@ public:

    QString type; // Review type
    QString description; // Type description, possibly empty
-    QString reviewer;
-    QString email;
+    GerritUser reviewer;
    int approval;
 };

@@ -76,9 +75,7 @@ public:
    int dependsOnNumber = 0;
    int neededByNumber = 0;
    QString title;
-    QString owner;
-    QString user;
-    QString email;
+    GerritUser owner;
    QString project;
    QString branch;
    QString status;
--- a/src/plugins/git/gerrit/gerritparameters.h
+++ b/src/plugins/git/gerrit/gerritparameters.h
@@ -32,6 +32,14 @@ QT_FORWARD_DECLARE_CLASS(QSettings)
 namespace Gerrit {
 namespace Internal {

+class GerritUser
+{
+public:
+    QString userName;
+    QString fullName;
+    QString email;
+};
+
 class GerritServer
 {
 public: