Skip to content

implemented changes suggested PR#4351 #4629

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.

Sign up for GitHub

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

Open
daharkan wants to merge 4 commits into
base: develop
Choose a base branch
from
+187 −29
Open

implemented changes suggested PR#4351 #4629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6ad6794
implemented changes suggested PR#4351
daharkan Dec 19, 2021
babc1de
new line removed
daharkan Dec 19, 2021
79d8355
paymentRemaining calculation fixed
daharkan Jan 2, 2022
733b2b9
added new test
daharkan Jan 2, 2022
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
3 changes: 3 additions & 0 deletions src/Makefile.qttest.include
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ TESTS += qt/test/test_dash-qt
TEST_QT_MOC_CPP = \
qt/test/moc_apptests.cpp \
qt/test/moc_compattests.cpp \
qt/test/moc_proposaltests.cpp \
qt/test/moc_rpcnestedtests.cpp \
qt/test/moc_trafficgraphdatatests.cpp \
qt/test/moc_uritests.cpp
Expand All @@ -31,6 +32,7 @@ TEST_QT_H = \
qt/test/uritests.h \
qt/test/util.h \
qt/test/paymentrequestdata.h \
qt/test/proposaltests.h \
qt/test/paymentservertests.h \
qt/test/trafficgraphdatatests.h \
qt/test/wallettests.h
Expand All @@ -46,6 +48,7 @@ qt_test_test_dash_qt_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BITCOIN_QT_
qt_test_test_dash_qt_SOURCES = \
qt/test/apptests.cpp \
qt/test/compattests.cpp \
qt/test/proposaltests.cpp \
qt/test/rpcnestedtests.cpp \
qt/test/test_main.cpp \
qt/test/trafficgraphdatatests.cpp \
Expand Down
151 changes: 126 additions & 25 deletions src/qt/governancelist.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ Proposal::Proposal(const CGovernanceObject* p, QObject* parent) :
QObject(parent),
pGovObj(p)
{
//current date is handled with attribute for making tests possible
m_currentDate = QDateTime::currentDateTime();

UniValue prop_data;
if (prop_data.read(pGovObj->GetDataAsPlainString())) {
if (UniValue titleValue = find_value(prop_data, "name"); titleValue.isStr()) {
Expand Down Expand Up @@ -58,6 +61,16 @@ QDateTime Proposal::startDate() const { return m_startDate; }

QDateTime Proposal::endDate() const { return m_endDate; }

QDateTime Proposal::currentDate() const { return m_currentDate; }

int Proposal::paymentRemaining() const
{
long remainingInSecs = endDate().toSecsSinceEpoch() - QDateTime::currentSecsSinceEpoch();
int remainingInDays = remainingInSecs / Params().GetConsensus().nPowTargetTimespan;
float remainingCycle = remainingInDays / CYCLE_IN_DAYS;
return round(remainingCycle);
}

float Proposal::paymentAmount() const { return m_paymentAmount; }

QString Proposal::url() const { return m_url; }
Expand Down Expand Up @@ -89,6 +102,21 @@ int Proposal::GetAbsoluteYesCount() const
return pGovObj->GetAbsoluteYesCount(VOTE_SIGNAL_FUNDING);
}

int Proposal::GetYesCount() const
{
return pGovObj->GetYesCount(VOTE_SIGNAL_FUNDING);
}

int Proposal::GetNoCount() const
{
return pGovObj->GetNoCount(VOTE_SIGNAL_FUNDING);
}

int Proposal::GetAbstainCount() const
{
return pGovObj->GetAbstainCount(VOTE_SIGNAL_FUNDING);
}

void Proposal::openUrl() const
{
QDesktopServices::openUrl(QUrl(m_url));
Expand Down Expand Up @@ -117,26 +145,32 @@ int ProposalModel::columnCount(const QModelIndex& index) const

QVariant ProposalModel::data(const QModelIndex& index, int role) const
{
if (role != Qt::DisplayRole && role != Qt::EditRole) return {};
if (role != Qt::DisplayRole && role != Qt::EditRole && role != Qt::TextAlignmentRole) return {};
const auto proposal = m_data[index.row()];
switch(role) {
case Qt::DisplayRole:
{
switch (index.column()) {
case Column::HASH:
return proposal->hash();
case Column::TITLE:
return proposal->title();
case Column::START_DATE:
return proposal->startDate().date();
case Column::END_DATE:
return proposal->endDate().date();
case Column::PAYMENT_AMOUNT:
return proposal->paymentAmount();
case Column::PAYMENTS_REMAINING:
return proposal->paymentRemaining();
case Column::YES_COUNT:
return proposal->GetYesCount();
case Column::NO_COUNT:
return proposal->GetNoCount();
case Column::ABSOLUTE_YES:
return proposal->GetAbsoluteYesCount();
case Column::ABSTEIN_COUNT:
Copy link

@Munkybooty Munkybooty Jan 3, 2022

Choose a reason for hiding this comment

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

Should be ABSTAIN

return proposal->GetAbstainCount();
case Column::IS_ACTIVE:
return proposal->isActive() ? "Y" : "N";
case Column::VOTING_STATUS:
return proposal->votingStatus(nAbsVoteReq);
case Column::URL:
return proposal->url();
default:
return {};
};
Expand All @@ -146,25 +180,61 @@ QVariant ProposalModel::data(const QModelIndex& index, int role) const
{
// Edit role is used for sorting, so return the raw values where possible
switch (index.column()) {
case Column::HASH:
return proposal->hash();
case Column::TITLE:
return proposal->title();
case Column::START_DATE:
return proposal->startDate();
case Column::END_DATE:
return proposal->endDate();
case Column::PAYMENT_AMOUNT:
return proposal->paymentAmount();
case Column::PAYMENTS_REMAINING:
return proposal->paymentRemaining();
case Column::YES_COUNT:
return proposal->GetYesCount();
case Column::NO_COUNT:
return proposal->GetNoCount();
case Column::ABSOLUTE_YES:
return proposal->GetAbsoluteYesCount();
case Column::ABSTEIN_COUNT:
Copy link

@Munkybooty Munkybooty Jan 3, 2022

Choose a reason for hiding this comment

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

Should be ABSTAIN

return proposal->GetAbstainCount();
case Column::IS_ACTIVE:
return proposal->isActive();
case Column::VOTING_STATUS:
return proposal->GetAbsoluteYesCount();
case Column::URL:
return proposal->url();
default:
return {};
};
break;
}
case Qt::TextAlignmentRole:
{
// Edit role is used for sorting, so return the raw values where possible
switch (index.column()) {
case Column::TITLE:
return Qt::AlignLeft;
case Column::PAYMENT_AMOUNT:
return Qt::AlignCenter;
case Column::PAYMENTS_REMAINING:
return Qt::AlignCenter;
case Column::YES_COUNT:
return Qt::AlignCenter;
case Column::NO_COUNT:
return Qt::AlignCenter;
case Column::ABSOLUTE_YES:
return Qt::AlignCenter;
case Column::ABSTEIN_COUNT:
Copy link

@Munkybooty Munkybooty Jan 3, 2022

Choose a reason for hiding this comment

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

Should be ABSTAIN

return Qt::AlignCenter;
case Column::IS_ACTIVE:
return Qt::AlignCenter;
case Column::VOTING_STATUS:
return Qt::AlignLeft;
case Column::URL:
return Qt::AlignLeft;
default:
return Qt::AlignLeft;
};
break;
}

};
return {};
}
Expand All @@ -173,20 +243,26 @@ QVariant ProposalModel::headerData(int section, Qt::Orientation orientation, int
{
if (orientation != Qt::Horizontal || role != Qt::DisplayRole) return {};
switch (section) {
case Column::HASH:
return "Hash";
case Column::TITLE:
return "Title";
case Column::START_DATE:
return "Start";
case Column::END_DATE:
return "End";
case Column::PAYMENTS_REMAINING:
return "Payments Remaining";
case Column::PAYMENT_AMOUNT:
return "Amount";
case Column::YES_COUNT:
return "Yes";
case Column::NO_COUNT:
return "No";
case Column::ABSOLUTE_YES:
return "Absolute Yes";
case Column::ABSTEIN_COUNT:
return "Abstein";
Comment on lines +258 to +259
Copy link

@Munkybooty Munkybooty Jan 3, 2022

Choose a reason for hiding this comment

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

Should be ABSTAIN and return "Abstain"

case Column::IS_ACTIVE:
return "Active";
case Column::VOTING_STATUS:
return "Status";
case Column::URL:
return "URL";
default:
return {};
}
Expand All @@ -195,16 +271,19 @@ QVariant ProposalModel::headerData(int section, Qt::Orientation orientation, int
int ProposalModel::columnWidth(int section)
{
switch (section) {
case Column::HASH:
return 80;
case Column::TITLE:
return 220;
case Column::START_DATE:
case Column::END_DATE:
case Column::ABSOLUTE_YES:
return 180;
case Column::YES_COUNT:
case Column::NO_COUNT:
case Column::ABSTEIN_COUNT:
Copy link

@Munkybooty Munkybooty Jan 3, 2022

Choose a reason for hiding this comment

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

Should be ABSTAIN

case Column::PAYMENT_AMOUNT:
return 110;
case Column::IS_ACTIVE:
return 80;
case Column::URL:
case Column::PAYMENTS_REMAINING:
case Column::VOTING_STATUS:
return 220;
default:
Expand Down Expand Up @@ -298,7 +377,7 @@ GovernanceList::GovernanceList(QWidget* parent) :
// Set up sorting.
proposalModelProxy->setSortRole(Qt::EditRole);
ui->govTableView->setSortingEnabled(true);
ui->govTableView->sortByColumn(ProposalModel::Column::START_DATE, Qt::DescendingOrder);
ui->govTableView->sortByColumn(ProposalModel::Column::PAYMENTS_REMAINING, Qt::AscendingOrder);

// Set up filtering.
proposalModelProxy->setFilterKeyColumn(ProposalModel::Column::TITLE); // filter by title column...
Expand Down Expand Up @@ -376,7 +455,7 @@ void GovernanceList::showProposalContextMenu(const QPoint& pos)
QAction* openProposalUrl = new QAction(proposal->url(), this);
proposalContextMenu->clear();
proposalContextMenu->addAction(openProposalUrl);
connect(openProposalUrl, &QAction::triggered, proposal, &Proposal::openUrl);
connect(openProposalUrl, &QAction::triggered, this, &GovernanceList::openUrl);
proposalContextMenu->exec(QCursor::pos());
}

Expand All @@ -396,3 +475,25 @@ void GovernanceList::showAdditionalInfo(const QModelIndex& index)

QMessageBox::information(this, windowTitle, json);
}

void GovernanceList::openUrl()
{
QAction *openProposalUrl = qobject_cast<QAction*>(sender());
Copy link
Member

@PastaPastaPasta PastaPastaPasta Jan 2, 2022

Choose a reason for hiding this comment

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

ptr should go on the type

Suggested change
QAction *openProposalUrl = qobject_cast<QAction*>(sender());
QAction* openProposalUrl = qobject_cast<QAction*>(sender());

QString urlString = openProposalUrl->text();
QUrl url(urlString);
if (!url.isValid()) {
QMessageBox::critical(this, "Invalid URL", "URL is not valid: " + urlString);
return;
}

QMessageBox urlWarning;
urlWarning.setWindowTitle("Warning");
urlWarning.setText("Do you want to open the url? " + urlString);
urlWarning.setStandardButtons(QMessageBox::Yes);
urlWarning.addButton(QMessageBox::No);
if (urlWarning.exec() == QMessageBox::Yes) {
QDesktopServices::openUrl(QUrl(url));
} else {
return;
}
}
20 changes: 16 additions & 4 deletions src/qt/governancelist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <QWidget>

inline constexpr int GOVERNANCELIST_UPDATE_SECONDS = 10;
inline constexpr float CYCLE_IN_DAYS = 30.29;

namespace Ui {
class GovernanceList;
Expand All @@ -26,11 +27,13 @@ class Proposal : public QObject
{
private:
Q_OBJECT
friend class ProposalTests;

const CGovernanceObject* pGovObj;
QString m_title;
QDateTime m_startDate;
QDateTime m_endDate;
QDateTime m_currentDate;
float m_paymentAmount;
QString m_url;

Expand All @@ -40,11 +43,16 @@ class Proposal : public QObject
QString hash() const;
QDateTime startDate() const;
QDateTime endDate() const;
QDateTime currentDate() const;
int paymentRemaining() const;
float paymentAmount() const;
QString url() const;
bool isActive() const;
QString votingStatus(int nAbsVoteReq) const;
int GetAbsoluteYesCount() const;
int GetYesCount() const;
int GetNoCount() const;
int GetAbstainCount() const;

void openUrl() const;

Expand All @@ -62,13 +70,16 @@ class ProposalModel : public QAbstractTableModel
QAbstractTableModel(parent){};

enum Column : int {
HASH = 0,
TITLE,
START_DATE,
END_DATE,
TITLE = 0,
PAYMENTS_REMAINING,
PAYMENT_AMOUNT,
IS_ACTIVE,
YES_COUNT,
NO_COUNT,
ABSTEIN_COUNT,
Copy link

@Munkybooty Munkybooty Jan 3, 2022

Choose a reason for hiding this comment

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

Should be ABSTAIN

ABSOLUTE_YES,
VOTING_STATUS,
URL,
_COUNT // for internal use only
};

Expand Down Expand Up @@ -110,6 +121,7 @@ private Q_SLOTS:
void updateProposalCount() const;
void showProposalContextMenu(const QPoint& pos);
void showAdditionalInfo(const QModelIndex& index);
void openUrl();
};


Expand Down
21 changes: 21 additions & 0 deletions src/qt/test/proposaltests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#include "proposaltests.h"
Copy link
Member

@PastaPastaPasta PastaPastaPasta Jan 2, 2022

Choose a reason for hiding this comment

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

you need add copyright

#include <test/setup_common.h>

#include <qt/governancelist.h>

void ProposalTests::proposaltests()
{
CGovernanceObject *pGovObj = new CGovernanceObject;
Proposal proposal(pGovObj);
proposal.m_currentDate = QDateTime(QDate(2022, 1, 2), QTime(12, 00));
proposal.m_endDate = QDateTime(QDate(2022, 2, 9), QTime(12, 00));
QVERIFY( proposal.paymentRemaining() == 1) ;

proposal.m_currentDate = QDateTime(QDate(2022, 1, 2), QTime(12, 00));
proposal.m_endDate = QDateTime(QDate(2022, 3, 5), QTime(12, 00));
QVERIFY( proposal.paymentRemaining() == 2);

proposal.m_currentDate = QDateTime(QDate(2022, 1, 2), QTime(12, 00));
proposal.m_endDate = QDateTime(QDate(2022, 4, 10), QTime(12, 00));
QVERIFY( proposal.paymentRemaining() == 3);
Comment on lines +10 to +20
Copy link
Member

@PastaPastaPasta PastaPastaPasta Jan 2, 2022

Choose a reason for hiding this comment

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

should test the exact edges, and preferably de-duplicate this code smth like

const auto& check_paymentRemaining = [&proposal](const auto& begin, const auto& end, int expected) {
    proposal.m_currentDate = begin;
    proposal.m_endDate = end;
    QVERIFY( proposal.paymentRemaining() == expected) ;
}

Also, I think there may be a bug here... the currentDate and endDate shouldn't be enough to calculate payments remaining.. I think you also need to know when the last superblock was. @UdjinM6 maybe @thephez, what should the calculation be to determine payments remaining?

Copy link
Collaborator

@thephez thephez Jan 3, 2022

Choose a reason for hiding this comment

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

Yes, it doesn't seem possible to fully evaluate without comparing against superblock times. I'm not sure what the full calculation would be, but it should be available somewhere in either Core or Sentinel. Perhaps @UdjinM6 has a reference? 🤞 DMT definitely does this calc also since it displays this info. My guess would be that it happens somewhere in here, but that's a long file.

Also, since the proposal start/end are defined as timestamps while the superblocks are based on block heights, it's not a precise calculation (i.e. it's impacted by variations in block time).

Copy link
Author

@daharkan daharkan Jan 3, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Should it be something like this at line # 260 at here ?

payment_remaining = payment_cycles - cur_cycle

        payment_cycles = int((end_sb - start_sb) / cycle_blocks) + 1

        # calculate number of payment-months that passed already for the proposal
        if start_sb > last_superblock:
            cur_cycle = 0
        else:
            cur_cycle = int(((last_superblock - start_sb) / cycle_blocks)) + 1`

}
15 changes: 15 additions & 0 deletions src/qt/test/proposaltests.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#ifndef PROPOSALTESTS_H
Copy link
Member

@PastaPastaPasta PastaPastaPasta Jan 2, 2022

Choose a reason for hiding this comment

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

copyright

#define PROPOSALTESTS_H

#include <QObject>
#include <QTest>

class ProposalTests : public QObject
{
Q_OBJECT

private Q_SLOTS:
void proposaltests();
};

#endif // PROPOSALTESTS_H
Loading