[Qt] sendcoinsdialog: convert QMessageBox usage to message() #3189

Diapolo commented at 2:40 PM on October 30, 2013: none

add new processSendCoinsReturn() function, which parses the status of WalletModel::SendCoinsReturn and generates a pair consisting of a message and message flags
result is we only need one emit message() call
this change ensures that the GUI is shown for warnings/errors in sendcoinsdialog, because of message() taking care of that for modal messages
changes 2 warning message into error messages and removed "Error:" in front of the actual error message string

in src/qt/sendcoinsdialog.cpp:None in 8d322a10df outdated

 141 | @@ -141,29 +142,29 @@ void SendCoinsDialog::on_sendButton_clicked()
 142 |      switch(prepareStatus.status)
 143 |      {
 144 |      case WalletModel::InvalidAddress:
 145 | -        QMessageBox::warning(this, strSendCoins,
 146 | -            tr("The recipient address is not valid, please recheck."));
 147 | +        emit message(strSendCoins, tr("The recipient address is not valid, please recheck."),

laanwj commented at 3:00 PM on October 30, 2013:

It would be even nicer to wrap this in a function that converts a WalletModel::StatusCode to a (QString, CClientUIInterface::MessageBoxFlags) pair, so that we only need one emit messaget().

in src/qt/sendcoinsdialog.cpp:None in 8d322a10df outdated

 212 | @@ -212,8 +213,8 @@ void SendCoinsDialog::on_sendButton_clicked()
 213 |      switch(sendstatus.status)
 214 |      {
 215 |      case WalletModel::TransactionCommitFailed:
 216 | -        QMessageBox::warning(this, strSendCoins,
 217 | -            tr("Error: The transaction was rejected. This might happen if some of the coins in your wallet were already spent, such as if you used a copy of wallet.dat and coins were spent in the copy but not marked as spent here."));
 218 | +        emit message(strSendCoins, tr("The transaction was rejected. This might happen if some of the coins in your wallet were already spent, such as if you used a copy of wallet.dat and coins were spent in the copy but not marked as spent here."),

laanwj commented at 3:01 PM on October 30, 2013:

And could use the same function here

laanwj commented at 3:01 PM on October 30, 2013: member

ACK otherwise

Diapolo commented at 4:27 PM on October 30, 2013: none

Updated:

add new parseWalletModelSendCoinsReturn() function, which parses the status of WalletModel::SendCoinsReturn and generates a pair consisting of a message and message flags
result is we only need one emit message() call

Diapolo commented at 4:54 PM on October 30, 2013: none

@laanwj So, time for review ;).

in src/qt/sendcoinsdialog.cpp:None in f6517eea59 outdated

 169 | -    case WalletModel::OK:
 170 | -    case WalletModel::Aborted: // User aborted, nothing to do
 171 | -    default:
 172 | -        break;
 173 | -    }
 174 | +    // parse prepareStatus and on error generate message shown to user

laanwj commented at 5:39 PM on October 30, 2013:

I agree with the general idea. But the method name shouldn't be called "parseXXX" (as it doesn't really parse anything) and is too long. Maybe "processSendCoinsReturn"?

in src/qt/sendcoinsdialog.cpp:None in f6517eea59 outdated

 181 | -    switch(sendstatus.status)
 182 | +    WalletModel::SendCoinsReturn sendStatus = model->sendCoins(currentTransaction);
 183 | +    // parse sendStatus and on error generate message shown to user
 184 | +    parseWalletModelSendCoinsReturn(sendStatus);
 185 | +
 186 | +    if (sendStatus.status)

laanwj commented at 5:44 PM on October 30, 2013:

Please compare explicitly against OK, like above, that's more clear

 if(sendStatus.status == WalletModel::OK) {

Diapolo commented at 9:16 PM on October 30, 2013:

That change was by mistake...

Diapolo commented at 9:21 PM on October 30, 2013: none

Updated:

renamed new function
re-add explicit check for WalletModel::OK

[Qt] sendcoinsdialog: convert QMessageBox usage to message()

- add new processSendCoinsReturn() function, which parses the
  status of WalletModel::SendCoinsReturn and generates a pair consisting
  of a message and message flags
- result is we only need one emit message() call
- this change ensures that the GUI is shown for warnings/errors in
  sendcoinsdialog, because of message() taking care of that for modal
  messages
- changes 2 warning message into error messages and removed "Error:"
  in front of the actual error message string

71ba4670fc

BitcoinPullTester commented at 10:17 PM on October 30, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/71ba4670fcd35209db230381bc8a63ac2d1cc22a for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

laanwj referenced this in commit cd6426ec72 on Oct 31, 2013

laanwj merged this on Oct 31, 2013

laanwj closed this on Oct 31, 2013

DrahtBot locked this on Sep 8, 2021