After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 779079 - Show number of my jobs only
Show number of my jobs only
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Printers
git master
Other Linux
: Normal normal
: ---
Assigned To: Felipe Borges
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-02-22 16:29 UTC by Marek Kašík
Modified: 2017-02-27 15:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
printers: Store PpPrinterEntry instances for individual manipulation (1.90 KB, patch)
2017-02-24 10:35 UTC, Felipe Borges
none Details | Review
printers: Subscribe to jobs notifications (8.14 KB, patch)
2017-02-24 10:35 UTC, Felipe Borges
committed Details | Review
printers: Store PpPrinterEntry instances for individual manipulation (2.08 KB, patch)
2017-02-27 14:23 UTC, Felipe Borges
committed Details | Review
printers: Free the PpJobsDialog after closing it (895 bytes, patch)
2017-02-27 14:29 UTC, Felipe Borges
committed Details | Review
printers: Count only current users job (1.02 KB, patch)
2017-02-27 14:32 UTC, Felipe Borges
committed Details | Review

Description Marek Kašík 2017-02-22 16:29:22 UTC
Commit 37e37961e5577da0b4fbff78c4d2a03fa7618779 changed count of active jobs from current user jobs to jobs of all users. This is wrong. We have to show number of jobs of current user only!
Comment 1 Felipe Borges 2017-02-24 10:35:50 UTC
Created attachment 346622 [details] [review]
printers: Store PpPrinterEntry instances for individual manipulation

We were actualizing the whole printers collection everytime
something should change.

These patch introduces a HashTable keyed by the unique printer.name,
which allows us to access individual instances of PpPrinterEntry.
Comment 2 Felipe Borges 2017-02-24 10:35:56 UTC
Created attachment 346623 [details] [review]
printers: Subscribe to jobs notifications

The previous implementation of the panel was unable to individually
update a PpPrinterEntry jobs count and its PpJobsDialog.

These changes make the job notifications trigger updates in the
PpPrinterEntry UIs, keeping track of job events on the go.
Comment 3 Marek Kašík 2017-02-27 13:09:58 UTC
Review of attachment 346622 [details] [review]:

::: panels/printers/cc-printers-panel.c
@@ +498,3 @@
   gtk_widget_show_all (content);
+
+  g_hash_table_insert (priv->printer_entries, g_strdup (printer.name), printer_entry);

You are leaking the printer.name here. You should specify key_destroy_func.
Comment 4 Marek Kašík 2017-02-27 13:11:24 UTC
Review of attachment 346623 [details] [review]:

Thank you for the patch. It looks good to me. Wait with its push for the second patch please.
Comment 5 Marek Kašík 2017-02-27 13:17:45 UTC
Also prepare a patch where you change the third parameter from 0 to 1 in PpPrinterEntry please.
Comment 6 Felipe Borges 2017-02-27 14:23:40 UTC
Created attachment 346820 [details] [review]
printers: Store PpPrinterEntry instances for individual manipulation

We were actualizing the whole printers collection everytime
something should change.

These patch introduces a HashTable keyed by the unique printer.name,
which allows us to access individual instances of PpPrinterEntry.
Comment 7 Felipe Borges 2017-02-27 14:29:09 UTC
Created attachment 346821 [details] [review]
printers: Free the PpJobsDialog after closing it

Now we can safely pp_jobs_dialog_free () the PpJobsDialog.
Comment 8 Felipe Borges 2017-02-27 14:32:51 UTC
Created attachment 346822 [details] [review]
printers: Count only current users job

Instead of querying for all the jobs, query just for jobs belonging
to the current user.
Comment 9 Marek Kašík 2017-02-27 15:24:06 UTC
Review of attachment 346820 [details] [review]:

Looks good to me now.
Comment 10 Marek Kašík 2017-02-27 15:26:48 UTC
Review of attachment 346822 [details] [review]:

Thank you for the fix.
Comment 11 Felipe Borges 2017-02-27 15:31:22 UTC
Thanks for your reviews!

Attachment 346623 [details] pushed as 5302047 - printers: Subscribe to jobs notifications
Attachment 346820 [details] pushed as 89ff6a6 - printers: Store PpPrinterEntry instances for individual manipulation
Attachment 346822 [details] pushed as fe51a74 - printers: Count only current users job
Comment 12 Marek Kašík 2017-02-27 15:45:10 UTC
Review of attachment 346821 [details] [review]:

This one too. Thanks.
Comment 13 Felipe Borges 2017-02-27 15:53:13 UTC
Attachment 346821 [details] pushed as 5dd8e58 - printers: Free the PpJobsDialog after closing it