CICD polish #2609

hartra344 · 2018-04-24T21:30:48Z

Mostly adding validation, polish, tests.

nertim · 2018-04-24T22:08:39Z

client/src/app/shared/test-mocks/log.service.mock.ts

+
+@Injectable()
+export class MockLogService {
+


Delete this and move ng-select-helpers in app/test/mocks #Resolved

nertim · 2018-04-24T22:19:53Z

server/Resources/Resources.resx

+    <value>Validating...</value>
+  </data>
+  <data name="vstsReleaseBuildPermissions" xml:space="preserve">
+    <value>You don't have permissions to create a Build Definition or a Release definition on this project. Contact your project administrator</value>


"do not" instead of "don't" #Resolved

Haha, I should have proofread these when I ripped them from vsts 😜

In reply to: 183897388 [](ancestors = 183897388)

nertim · 2018-04-24T22:20:16Z

server/Resources/Resources.resx

+    <value>You don't have permissions to create a Build Definition or a Release definition on this project. Contact your project administrator</value>
+  </data>
+  <data name="vstsNameUnavailable" xml:space="preserve">
+    <value>The account name supplied is not valid. please supply another account name.</value>


Capital P in please. #Resolved

nertim · 2018-04-24T22:23:01Z

client/src/app/shared/directives/invalid-control-message.directive.ts

+        const headers = new Headers();
+        headers.append('Accept', 'image/webp,image/apng,image/*,*/*;q=0.8');
+        headers.append('x-ms-client-request-id', Guid.newGuid());
+        // headers.append('Cache-Control', 'max-age=60000');


delete commented out code. #Resolved

nertim · 2018-04-24T22:24:07Z

client/src/app/shared/directives/invalid-control-message.directive.ts

+        this._cacheService.get(`image/spinner.svg?cacheBreak=${window.appsvc.cacheBreakQuery}`, false, headers)
+            .subscribe(image => {
+                spinnerElement.innerHTML = image.text();
+            });


can we generalize this to a service of its own? #WontFix

it already is for the most part, its in image-loader directive but since this is it's own directive then no because it's doing direct DOM manipulation. This was something elliott and I came up with earlier as a stop gap but not something we want to encourage to be used elsewhere #Resolved

Since this logic is duplicated from load-image.directive.ts, can you just make a static function in LoadImageDirective and just reference it here? That way if we need to modify the logic, it's all in one place.

In reply to: 183901344 [](ancestors = 183901344)

nertim · 2018-04-24T22:26:46Z

client/src/app/shared/models/constants.ts

@@ -270,8 +271,11 @@ export class LogCategories {
    public static readonly serverFarm = 'ServerFarm';
 }

+export class ARM {
+    public static websiteApiVersion = '2015-08-01';
+}


make the class ARMApiVersions.. and than have arm resources within it?

class ARMApiVersions {
public static readonly websiteApiVersion = '2015-08-01'
} #Resolved

nertim · 2018-04-24T22:26:53Z

client/src/app/shared/models/constants.ts

@@ -270,8 +271,11 @@ export class LogCategories {
    public static readonly serverFarm = 'ServerFarm';
 }

+export class ARM {
+    public static websiteApiVersion = '2015-08-01';


static [](start = 11, length = 6)

public static readonly #Resolved

nertim · 2018-04-24T22:28:44Z

client/src/app/shared/services/arm.service.ts

@@ -18,7 +19,7 @@ export class ArmService {
    public armPermissionsVersion = '2015-07-01';
    public armLocksApiVersion = '2015-01-01';
    public storageApiVersion = '2015-05-01-preview';
-    public websiteApiVersion = '2015-08-01';
+    public websiteApiVersion = ARM.websiteApiVersion;


websiteApiVersion = ARM.websiteApiVersion; [](start = 11, length = 42)

can we move the rest in the constants file as well? #WontFix

We can though I might not in this PR, don't want to deal with unreleated fallout. Feel free to open an issue for it though. We should track things like this so it gets done. #WontFix

I don't think it's a big deal, but I'm actually fine if we just want to leave everything defined here. The only change I'd make would be to make these static readonly.

In reply to: 183899111 [](ancestors = 183899111)

nertim · 2018-04-24T22:29:49Z

client/src/app/shared/services/broadcast.service.ts

@@ -99,7 +99,11 @@ export class BroadcastService {
        }

        return subject
-            .do(e => this._logService.verbose(LogCategories.broadcastService, BroadcastEvent[e.eventType]))
+            .do(e => {
+                if (this._logService) {


_logService [](start = 25, length = 11)

man I think we've had this conversation before.. and I am sorry but I forget the result of it.. why would _logService be null? #Resolved

hmm.. what I dont like about this is that we are updating our product code just for the tests. @ehamai what are your thoughts? I would prefer the prod code not make any concessions for test run time. #Resolved

@nertim - I sort of agree with you. I think prod code should be written in a way that's easily testable. But you're right, in this case I don't think that it makes sense to require product code to have to think about conditions of the environment that would only be relevant to a test. #Resolved

nertim · 2018-04-24T22:31:42Z

client/src/app/shared/test-mocks/ng-select-helpers.mock.ts

+    Backspace = 8
+}
+
+export class NgSelectTestHelpers {


NgSelectTestHelpers [](start = 13, length = 19)

move to app/test/mocks #Resolved

nertim · 2018-04-24T22:32:25Z

client/src/app/site/deployment-center/deployment-center.module.ts

@@ -24,6 +24,7 @@ import { ConfigureLocalGitComponent } from './deployment-center-setup/step-confi
 import { SidebarModule } from 'ng-sidebar';
 import { NgSelectModule } from '@ng-select/ng-select';

+
 @NgModule({


remove extra line #Resolved

ehamai · 2018-04-25T16:13:49Z

.../app/site/deployment-center/deployment-center-setup/step-complete/step-complete.component.ts

-                this._busyManager.clearBusy();
-                this._broadcastService.broadcastEvent(BroadcastEvent.ReloadDeploymentCenter);
+                this.clearBusy();
+                this._broadcastService.broadcastEvent<any>(BroadcastEvent.ReloadDeploymentCenter);


any [](start = 54, length = 3)

void #Resolved

ehamai · 2018-04-25T16:15:55Z

...eployment-center-setup/step-configure/configure-bitbucket/configure-bitbucket.component.html

@@ -5,7 +5,7 @@ <h3 class="first-config-heading">{{'code' | translate}}</h3>
    <div class="setting-wrapper">
      <div class="setting-label">{{'repository' | translate}}</div>
      <div class="setting-control-container">
-        <ng-select class="custom-select" [items]="RepoList" bindLabel="displayLabel" bindValue="value" placeholder="Select Repo"
+        <ng-select id='configure-bitbucket-repo-select' class="custom-select" [items]="RepoList" bindLabel="displayLabel" bindValue="value" placeholder="Select Repo"


Select Repo [](start = 153, length = 11)

resx for placeholder #Resolved

ehamai · 2018-04-25T16:16:21Z

...er/deployment-center-setup/step-configure/configure-dropbox/configure-dropbox.component.html

@@ -6,8 +6,8 @@ <h3 class="first-config-heading">{{'code' | translate}}</h3>
    <div class="setting-wrapper">
      <div class="setting-label">{{'folder' | translate}}</div>
      <div class="setting-control-container">
-        <ng-select class="custom-select" [items]="folderList" bindLabel="displayLabel" bindValue="value" placeholder="Select Folder"
-          [clearable]="false" [(ngModel)]="selectedFolder"  [loading]="foldersLoading"  formControlName="repoUrl ">
+        <ng-select id="configure-dropbox-folder-select" class="custom-select" [items]="folderList" bindLabel="displayLabel" bindValue="value" placeholder="Select Folder"


"Select Folder" [](start = 154, length = 15)

resx #Resolved

ehamai · 2018-04-25T16:20:18Z

client/src/app/site/deployment-center/deployment-center-setup/validators/slot-name-validator.ts

+                return Observable.of({ invalidSiteName: _translateService.instant(PortalResources.validation_siteNameInvalidChar).format(matchingChar[0]) });
+            }
+
+            return _cacheService.getArm(`${_siteId}/slots`)


_cacheService.getArm(${_siteId}/slots) [](start = 19, length = 40)

call SiteService.getSlots instead #Resolved

Also don't forget to check for response success.

In reply to: 184122441 [](ancestors = 184122441)

also made a non committed fix for this to just be siteid and not the old string

In reply to: 184122617 [](ancestors = 184122617,184122441)

ehamai · 2018-04-25T16:22:33Z

client/src/app/site/deployment-center/deployment-center-setup/validators/slot-name-validator.ts

+                    const slots = r.json().value as ArmObj<Site>[];
+                    let existingSlot = null;
+                    const name = control.value;
+                    if (name) {


Shouldn't we avoid making the HTTP request if the control doesn't have a value? #Resolved

We don't make the http call if there is no value, in fact we don't make any calls until it's more than 2 characters #Resolved

Okay. Just wondering why the name check is necessary then. You're already checking for length in the beginning. If the value were null, then you would have had a null ref thrown in the beginning. #Resolved

nertim · 2018-04-25T16:25:26Z

client/src/app/shared/models/constants.ts

 export class SubscriptionQuotaIds {
-    public static dreamSparkQuotaId: string = 'DreamSpark_2015-02-01';
+    public static dreamSparkQuotaId = 'DreamSpark_2015-02-01';


public static readonly. #Resolved

ehamai · 2018-04-25T16:29:45Z

...te/deployment-center/deployment-center-setup/wizard-logic/deployment-center-state-manager.ts

        portalService: PortalService) {
        this.resourceIdStream$.switchMap(r => {
            this._resourceId = r;
-            return this._armService.get(this._resourceId)
+            return this._cacheService.getArm(this._resourceId);


this._cacheService.getArm(this._resourceId); [](start = 19, length = 44)

siteService.getSite #Resolved

ehamai · 2018-04-25T16:31:33Z

...te/deployment-center/deployment-center-setup/wizard-logic/deployment-center-state-manager.ts

            properties: payload
        }).map(r => r.json());
    }

    private _deployVsts() {
        return this._startVstsDeployment().concatMap(id => {
-            return Observable.interval(1000)
+            return Observable.timer(1000, 1000)


1000 [](start = 42, length = 4)

I'm confused about this. You're specifying a poll interval but you're taking the first result, which makes it unnecessary. Did you mean to only specify the first param? #ByDesign

Wait that doesn't sound right. The first param is the initial wait time. The second param is how long each subsequent event will take to fire. https://www.learnrxjs.io/operators/creation/timer.html

Also if you need to ensure that it's ready, shouldn't you actually do polling instead of hoping that 1 sec is enough? #ByDesign

ehamai · 2018-04-25T16:32:12Z

client/src/app/site/deployment-center/Models/deployment-data.ts

-    sourceControls: ArmObj<any>;
-    publishingUser: ArmObj<{
+    publishingCredentials?: ArmObj<PublishingCredentials>;
+    sourceControls?: ArmObj<any>;


any [](start = 28, length = 3)

Please define a type #Resolved

Sorry I'm pointing at sourceControls #Resolved

ehamai

🕐

ehamai · 2018-04-25T16:37:50Z

client/src/app/shared/services/arm-embedded.service.ts

@@ -28,14 +28,6 @@ export class ArmEmbeddedService extends ArmService {
        '/api/'
    ];

-    // eventually move to sever
-    public static getRPUrl(): string {


getRPUrl [](start = 18, length = 8)

If you're going to remove the duplicate code, I think it makes sense to remove it from the ArmService instead of the ArmEmbeddedService. #ByDesign

ehamai

🕐

ehamai

🕐

nertim · 2018-04-25T17:58:00Z

client/src/app/site/deployment-center/deployment-center.component.ts

@@ -31,6 +33,7 @@ export class DeploymentCenterComponent implements OnDestroy {
    public resourceId: string;
    public viewInfoStream = new Subject<TreeViewInfo<SiteData>>();
    public viewInfo: TreeViewInfo<SiteData>;
+    public dashboardProviderType: ProviderDashboardType = '';


private _dashboardProviderType #Resolved

unless if it being used externally.. but I didn't see it. #Resolved

its not yet, it will be but I'll make it private for now

In reply to: 184153622 [](ancestors = 184153622)

nertim · 2018-04-25T18:07:41Z

...ployment-center/deployment-center-setup/step-source-control/step-source-control.component.ts


 @Component({
    selector: 'app-step-source-control',
    templateUrl: './step-source-control.component.html',
    styleUrls: ['./step-source-control.component.scss', '../deployment-center-setup.component.scss']
 })
 export class StepSourceControlComponent {
+
+   private _authProviderSpots = {


Please dont kill me!!!! its the last time I'll harp on this... and it is a nit 😀

How about moving this object below the provider cards array and doing a find for each provider type? this will end up with just the find getting called only once when the class is constructed...

Please dont kill me 😁 #WontFix

I just tried this and it still looked messy. Will think about this but honestly any more than this feels like over engineering for something that I dont think will be a big problem

In reply to: 184156520 [](ancestors = 184156520)

nertim

Thank you 😃 I have a couple more comments.. but nothing blocking.

* Introduce application insights based monitoring view (#2516) Introduce application insights based monitoring view and do a little of refactor on the existing monitoring view. * Redoing the extension cache fix (#2555) * [SpecPicker] Adding individual banner to each group. Also several other bug fixes (#2552) * Removing AzureFunctions.AngularClient folder * Pass context for create (#2559) * Initial upload experience for Prod Functions (#2557) * Hotfix for set model after token loads (#2562) (#2563) * Hotfix for set model after token loads * update v1 version number * adding top level tslint for codacy * adjusting linting rules for codacy * adding azure-cli script for swapping ux slots (#2568) * add control with placeholder logic * add lodash to blacklisted imports (#2572) * update setting values * adding moment to blacklist as well (#2573) * updating production slot swap to fix a couple of bugs * update resources * Add metrics feature to monitoring section (#2578) Add metrics feature link to monitoring page. * Fixes #2480 - Add accessibility support to spec picker (#2575) * Fixes #2480 - Add accessibility to spec picker * Updated from PR comments * Add ~2 to list of stable runtimes. Closes #2564 * Set readOnly state for prod dynmic linux. Closes #2565 * Configure page for Application Insights (#2560) monitor configure page * update check scenario * Fix slot locks * Update name of site config property * poll function logs if the app is dynamic linux. Closes #2571 * Update id check (#2588) * PR updates * Wait on existing polling request if it takes longer than 2 seconds. * more pr updates * remove console log * Add separate list for recommended skus and add resources for strings (#2585) * Add separate list for recommended skus and add resources for strings * Removing duplicate identifier * Updated from feedback * updated feedback * hide behind feature flag * Inject build number to package.json and an api * Collect perf counters from backend * Add build defintion * as per discussion with elliot prepending 1.0. to the build number for version number * fixing issue where server won't run locally due to invalid package version * make server file pull version from package.json * update axios to 0.18 * set keepAlive to true for proxied connections * Fix issue where UI doesn't handle scaling an isolated SKU. Plus other bug fixes (#2599) * Fix issue where UI doesn't handle scaling an isolated SKU. Plus other bug fixes * Remove shield but disable apply button when isolated scale operation in progress * Updated from PR feedback * Updated links and changed wording on ACU language * Add global mocks for log and telemetry service (#2601) Add global mocks for log and telemetry service * Fix initial items Alex found. (#2598) * prefix class name (#2602) * Add dark theme to spec picker. Closes #2481 (#2604) * Removing feature flag and enabling new spec picker by default (#2605) * Removing feature flag and enabling new spec picker by default * remove url import * CICD polish (#2609) * Changing auth token to be passed through body of passthrough rather t… (#2618) * Changing auth token to be passed through body of passthrough rather than header * Codacy * Update the scenario status for all environments for AI based monitoring. (#2615) Update the scenario status for all environments for AI based monitoring. * Spec picker bugs for styling and xenon (#2630) * Spec picker bugs for styling and xenon * PR feedback * More styling, fwdlinks, and spec clean up - Fixes #2627, Fixes #2612, Fixes #2628 (#2632) * Add support for linux pricing (#2635) * Fix up some acessibility items and dark theme items. (#2634) * Inject empty headers to urls going to our backend, not using arm headers (#2636) * Inject empty headers to urls going to our backend, not using arm headers * Codacy * Add busy state when opening create sidebar (#2633) * add busy state when opening create sidebar * use busy manager * remove underscore * Handle busy error (#2637) * Changing desgin of cards for source and build providers (#2639) * Changing desgin of cards for source and build providers * udpating strings * PR Comments * changing settings to not show runtime info if compose or kubernetes i… (#2640) * changing settings to not show runtime info if compose or kubernetes is used * PR Comment * KUBE| * Make the datetime more granular and add sorting capability. (#2647) * Adding diagnose and solve problems (#2641) * Adding diagnose and solve problems * Removing RBAC check * Sort app settings by name (#2648) * Use resources for FTP/S options (#2650) * Fix the table styling, delay message, and linux function configuration page (#2649) Fix the table styling, delay message, and linux function configuration page. * Update extension install message (#2651) * update extension install message * + is * Add a close button on top right. This aligns with some of the other slide outs with the close buttons. (#2652) * Add a close button on top right. This aligns with some of the other slide outs with the close buttons. * Change to Subject() * Move application insights link to essentials col (#2653) * Move application insights link to essentials col Indicate number of results returned in query and provide a link to open in the AI query editor. * PR Comments. * Fixing issue where S2 and S3 are not available for Linux (#2655) * Fixing issue where S2 and S3 are not available for Linux * Removing kinds reference * String change for AI configuration instructions. (#2656) * Remove FTP feature flag (#2657) * Hardcoding Linux Pv2 regions until the georegions API works properly (#2658) * Fix read\write -> read/write. Closes #2507 (#2659) * Adding discount banner for existing linux isolated group in spec picker (#2660) * Updating MSI title to preview * Updating resource strings

Squash/Merge

4489cd2

hartra344 requested review from nertim, ahmelsayed, ehamai and ammanifold April 24, 2018 21:30

Travis Harris added 2 commits April 24, 2018 14:39

Fixing Test

f2a3c84

Codacy Fixes

8e2a3f8

Azure deleted a comment Apr 24, 2018

Azure deleted a comment from ahmelsayed Apr 24, 2018

Azure deleted a comment Apr 24, 2018

nertim reviewed Apr 24, 2018

View reviewed changes

ehamai reviewed Apr 25, 2018

View reviewed changes

nertim reviewed Apr 25, 2018

View reviewed changes

ehamai reviewed Apr 25, 2018

View reviewed changes

ehamai requested changes Apr 25, 2018

View reviewed changes

ehamai reviewed Apr 25, 2018

View reviewed changes

ehamai requested changes Apr 25, 2018

View reviewed changes

PR Comments

7d913f2

nertim reviewed Apr 25, 2018

View reviewed changes

PR Comments"

faa0a34

Azure deleted a comment Apr 25, 2018

nertim reviewed Apr 25, 2018

View reviewed changes

nertim approved these changes Apr 25, 2018

View reviewed changes

Pr Comments

02ac34b

Azure deleted a comment Apr 25, 2018

tslint exception for codacy

d45e2b2

Azure deleted a comment Apr 25, 2018

Travis Harris added 3 commits April 25, 2018 13:02

change case of deployment-enums file

552d92d

remove deployment-enums

306d25d

readd deployment-enums with correct casing

423b0ca

ehamai approved these changes Apr 25, 2018

View reviewed changes

hartra344 merged commit 4f3cadb into dev Apr 25, 2018

hartra344 deleted the cicd-semi-final branch April 25, 2018 22:49

CICD polish #2609

CICD polish #2609

Conversation

hartra344 commented Apr 24, 2018

nertim Apr 24, 2018 • edited by hartra344

Choose a reason for hiding this comment

nertim Apr 24, 2018 • edited by hartra344

Choose a reason for hiding this comment

hartra344 Apr 25, 2018

Choose a reason for hiding this comment

nertim Apr 24, 2018 • edited by hartra344

Choose a reason for hiding this comment

nertim Apr 24, 2018 • edited by hartra344

Choose a reason for hiding this comment

nertim Apr 24, 2018 • edited by hartra344

Choose a reason for hiding this comment

hartra344 Apr 24, 2018 • edited

Choose a reason for hiding this comment

ehamai Apr 25, 2018

Choose a reason for hiding this comment

nertim Apr 24, 2018 • edited by hartra344

Choose a reason for hiding this comment

nertim Apr 24, 2018 • edited by hartra344

Choose a reason for hiding this comment

nertim Apr 24, 2018 • edited by hartra344

Choose a reason for hiding this comment

hartra344 Apr 24, 2018 • edited

Choose a reason for hiding this comment

ehamai Apr 25, 2018

Choose a reason for hiding this comment

nertim Apr 24, 2018 • edited by hartra344

Choose a reason for hiding this comment

nertim Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

ehamai Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

nertim Apr 24, 2018 • edited by hartra344

Choose a reason for hiding this comment

nertim Apr 24, 2018 • edited by hartra344

Choose a reason for hiding this comment

ehamai Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

ehamai Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

ehamai Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

ehamai Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

ehamai Apr 25, 2018

Choose a reason for hiding this comment

hartra344 Apr 25, 2018

Choose a reason for hiding this comment

ehamai Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

hartra344 Apr 25, 2018 • edited

Choose a reason for hiding this comment

ehamai Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

nertim Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

ehamai Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

ehamai Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

ehamai Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

ehamai Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

ehamai Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

ehamai left a comment

Choose a reason for hiding this comment

ehamai Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

ehamai left a comment

Choose a reason for hiding this comment

ehamai left a comment

Choose a reason for hiding this comment

nertim Apr 25, 2018 • edited by hartra344

Choose a reason for hiding this comment

nertim Apr 24, 2018 •

edited by hartra344

nertim Apr 24, 2018 •

edited by hartra344

nertim Apr 24, 2018 •

edited by hartra344

nertim Apr 24, 2018 •

edited by hartra344

nertim Apr 24, 2018 •

edited by hartra344

hartra344 Apr 24, 2018 •

edited

nertim Apr 24, 2018 •

edited by hartra344

nertim Apr 24, 2018 •

edited by hartra344

nertim Apr 24, 2018 •

edited by hartra344

hartra344 Apr 24, 2018 •

edited

nertim Apr 24, 2018 •

edited by hartra344

nertim Apr 25, 2018 •

edited by hartra344

ehamai Apr 25, 2018 •

edited by hartra344

nertim Apr 24, 2018 •

edited by hartra344

nertim Apr 24, 2018 •

edited by hartra344

ehamai Apr 25, 2018 •

edited by hartra344

ehamai Apr 25, 2018 •

edited by hartra344

ehamai Apr 25, 2018 •

edited by hartra344

ehamai Apr 25, 2018 •

edited by hartra344

ehamai Apr 25, 2018 •

edited by hartra344

hartra344 Apr 25, 2018 •

edited

ehamai Apr 25, 2018 •

edited by hartra344

nertim Apr 25, 2018 •

edited by hartra344

ehamai Apr 25, 2018 •

edited by hartra344

ehamai Apr 25, 2018 •

edited by hartra344

ehamai Apr 25, 2018 •

edited by hartra344

ehamai Apr 25, 2018 •

edited by hartra344

ehamai Apr 25, 2018 •

edited by hartra344

ehamai Apr 25, 2018 •

edited by hartra344

nertim Apr 25, 2018 •

edited by hartra344

nertim Apr 25, 2018 •

edited by hartra344

nertim Apr 25, 2018 •

edited by hartra344