Merge lp:~ruben-reina-dev/appcenter/installed-apps-list-fixes into lp:~elementary-apps/appcenter/appcenter

[Rubén Reina]

roperly refresh the Installed Apps list when an app is installed or uninstalled from any App Info View

Properly refresh the Installed Apps list when an app is installed or uninstalled from any App Info View.

The list is updated in an non-blocking and optimized way.

[Cody Garver (codygarver)]

Thank you for your submission. Corentin is the best person to review this branch, please be patient until he has time for it.

In the mean time, I plan on creating more AppCenter bounties in the very near future. So please continue checking for new bounties.

Thanks again for your submission.

[Adam Bieńkowski (donadigo)]

Some things about the code:
* I am not sure if a separate installed_view variable is needed here. In vala you can use something like:
"get; private set;"

* We use "lower_case" variables, and we should keep that also there, the name "mainWindow" should become "main_window" to keep the same code style.

* I don't like the use of get_toplevel () here. Maybe you could try to get make installed_view a static object, this way, you wouldn't need to get the MainWindow instance. If not, you can use Gtk's built in functions to get that window like this:

var main_window = (MainWindow)((Gtk.Application)Application.get_default ()).get_active_window ();
Also, make sure that the instance of MainWindow is not null, before doing any operations with it.

Otherwise, it looks OK.

review: Needs Fixing (code)

[Rubén Reina (ruben-reina-dev)]

Hello Adam,

Thank you for your tips and suggestions, they were really useful!

I have added the following changes to the branch:
1. "installed_view" variable in MainWindow is now a static auto-generated
property "public static Views.InstalledView installed_view { get; private
set; }". It always will be assigned because in the "static constructor" of
the MainWindow the variable is assigned with an instance of InstalledView
2. Fix code style issue.

The branch is ready to be reviewed again, so please, check it again when
you can.
Thank you.

2016-06-25 22:26 GMT+02:00 Adam Bieńkowski <email address hidden>:

> Review: Needs Fixing code
>
> Some things about the code:
> * I am not sure if a separate installed_view variable is needed here. In
> vala you can use something like:
> "get; private set;"
>
> * We use "lower_case" variables, and we should keep that also there, the
> name "mainWindow" should become "main_window" to keep the same code style.
>
> * I don't like the use of get_toplevel () here. Maybe you could try to get
> make installed_view a static object, this way, you wouldn't need to get the
> MainWindow instance. If not, you can use Gtk's built in functions to get
> that window like this:
>
> var main_window = (MainWindow)((Gtk.Application)Application.get_default
> ()).get_active_window ();
> Also, make sure that the instance of MainWindow is not null, before doing
> any operations with it.
>
> Otherwise, it looks OK.
> --
>
> https://code.launchpad.net/~ruben-reina-dev/appcenter/installed-apps-list-fixes/+merge/298372
> You are the owner of
> lp:~ruben-reina-dev/appcenter/installed-apps-list-fixes.
>

[Rubén Reina (ruben-reina-dev)]

Thanks to you Cody. I will check those bounties for sure when I have time.

2016-06-25 21:43 GMT+02:00 Cody Garver <email address hidden>:

> Thank you for your submission. Corentin is the best person to review this
> branch, please be patient until he has time for it.
>
> In the mean time, I plan on creating more AppCenter bounties in the very
> near future. So please continue checking for new bounties.
>
> Thanks again for your submission.
> --
>
> https://code.launchpad.net/~ruben-reina-dev/appcenter/installed-apps-list-fixes/+merge/298372
> You are the owner of
> lp:~ruben-reina-dev/appcenter/installed-apps-list-fixes.
>

[lp:~ruben-reina-dev/appcenter/installed-apps-list-fixes]

217. By Ruben Reina <email address hidden> on 2016-06-26

Updated with base

218. By Ruben Reina <email address hidden> on 2016-06-26

Code fixes proposed after the first review

[Adam Bieńkowski (donadigo)]

Thank you for updating the branch! It looks very clean now. I'm not sure stars replaced in the license headers, but I'll approve it. Note that I did not compile / test it.

