Merge lp:~sylvain-pineau/checkbox/937715_submit_disabled_at_startup into lp:checkbox

Proposed by Sylvain Pineau on 2012-04-04
Status: Merged
Merged at revision: 1353
Proposed branch: lp:~sylvain-pineau/checkbox/937715_submit_disabled_at_startup
Merge into: lp:checkbox
Diff against target: 33 lines (+6/-0)
2 files modified
debian/changelog (+2/-0)
qt/frontend/qtfront.cpp (+4/-0)
To merge this branch: bzr merge lp:~sylvain-pineau/checkbox/937715_submit_disabled_at_startup
Reviewer Review Type Date Requested Status
Jeff Lane 2012-04-04 Approve on 2012-04-09
Review via email:

Description of the change

This MR tries to solve a new feature only available with the Qt UI. The ability to switch from one tab to another is quite complex to handle with checkbox events. So I prefer to disable the submission buttons until the end of the test run.

To post a comment you must log in.
TienFu Chen (ctf) wrote :

The behavior after the patch looks reasonable to me.
But I still see the warning text in the content of results tab.
Warning: Not all tests have been run yet.
You can send the results now, but the submission won't make it to Ubuntu Friendly.
After the buttons being disabled, user has no way to submit the result before all tests are finished.
For now, to prevent confusing users, should we change the texts to something like "You have to finish all the tests before you can submit the result"?

Sylvain Pineau (sylvain-pineau) wrote :

I think we could keep this feature (submit at any time) for the future as the Qt ui allows it. But for now and if we want the certification client to also benefit asap from the good stuff it's a good idea to simply disable the buttons. In addition to this, disabling the final tab click/navigation could be also something to implement.
I will change the warning message to match the new bahaviour. thank you.
Again I keep in mind that submit at any time could be a good addition, perhaps should we target it for 14.x ?

Jeff Lane (bladernr) wrote :

Setting to needs fixing per your comments above... I agree with Tim, once the warning message is changed (hopefully that only changes it while the buttons are disabled and will revert to the standard warning after testing is done) this can probably be pushed.

I also like the idea of leaving the facility there for 14.x when we have more time to sort out the complexity and maybe get "Submit at any time" working. That could be quite useful for debugging specific tests or issues, or other uses.

review: Needs Fixing
Jeff Lane (bladernr) wrote :

Per discussion w/ ara this morning, merging. The code is fine, the message can be handled elsewhere (there's already a different bug open about the warning message).

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2012-04-03 11:38:10 +0000
3+++ debian/changelog 2012-04-04 14:28:26 +0000
4@@ -19,6 +19,8 @@
5 * qt/frontend/qtfront.cpp, qt/frontend/qtfront.h, plugins/,
6 checkbox_qt/, plugins/ The selection tree is
7 now updated when recovering from a previous run (LP: #937696)
8+ * qt/frontend/qtfront.cpp: Submit/View results buttons are disabled until
9+ every selected test has been run (LP: #937715)
11 [Brendan Donegan]
12 * [FEATURE] Added touchpad tests from CE QA Checkbox to allow touchpad
14=== modified file 'qt/frontend/qtfront.cpp'
15--- qt/frontend/qtfront.cpp 2012-04-03 11:38:10 +0000
16+++ qt/frontend/qtfront.cpp 2012-04-04 14:28:26 +0000
17@@ -73,6 +73,9 @@
18 ui->treeView->setContextMenuPolicy(Qt::CustomContextMenu);
19 connect(ui->treeView, SIGNAL(customContextMenuRequested(const QPoint&)), this, SLOT(onSelectAllContextMenu(const QPoint&)));
20 ui->stepsFrame->setFixedHeight(0);
21+ ui->buttonSubmitResults->setEnabled(false);
22+ ui->lineEditEmailAddress->setEnabled(false);
23+ ui->buttonViewResults->setEnabled(false);
25 // comment box
26 ui->commentTestButton->setMenu(new QMenu());
27@@ -285,6 +288,7 @@
29 ui->buttonSubmitResults->setEnabled(true);
30 ui->lineEditEmailAddress->setEnabled(true);
31+ ui->buttonViewResults->setEnabled(true);
33 }


People subscribed via source and target branches