fix API availability/version detection to not be terribly racey
ClosedPublic

Authored by sitter on Jul 8 2019, 2:00 PM.

Details

Summary

previously at a random point in time there'd be a call to get the bugzilla
version, that call would (hopefully) reach the login page and cause it
allow login. this is essentially a blocking safe guard to not talk to
unsupported or unreachable bugzillas. because this was entirely
synchronized there was great potential for signal and state racing
where the login page could randomly end up with a disabled login
button when the api communication failed for whatever reason.

this was further complicated by the fact that there was zero UI backing
for the entire check resulting in the login button getting disabled but
the user not having any indication as to why or any means to retry.

to resolve this problem a bunch of changes are necessary:

  • bugzillalib now has an error signal when the version check fails (previously not having received a finished signal meant error, when in fact that may also mean not-yet-done)
  • buzillalib also no longer automatically issues a version lookup as it itself has no use of knowing the version right now
  • the login page logic has had all awareness of bugzilla availability stripped. the assumption now is that when login is reached bugzilla could be contacted.
  • there is a new 'version' page now sorted before the login page. the version page's only use is to provide a UI for the version check or more generally if bugzilla is reachable. it has a busy state and an error stage, once the version check completed once the page is marked inappropriate and would automatically skip ahead as necessary. the appropriate value specifically allows kassistantdialog to ignore the page when skipping forward/backward effectively hiding the page from the user.
  • for purposes of controlling its own appropriateness the KPageWidgetItem of this new page is controlled by the page itself, a design which IMO should be also adopted for the other pages
  • for visual consistency with plasma a qml busyindicator is used on the page. also looks nicer than the ksquencepixmap thingy

The result of this is that upon startup of the dialog, the version page
is created (albeit not visible) and issues a version check. When that
returns the page turns itself inappropriate (and skips ahead if necessary).
Ideally this means the user never sees the page, unless there is an error
in which case the page will block progressing to actually submit the
error to bugzilla. At the same time the user can choose to go back to the
backtrace page to manually grab it and file a bug in the event that
drkonqi cannot get beyond the version page.

BUG: 373099
BUG: 354292
FIXED-IN: 5.17.0
CHANGELOG: Contact to drkonqi is now more reliably verified and the login button enabled when possible

Test Plan
  • slow internet results in page to show up
  • errors result in page to go into error state
  • fast internet lets the page not show up
  • retry button retries when there was an error on first try
  • page switches between states correctly

Diff Detail

Repository
R871 DrKonqi
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter created this revision.Jul 8 2019, 2:00 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 8 2019, 2:00 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
sitter requested review of this revision.Jul 8 2019, 2:00 PM
sitter added a comment.Jul 8 2019, 3:13 PM

Looks comme ca when the page actually is necessary:

broulik added a subscriber: broulik.Jul 8 2019, 3:16 PM

I wasn't serious when I said you should use a QML view with a BusyIndicator. I don't think that's appropriate. You don't want DrKonqi crashing when you want to report a crash caused by the nvidia driver. Or maybe that's the plan. Clever.

sitter added a comment.Jul 8 2019, 3:59 PM

That is rather disgusting snark there. Thanks for that

sitter updated this revision to Diff 61513.Jul 10 2019, 2:15 PM

use qprogressbar until a proper busyindicator becomes available

sitter updated this revision to Diff 61514.Jul 10 2019, 2:17 PM

build fix

apol accepted this revision.Jul 10 2019, 2:51 PM
This revision is now accepted and ready to land.Jul 10 2019, 2:51 PM
This revision was automatically updated to reflect the committed changes.