From 8ef2626610e6c577c0b65319969535fe9368f285 Mon Sep 17 00:00:00 2001
From: Chris Veilleux <veilleux.chris@gmail.com>
Date: Sun, 16 Jun 2019 18:33:34 -0500
Subject: [PATCH] refactored snackbars to use shared library, fixed a couple of
 bugs around skill settings saving, added logic to show device lists on skills
 without skill settings.

---
 .../skill-settings.component.html             | 29 +++++-----
 .../skill-setting/skill-settings.component.ts | 55 ++++++++++++-------
 .../src/app/modules/skill/skill.component.ts  |  5 +-
 .../app/shared/models/skill-settings.model.ts |  2 +-
 .../app/shared/models/skill_family.model.ts   |  7 +++
 5 files changed, 59 insertions(+), 39 deletions(-)
 create mode 100644 projects/account/src/app/shared/models/skill_family.model.ts

diff --git a/projects/account/src/app/modules/skill/skill-setting/skill-settings.component.html b/projects/account/src/app/modules/skill/skill-setting/skill-settings.component.html
index 5367db4..145e187 100644
--- a/projects/account/src/app/modules/skill/skill-setting/skill-settings.component.html
+++ b/projects/account/src/app/modules/skill/skill-setting/skill-settings.component.html
@@ -2,21 +2,18 @@
     <!-- If we only have one set of settings, we don't need the tabs -->
     <ng-container *ngIf="settings.length === 1">
         <h2 class="mat-h2">Devices</h2>
-        <p *ngIf="settings[0].devices.length === deviceCount" class="mat-body">
-            This skill is installed on all devices
-        </p>
-        <p *ngIf="settings[0].devices.length < deviceCount" class="mat-body">
-            {{settings[0].devices.join(', ')}}
-        </p>
-        <account-skill-setting-section
-            class="mat-elevation-z0"
-            *ngFor="let section of settings[0].settingsDisplay.sections"
-            [sectionDefinition]="section"
-            [settingsValues]="settings[0].settingsValues"
-            (newValue)="onValueChange(settings[0], $event)"
-            (skillOauth)="onSkillOauth($event)"
-        >
-        </account-skill-setting-section>
+        <p class="mat-body">{{buildDeviceList(settings[0])}}</p>
+        <ng-container *ngIf="settings[0].settingsDisplay">
+            <account-skill-setting-section
+                class="mat-elevation-z0"
+                *ngFor="let section of settings[0].settingsDisplay.sections"
+                [sectionDefinition]="section"
+                [settingsValues]="settings[0].settingsValues"
+                (newValue)="onValueChange(settings[0], $event)"
+                (skillOauth)="onSkillOauth($event)"
+            >
+            </account-skill-setting-section>
+        </ng-container>
     </ng-container>
 
     <!-- If we have more than one set of settings, use tabs to delineate that -->
@@ -27,7 +24,7 @@
                     label="Version {{getGroupNumber(settingsGroup, settings)}}"
             >
                 <h2 class="mat-h2">Devices</h2>
-                <p class="mat-body">{{settingsGroup.devices.join(', ')}}</p>
+                <p class="mat-body">{{buildDeviceList(settingsGroup)}}</p>
                 <account-skill-setting-section
                         *ngFor="let section of settingsGroup.settingsDisplay.sections"
                         [sectionDefinition]="section"
diff --git a/projects/account/src/app/modules/skill/skill-setting/skill-settings.component.ts b/projects/account/src/app/modules/skill/skill-setting/skill-settings.component.ts
index f875835..4b7767d 100644
--- a/projects/account/src/app/modules/skill/skill-setting/skill-settings.component.ts
+++ b/projects/account/src/app/modules/skill/skill-setting/skill-settings.component.ts
@@ -3,10 +3,11 @@ import { Observable } from 'rxjs';
 import { tap } from 'rxjs/operators';
 
 import { SettingChange } from '@account/models/setting-change.model';
-import { Skill } from '@account/models/skill.model';
+import { SkillFamily } from '@account/models/skill_family.model';
 import { SkillSettings } from '@account/models/skill-settings.model';
 import { SkillService } from '@account/http/skill.service';
 import { MatSnackBar, MatSnackBarConfig } from '@angular/material';
+import { SnackbarComponent } from 'shared';
 
 const fiveSeconds = 5000;
 
