Checkbox

Merge lp://staging/~brendan-donegan/checkbox/gui_submission_screen into lp://staging/checkbox

gui_submission_screen
Merge into trunk

Proposed by Brendan Donegan on 2014-03-19

Status:	Merged
Approved by:	Sylvain Pineau on 2014-03-20
Approved revision:	2804
Merged at revision:	2807
Proposed branch:	lp://staging/~brendan-donegan/checkbox/gui_submission_screen
Merge into:	lp://staging/checkbox
Diff against target:	283 lines (+196/-7) 5 files modified checkbox-gui/checkbox-gui/qml/SubmissionDialog.qml (+76/-7) checkbox-gui/gui-engine/gui-engine.cpp (+73/-0) checkbox-gui/gui-engine/gui-engine.h (+11/-0) checkbox-ng/checkbox_ng/service.py (+14/-0) plainbox/plainbox/impl/highlevel.py (+22/-0)
To merge this branch:	bzr merge lp://staging/~brendan-donegan/checkbox/gui_submission_screen
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Sylvain Pineau (community)		2014-03-19	Approve on 2014-03-20
Review via email: mp+211779@code.staging.launchpad.net

Description of the change

Adapt the SubmissionDialog to use a settings file to customise itself. An example of such a config is:

[submission]
input_type = "regex"
input_placeholder = "Secure ID (15 or 18 characters)"
ok_btn_text = "Submit Results"
submit_to_hexr = "true"

[exporter]
export_path = "/tmp/submission.xml"

[transport]
submit_to = "certification"

The current default behaviour is to allow the user to save the XML locally (or view the report, as will always be supported in any version of the SubmissionDialog)

To support the submit_to = "certification" option, I have also added code to perform the sending of a submission over the certification transport.

Revision history for this message

Sylvain Pineau (sylvain-pineau) wrote on 2014-03-20:

[exporter]
export_path = "/tmp/submission.xml"

[transport]
submit_to = "certification"

I'd prefer to distinguish exporters properties like this:

[exporter]
xml = enabled
html = enabled
xml_export_path = "/tmp/submission.xml"
xlsx = enabled
xlsx_export_path = "/tmp/submission.xlsx"
xlsx_export_options = "with_foo with_bar"

Same thing for transports:

[transport]
certification = enabled
certification_secure_id = axxx00000000
launchpad = disabled
hexr = disabled

Revision history for this message

Sylvain Pineau (sylvain-pineau) wrote on 2014-03-20:

Why do you have SendDataViaTransport and SendSubmissionViaCertificationTransport? Could it be possible to just call a generic function (say SendDataViaTransport) with all the necessary parameters?

review: Needs Information

Revision history for this message

Brendan Donegan (brendan-donegan) wrote on 2014-03-20:

> [exporter]
> export_path = "/tmp/submission.xml"
>
> [transport]
> submit_to = "certification"
>
> I'd prefer to distinguish exporters properties like this:
>
> [exporter]
> xml = enabled
> html = enabled
> xml_export_path = "/tmp/submission.xml"
> xlsx = enabled
> xlsx_export_path = "/tmp/submission.xlsx"
> xlsx_export_options = "with_foo with_bar"

This would require a pretty major change in how checkbox-gui currently works. It doesn't export anything until explicitly requested by the user, so each export has to be ordered by the UI, so saying e.g. html = enabled doesn't make sense at the moment.

>
> Same thing for transports:
>
> [transport]
> certification = enabled
> certification_secure_id = axxx00000000
> launchpad = disabled
> hexr = disabled

This kind of scheme would only be useful if we wanted to upload to multiple places at once - there's no use-case for this at the moment. Submitting to HEXR via c3 is supported via a http header (it's not actually sent to HEXR seperately). I do support the secure_id being preset here, but I called the key 'secure_id' instead of certification_secure_id.

Revision history for this message

Brendan Donegan (brendan-donegan) wrote on 2014-03-20:

> Why do you have SendDataViaTransport and
> SendSubmissionViaCertificationTransport? Could it be possible to just call a
> generic function (say SendDataViaTransport) with all the necessary parameters?

Admittedly at the moment it looks a bit like overkill, but it makes sense when we add the SendSubmissionViaLaunchpadTransport function - the function with the named transport supports gathering the information for that particular transport and then forwards this on to the generic function which just calls the send function on the right transport.

lp://staging/~brendan-donegan/checkbox/gui_submission_screen updated on 2014-03-20

2804. By Brendan Donegan on 2014-03-20

checkbox-gui: Customise the SubmissionDialog component according to settings

Use a settings file to customise the behaviour of the SubmissionDialog
component. Also implement the Certification transport so that we can send
submissions to certification.canonical.com

Signed-off-by: Brendan Donegan <email address hidden>

Revision history for this message

Sylvain Pineau (sylvain-pineau) wrote on 2014-03-20:

Looks good, thanks.

review: Approve