review: Approve (code)

[Danielle Foré (danrabbit)]

I built and testing the branch and can confirm that it works as expected

review: Approve (testing)

=== modified file 'src/MainWindow.vala'
--- src/MainWindow.vala	2016-06-19 21:09:16 +0000
+++ src/MainWindow.vala	2016-06-26 10:34:12 +0000
@@ -22,10 +22,11 @@
     private Gtk.SearchEntry search_entry;
     private Views.CategoryView category_view;
     private Views.FeaturedView featured_view;
-    private Views.InstalledView installed_view;
     private Views.SearchView search_view;
     private Gtk.Button return_button;
     private ulong task_finished_connection = 0U;
+    
+    public static Views.InstalledView installed_view { get; private set; }
 
     public MainWindow (Gtk.Application app) {
         Object (application: app);

=== modified file 'src/Views/AppInfoView.vala'
--- src/Views/AppInfoView.vala	2016-06-24 17:12:09 +0000
+++ src/Views/AppInfoView.vala	2016-06-26 10:34:12 +0000
@@ -279,8 +279,6 @@
     }
 
     private async void action_clicked () {
-        var treeset = new Gee.TreeSet<AppCenterCore.Package> ();
-        treeset.add (package);
         try {
             if (package.update_available) {
                 yield package.update ();
@@ -294,6 +292,9 @@
                     uninstall_button.no_show_all = false;
                     uninstall_button.show ();
                 }
+                
+                // Add this app to the Installed Apps View
+                MainWindow.installed_view.add_app.begin (package);
             }
         } catch (Error e) {
             critical (e.message);
@@ -302,13 +303,14 @@
     }
 
     private async void uninstall_clicked () {
-        var treeset = new Gee.TreeSet<AppCenterCore.Package> ();
-        treeset.add (package);
         try {
             yield package.uninstall ();
             action_button.label = _("Install");
             uninstall_button.no_show_all = true;
             uninstall_button.hide ();
+
+            // Remove this app from the Installed Apps View
+            MainWindow.installed_view.remove_app.begin (package);
         } catch (Error e) {
             critical (e.message);
             action_stack.set_visible_child_name ("buttons");

=== modified file 'src/Views/AppListView.vala'
--- src/Views/AppListView.vala	2016-06-15 23:49:38 +0000
+++ src/Views/AppListView.vala	2016-06-26 10:34:12 +0000
@@ -76,6 +76,16 @@
         row.show_all ();
         list_box.add (row);
     }
+    
+    public void remove_package (AppCenterCore.Package package) {
+        var pkg_rows = list_box.get_children ();
+        foreach (var row in pkg_rows) {
+            if (((Widgets.PackageRow) row).package == package) {
+                row.destroy ();
+                break;
+            }
+        }
+    }
 
     public Gee.Collection<AppCenterCore.Package> get_packages () {
         var tree_set = new Gee.TreeSet<AppCenterCore.Package> ();

=== modified file 'src/Views/InstalledView.vala'
--- src/Views/InstalledView.vala	2016-06-15 16:34:56 +0000
+++ src/Views/InstalledView.vala	2016-06-26 10:34:12 +0000
@@ -6,12 +6,12 @@
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation, either version 3 of the License, or
  * (at your option) any later version.
- * 
+ *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- * 
+ *
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
@@ -50,6 +50,7 @@
 
     public async void get_apps () {
         unowned Client client = Client.get_default ();
+
         var installed_apps = yield client.get_installed_applications ();
         foreach (var app in installed_apps) {
             app_list_view.add_package (app);
@@ -57,4 +58,19 @@
 
         yield client.get_updates ();
     }
+    
+    public async void add_app (AppCenterCore.Package package) {
+        unowned Client client = Client.get_default ();
+        var installed_apps = yield client.get_installed_applications ();
+        foreach (var app in installed_apps) {
+            if (app == package) {
+                app_list_view.add_package (app);
+                break;
+            }
+        }
+    }
+    
+    public async void remove_app (AppCenterCore.Package package) {
+        app_list_view.remove_package (package);
+    }
 }