[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