@@ -19,7 +20,7 @@ export class SkillSettingsComponent implements OnInit {
     public disableSave = true;
     @Input() deviceCount: number;
     private snackbarConfig = new MatSnackBarConfig();
-    @Input() skill: Skill;
+    @Input() skill: SkillFamily;
     public skillSettings$: Observable<SkillSettings[]>;
     public skillSettings: SkillSettings[];
     @Output() done = new EventEmitter();
@@ -30,15 +31,26 @@ export class SkillSettingsComponent implements OnInit {
     }
 
     ngOnInit() {
-        this.skillSettings$ = this.skillService.getSkillSettings(this.skill.id).pipe(
+        this.skillSettings$ = this.skillService.getSkillSettings(this.skill.familyName).pipe(
             tap((skillSettings) => { this.skillSettings = skillSettings; })
         );
     }
 
+    buildDeviceList(settingsGroup: SkillSettings): string {
+        let deviceList: string;
+        if (settingsGroup.deviceNames.length === this.deviceCount) {
+            deviceList = 'This skill is installed on all devices';
+        } else {
+            deviceList = settingsGroup.deviceNames.join(', ');
+        }
+
+        return deviceList;
+    }
+
     onValueChange(skillSettings: SkillSettings, newValue: SettingChange): void {
         this.disableSave = false;
         const settingsToChange = this.skillSettings.find(
-            (settings) => settings.settingsValues = skillSettings.settingsValues
+            (settings) => settings.settingsValues === skillSettings.settingsValues
         );
         settingsToChange.settingsValues[newValue.name] = newValue.value;
     }
@@ -54,25 +66,30 @@ export class SkillSettingsComponent implements OnInit {
     }
 
     saveSettings(): void {
-        this.skillService.updateSkillSettings(this.skill.id, this.skillSettings).subscribe(
-            () => {
-                this.snackbar.open(
-                    'Changes to ' + this.skill.name + 'settings saved',
-                    null,
-                    this.snackbarConfig
-                );
-            },
-            () => {
-                this.snackbar.open(
-                    'Error saving changes to ' + this.skill.name + ' settings',
-                    null,
-                    this.snackbarConfig
-                );
-            }
+        this.skillService.updateSkillSettings(this.skill, this.skillSettings).subscribe(
+            () => { this.openSuccessSnackbar(); },
+            () => { this.openErrorSnackbar(); }
         );
         this.done.emit();
     }
 
+    openErrorSnackbar() {
+        const config = new MatSnackBarConfig();
+        config.data = {
+            type: 'error',
+            message: 'Error saving changes to ' + this.skill.name + ' settings'
+        };
+        this.snackbar.openFromComponent(SnackbarComponent, config);
+    }
+
+    openSuccessSnackbar() {
+        const config = new MatSnackBarConfig();
+        config.data = {
+            type: 'success',
+            message: 'Changes to ' + this.skill.name + ' settings saved'};
+        this.snackbar.openFromComponent(SnackbarComponent, config);
+    }
+
     onCancelClick() {
         this.done.emit();
     }
diff --git a/projects/account/src/app/modules/skill/skill.component.ts b/projects/account/src/app/modules/skill/skill.component.ts
index fa7aa09..edf1ca3 100644
--- a/projects/account/src/app/modules/skill/skill.component.ts
+++ b/projects/account/src/app/modules/skill/skill.component.ts
@@ -2,7 +2,7 @@ import { Component, OnInit } from '@angular/core';
 import { Observable } from 'rxjs';
 
 import { environment } from '../../../environments/environment';
-import { Skill } from '@account/models/skill.model';
+import { SkillFamily } from '@account/models/skill_family.model';
 import { SkillService } from '@account/http/skill.service';
 
 export interface WebApps {
@@ -26,7 +26,7 @@ export class SkillComponent implements OnInit {
     public marketplaceLink: string;
     public moreSkillsText: string;
     public mycroftUrls: WebApps = environment.mycroftUrls;
-    public skills$: Observable<Skill[]>;
+    public skills$: Observable<SkillFamily[]>;
     public deviceCount: number;
 
     constructor(private skillService: SkillService) {
@@ -40,7 +40,6 @@ export class SkillComponent implements OnInit {
         );
     }
 
-
     private defineAssistiveText() {
         this.helpText = 'Select a skill below to update its settings, ' +
             'see more information about it, or remove it from your device(s).  ' +
diff --git a/projects/account/src/app/shared/models/skill-settings.model.ts b/projects/account/src/app/shared/models/skill-settings.model.ts
index 2a432a8..9c6a2b3 100644
--- a/projects/account/src/app/shared/models/skill-settings.model.ts
+++ b/projects/account/src/app/shared/models/skill-settings.model.ts
@@ -3,5 +3,5 @@ import { SettingsDisplay } from '@account/models/settings-display.model';
 export interface SkillSettings {
     settingsDisplay: SettingsDisplay;
     settingsValues: any;
-    devices: string[];
+    deviceNames: string[];
 }
diff --git a/projects/account/src/app/shared/models/skill_family.model.ts b/projects/account/src/app/shared/models/skill_family.model.ts
new file mode 100644
index 0000000..7516315
--- /dev/null
+++ b/projects/account/src/app/shared/models/skill_family.model.ts
@@ -0,0 +1,7 @@
+export interface SkillFamily {
+    familyName: string;
+    name: string;
+    hasSettings: boolean;
+    marketId: string;
+    skillIds: string[];
+}