[Merge] lp:~ahayzen/ubuntu-settings-components/model-update-printer-printerjob into lp:~phablet-team/ubuntu-settings-components/printer-components
Jonas G. Drange
jonas.drange at canonical.com
Fri Feb 3 14:16:43 UTC 2017
Review: Needs Fixing
two minor comments
Diff comments:
>
> === modified file 'plugins/Ubuntu/Settings/Printers/models/jobmodel.cpp'
> --- plugins/Ubuntu/Settings/Printers/models/jobmodel.cpp 2017-02-02 17:22:55 +0000
> +++ plugins/Ubuntu/Settings/Printers/models/jobmodel.cpp 2017-02-03 13:58:37 +0000
> @@ -68,9 +68,14 @@
> bool exists = false;
>
> Q_FOREACH(QSharedPointer<PrinterJob> p, newJobs) {
> - // TODO: update status here
> if (p->jobId() == m_jobs.at(i)->jobId()) {
> exists = true;
> +
> + // Ensure the other properties of the job are up to date
> + if (m_jobs.at(i)->updateFrom(p)) {
> + Q_EMIT dataChanged(index(i), index(i), roleNames().keys().toVector());
roleNames().keys().toVector() can be dropped. From docs: “An empty vector in the roles argument means that all roles should be considered modified.”
> + }
> +
> break;
> }
> }
>
> === modified file 'plugins/Ubuntu/Settings/Printers/models/printermodel.cpp'
> --- plugins/Ubuntu/Settings/Printers/models/printermodel.cpp 2017-02-03 12:04:46 +0000
> +++ plugins/Ubuntu/Settings/Printers/models/printermodel.cpp 2017-02-03 13:58:37 +0000
> @@ -80,6 +80,12 @@
> Q_FOREACH(Printer *p, newPrinters) {
> if (p->name() == m_printers.at(i)->name()) {
> exists = true;
> +
> + // Ensure the other properties of the Printer are up to date
> + if (m_printers.at(i)->updateFrom(p)) {
> + Q_EMIT dataChanged(index(i), index(i), roleNames().keys().toVector());
Same here
> + }
> +
> break;
> }
> }
--
https://code.launchpad.net/~ahayzen/ubuntu-settings-components/model-update-printer-printerjob/+merge/316332
Your team Ubuntu Phablet Team is subscribed to branch lp:~phablet-team/ubuntu-settings-components/printer-components.
More information about the Ubuntu-reviews
mailing list