From 1418d6ab1c8e3fa71c9045a6ae50737e6c665c08 Mon Sep 17 00:00:00 2001 From: Orgad Shaneh Date: Sat, 25 Feb 2017 21:39:08 +0200 Subject: [PATCH] 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 9f697128c3cab82e3aa5b267ec727c7b77428369). Change-Id: I5729abc9006cec0d0cb615b01c8ace5def60ce27 Reviewed-by: Tobias Hunger --- src/plugins/git/gerrit/gerritmodel.cpp | 55 ++++++++++++----------- src/plugins/git/gerrit/gerritmodel.h | 11 ++--- src/plugins/git/gerrit/gerritparameters.h | 8 ++++ 3 files changed, 40 insertions(+), 34 deletions(-) diff --git a/src/plugins/git/gerrit/gerritmodel.cpp b/src/plugins/git/gerrit/gerritmodel.cpp index 493d5b76d30..8e49c9385d9 100644 --- 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 #include +#include #include #include @@ -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.email << ""; + str << a.reviewer.fullName; + if (!a.reviewer.email.isEmpty()) + str << " " << a.reviewer.email << ""; str << ": " << forcesign << a.approval << noforcesign; } str << "\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 << "" << "" << "" - << "" + << "" << "" << 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 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 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. diff --git a/src/plugins/git/gerrit/gerritmodel.h b/src/plugins/git/gerrit/gerritmodel.h index 15582776152..45820f9bf62 100644 --- a/src/plugins/git/gerrit/gerritmodel.h +++ b/src/plugins/git/gerrit/gerritmodel.h @@ -25,6 +25,8 @@ #pragma once +#include "gerritparameters.h" + #include #include #include @@ -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; diff --git a/src/plugins/git/gerrit/gerritparameters.h b/src/plugins/git/gerrit/gerritparameters.h index 0b0dcd1754f..a73d1868814 100644 --- 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:
" << subjectHeader << "" << c->fullTitle() << "
" << numberHeader << "url << "\">" << c->number << "
" << ownerHeader << "" << c->owner << ' ' - << "email << "\">" << c->email << "
" << ownerHeader << "" << c->owner.fullName << ' ' + << "owner.email << "\">" << c->owner.email << "
" << projectHeader << "" << c->project << " (" << c->branch << ")