Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: UI for the new "Conencted" badge #1144

Merged
merged 2 commits into from
Jan 17, 2025
Merged

feat: UI for the new "Conencted" badge #1144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b567e9
feat: UI for the new "Conencted" badge
JustARatherRidiculouslyLongUsername Jan 9, 2025
be16dd3
feat: fetch data from integrations API and show connected apps (#1143)
JustARatherRidiculouslyLongUsername Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/app/core/models/enum/enum.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,8 @@ export enum SizeOption {
export enum ThemeOption {
BRAND = 'brand',
LIGHT = 'light',
DARK = 'dark'
DARK = 'dark',
SUCCESS = 'success'
}

export enum QBDPreRequisiteState {
Expand Down
11 changes: 11 additions & 0 deletions src/app/core/models/integrations/integrations.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
import { AccountingIntegrationApp, AppUrl, ClickEvent, InAppIntegration, IntegrationView } from "../enum/enum.model";

export type Integration = {
id: number;
org_id: string;
org_name: string;
tpa_id: string;
tpa_name: string;
type: string;
is_active: boolean;
is_beta: boolean;
}

export type IntegrationsView = {
[IntegrationView.ACCOUNTING]: boolean,
[IntegrationView.ALL]: boolean,
Expand Down
16 changes: 16 additions & 0 deletions src/app/core/services/common/integrations.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { TestBed } from '@angular/core/testing';

import { IntegrationsService } from './integrations.service';

xdescribe('IntegrationsService', () => {
let service: IntegrationsService;

beforeEach(() => {
TestBed.configureTestingModule({});
service = TestBed.inject(IntegrationsService);
});

it('should be created', () => {
expect(service).toBeTruthy();
});
});
Comment on lines +5 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Enable tests and add coverage for getIntegrations method.

The test suite has the following issues:

  1. Tests are disabled with xdescribe
  2. Missing test coverage for getIntegrations method
  3. No error handling tests
  4. No API response mocking

Here's how to improve the test coverage:

-xdescribe('IntegrationsService', () => {
+describe('IntegrationsService', () => {
   let service: IntegrationsService;
+  let apiService: jasmine.SpyObj<ApiService>;
+  let helperService: jasmine.SpyObj<HelperService>;
 
   beforeEach(() => {
+    const apiSpy = jasmine.createSpyObj('ApiService', ['get']);
+    const helperSpy = jasmine.createSpyObj('HelperService', ['setBaseApiURL']);
     TestBed.configureTestingModule({
-      {}
+      providers: [
+        IntegrationsService,
+        { provide: ApiService, useValue: apiSpy },
+        { provide: HelperService, useValue: helperSpy }
+      ]
     });
     service = TestBed.inject(IntegrationsService);
+    apiService = TestBed.inject(ApiService) as jasmine.SpyObj<ApiService>;
+    helperService = TestBed.inject(HelperService) as jasmine.SpyObj<HelperService>;
   });
 
   it('should be created', () => {
     expect(service).toBeTruthy();
   });
+
+  describe('getIntegrations', () => {
+    it('should set base URL and return integrations', (done) => {
+      const mockIntegrations: Integration[] = [
+        {
+          id: 1,
+          org_id: 'org1',
+          org_name: 'Org 1',
+          tpa_id: 'tpa1',
+          tpa_name: 'TPA 1',
+          type: 'type1',
+          is_active: true,
+          is_beta: false
+        }
+      ];
+
+      helperService.setBaseApiURL.and.returnValue(undefined);
+      apiService.get.and.returnValue(of(mockIntegrations));
+
+      service.getIntegrations().subscribe({
+        next: (integrations) => {
+          expect(helperService.setBaseApiURL).toHaveBeenCalledWith(AppUrl.INTEGRATION);
+          expect(apiService.get).toHaveBeenCalledWith('/integrations/', {});
+          expect(integrations).toEqual(mockIntegrations);
+          done();
+        },
+        error: done.fail
+      });
+    });
+
+    it('should handle API errors', (done) => {
+      const errorResponse = new HttpErrorResponse({
+        error: 'test 404 error',
+        status: 404,
+        statusText: 'Not Found'
+      });
+
+      helperService.setBaseApiURL.and.returnValue(undefined);
+      apiService.get.and.returnValue(throwError(() => errorResponse));
+
+      service.getIntegrations().subscribe({
+        next: () => done.fail('should have failed'),
+        error: (error) => {
+          expect(error.status).toBe(404);
+          done();
+        }
+      });
+    });
+  });
 });

Committable suggestion skipped: line range outside the PR's diff.

22 changes: 22 additions & 0 deletions src/app/core/services/common/integrations.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { Injectable } from '@angular/core';
import { AppUrl } from '../../models/enum/enum.model';
import { ApiService } from './api.service';
import { HelperService } from './helper.service';
import { Observable } from 'rxjs';
import { Integration } from '../../models/integrations/integrations.model';

@Injectable({
providedIn: 'root'
})
export class IntegrationsService {
constructor(
private apiService: ApiService,
private helper: HelperService
) {
}

getIntegrations(): Observable<Integration[]> {
this.helper.setBaseApiURL(AppUrl.INTEGRATION);
return this.apiService.get(`/integrations/`, {});
}
Comment on lines +18 to +21
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling and retry logic.

The getIntegrations method should:

  1. Handle API errors gracefully
  2. Implement retry logic for transient failures
  3. Consider caching the results to improve performance
 getIntegrations(): Observable<Integration[]> {
   this.helper.setBaseApiURL(AppUrl.INTEGRATION);
-  return this.apiService.get(`/integrations/`, {});
+  return this.apiService.get<Integration[]>('/integrations/', {}).pipe(
+    retry({
+      count: 3,
+      delay: (error, retryCount) => {
+        if (error.status === 404) {
+          return EMPTY;
+        }
+        return timer(1000 * retryCount);
+      }
+    }),
+    catchError((error: HttpErrorResponse) => {
+      console.error('Error fetching integrations:', error);
+      return throwError(() => error);
+    })
+  );
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
getIntegrations(): Observable<Integration[]> {
this.helper.setBaseApiURL(AppUrl.INTEGRATION);
return this.apiService.get(`/integrations/`, {});
}
getIntegrations(): Observable<Integration[]> {
this.helper.setBaseApiURL(AppUrl.INTEGRATION);
return this.apiService.get<Integration[]>('/integrations/', {}).pipe(
retry({
count: 3,
delay: (error, retryCount) => {
if (error.status === 404) {
return EMPTY;
}
return timer(1000 * retryCount);
}
}),
catchError((error: HttpErrorResponse) => {
console.error('Error fetching integrations:', error);
return throwError(() => error);
})
);
}

}
2 changes: 1 addition & 1 deletion src/app/integrations/integrations-routing.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const routes: Routes = [
component: LandingComponent
},
{
path: 'landing-v2',
path: 'landing_v2',
component: LandingV2Component
},
{
Expand Down
88 changes: 64 additions & 24 deletions src/app/integrations/landing-v2/landing-v2.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ <h3 class="landing-v2--section-heading">
href="mailto:[email protected]">support{{'@'}}fylehq.com</a>
</p>
</header>
<div class="tw-flex tw-text-14-px tw-border-y tw-border-separator">
<div *ngIf="!exposeC1Apps" class="tw-flex tw-text-14-px tw-border-y tw-border-separator">
<div class="landing-v2--tab" [ngClass]="{'tw-text-menu-inactive-text-color': !integrationTabs.ALL}"
(click)="switchView(IntegrationsView.ALL)">
All
Expand Down Expand Up @@ -41,9 +41,13 @@ <h3 class="landing-v2--section-heading">
<div *ngIf="isAppShown('NETSUITE')">
<div class="landing-v2--accounting-app"
(click)="openAccountingIntegrationApp(AccountingIntegrationApp.NETSUITE)">
<div class="tw-flex tw-justify-between">
<img src="assets/logos/netsuite-logo.png" />
<button class="btn-connect">Connect</button>
<div class="tw-flex tw-justify-between tw-items-center">
<img src="assets/logos/netsuite-logo-new.png" />
@if (isAppConnected('NETSUITE')) {
<app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge>
} @else {
<button class="btn-connect">Connect</button>
}
</div>
<div>
<span class="landing-v2--accounting-app-name">
Expand All @@ -57,9 +61,13 @@ <h3 class="landing-v2--section-heading">
</div>
<div *ngIf="isAppShown('INTACCT')" class="landing-v2--accounting-app"
(click)="openAccountingIntegrationApp(AccountingIntegrationApp.SAGE_INTACCT)">
<div class="tw-flex tw-justify-between">
<img src="assets/logos/intacct-logo.png" />
<button class="btn-connect">Connect</button>
<div class="tw-flex tw-justify-between tw-items-center">
<img src="assets/logos/intacct-logo-new.png" />
@if (isAppConnected('INTACCT')) {
<app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge>
} @else {
<button class="btn-connect">Connect</button>
}
</div>
<div>
<span class="landing-v2--accounting-app-name">
Expand All @@ -72,9 +80,13 @@ <h3 class="landing-v2--section-heading">
</div>
<div *ngIf="isAppShown('QBO')" class="landing-v2--accounting-app"
(click)="openAccountingIntegrationApp(AccountingIntegrationApp.QBO)">
<div class="tw-flex tw-justify-between">
<div class="tw-flex tw-justify-between tw-items-center">
<img src="assets/logos/quickbooks-logo.png" class="!tw-h-[30.7px]" />
<button class="btn-connect">Connect</button>
@if (isAppConnected('QBO')) {
<app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge>
} @else {
<button class="btn-connect">Connect</button>
}
</div>
<div>
<span class="landing-v2--accounting-app-name">
Expand All @@ -87,9 +99,13 @@ <h3 class="landing-v2--section-heading">
</div>
<div *ngIf="isAppShown('XERO')" class="landing-v2--accounting-app"
(click)="openAccountingIntegrationApp(AccountingIntegrationApp.XERO)">
<div class="tw-flex tw-justify-between">
<img src="assets/logos/xero-logo.png" />
<button class="btn-connect">Connect</button>
<div class="tw-flex tw-justify-between tw-items-center">
<img src="assets/logos/xero-logo-new.png" />
@if (isAppConnected('XERO')) {
<app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge>
} @else {
<button class="btn-connect">Connect</button>
}
</div>
<div>
<span class="landing-v2--accounting-app-name">
Expand All @@ -102,9 +118,13 @@ <h3 class="landing-v2--section-heading">
</div>
<div *ngIf="isAppShown('QBD')" class="landing-v2--accounting-app"
(click)="openInAppIntegration(InAppIntegration.QBD)">
<div class="tw-flex tw-justify-between">
<div class="tw-flex tw-justify-between tw-items-center">
<img src="assets/logos/quickbooks-logo.png" class="!tw-h-[30.7px]" />
<button class="btn-connect">Connect</button>
@if (isAppConnected('QBD')) {
<app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge>
} @else {
<button class="btn-connect">Connect</button>
}
</div>
<div>
<span class="landing-v2--accounting-app-name">
Expand All @@ -118,9 +138,13 @@ <h3 class="landing-v2--section-heading">
<!-- Direct -->
<div *ngIf="isAppShown('QBD_DIRECT')" class="landing-v2--accounting-app"
(click)="openInAppIntegration(InAppIntegration.QBD_DIRECT)">
<div class="tw-flex tw-justify-between">
<div class="tw-flex tw-justify-between tw-items-center">
<img src="assets/logos/quickbooks-logo.png" class="!tw-h-[30.7px]" />
<button class="btn-connect">Connect</button>
@if (isAppConnected('QBD_DIRECT')) {
<app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge>
} @else {
<button class="btn-connect">Connect</button>
}
</div>
<span class="landing-v2--accounting-app-name tw-items-center tw-gap-4">
<div>
Expand All @@ -132,9 +156,13 @@ <h3 class="landing-v2--section-heading">
</div>
<div *ngIf="isAppShown('SAGE300')" class="landing-v2--accounting-app"
(click)="openInAppIntegration(InAppIntegration.SAGE300)">
<div class="tw-flex tw-justify-between">
<div class="tw-flex tw-justify-between tw-items-center">
<img src="assets/logos/sage300-logo.png" class="tw-py-[4px]" />
<button class="btn-connect">Connect</button>
@if (isAppConnected('SAGE300')) {
<app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge>
} @else {
<button class="btn-connect">Connect</button>
}
</div>
<span class="landing-v2--accounting-app-name tw-items-center tw-gap-4">
<div>
Expand All @@ -147,9 +175,13 @@ <h3 class="landing-v2--section-heading">
</div>
<div *ngIf="isAppShown('BUSINESS_CENTRAL')" class="landing-v2--accounting-app"
(click)="openInAppIntegration(InAppIntegration.BUSINESS_CENTRAL)">
<div class="tw-flex tw-justify-between">
<div class="tw-flex tw-justify-between tw-items-center">
<img src="assets/logos/BusinessCentral-logo.svg" />
<button class="btn-connect">Connect</button>
@if (isAppConnected('BUSINESS_CENTRAL')) {
<app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge>
} @else {
<button class="btn-connect">Connect</button>
}
</div>
<span class="landing-v2--accounting-app-name tw-items-center tw-gap-4">
<div>
Expand All @@ -163,9 +195,13 @@ <h3 class="landing-v2--section-heading">

<div *ngIf="isAppShown('BAMBOO_HR')">
<div class="landing-v2--accounting-app" (click)="openInAppIntegration(InAppIntegration.BAMBOO_HR)">
<div class="tw-flex tw-justify-between">
<div class="tw-flex tw-justify-between tw-items-center">
<img src="assets/logos/bamboo-hr-logo.png" />
<button class="btn-connect">Connect</button>
@if (isAppConnected('BAMBOO_HR')) {
<app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge>
} @else {
<button class="btn-connect">Connect</button>
}
</div>
<div>
<span class="landing-v2--accounting-app-name">
Expand All @@ -180,9 +216,13 @@ <h3 class="landing-v2--section-heading">

<div *ngIf="isAppShown('TRAVELPERK')">
<div class="landing-v2--accounting-app" (click)="openInAppIntegration(InAppIntegration.TRAVELPERK)">
<div class="tw-flex tw-justify-between">
<div class="tw-flex tw-justify-between tw-items-center">
<img src="assets/logos/travelperk-logo.png" class="tw-py-[5px]" />
<button class="btn-connect">Connect</button>
@if (isAppConnected('TRAVELPERK')) {
<app-badge text="Connected" [theme]="ThemeOption.SUCCESS"></app-badge>
} @else {
<button class="btn-connect">Connect</button>
}
</div>
<div>
<span class="landing-v2--accounting-app-name">
Expand Down
35 changes: 34 additions & 1 deletion src/app/integrations/landing-v2/landing-v2.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { QboAuthService } from 'src/app/core/services/qbo/qbo-core/qbo-auth.serv
import { XeroAuthService } from 'src/app/core/services/xero/xero-core/xero-auth.service';
import { exposeAppConfig } from 'src/app/branding/expose-app-config';
import { NetsuiteAuthService } from 'src/app/core/services/netsuite/netsuite-core/netsuite-auth.service';
import { IntegrationsService } from 'src/app/core/services/common/integrations.service';

@Component({
selector: 'app-landing-v2',
Expand All @@ -31,6 +32,10 @@ export class LandingV2Component implements OnInit {

org: Org = this.orgService.getCachedOrg();

private connectedApps: IntegrationAppKey[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Initialize the connectedApps array to prevent undefined access.

The connectedApps array should be initialized to prevent potential undefined access in the isAppConnected method.

-  private connectedApps: IntegrationAppKey[];
+  private connectedApps: IntegrationAppKey[] = [];
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private connectedApps: IntegrationAppKey[];
private connectedApps: IntegrationAppKey[] = [];


readonly exposeC1Apps = brandingFeatureConfig.exposeC1Apps;

private readonly integrationTabsInitialState: IntegrationsView = {
[IntegrationView.ACCOUNTING]: false,
[IntegrationView.HRMS]: false,
Expand Down Expand Up @@ -72,6 +77,19 @@ export class LandingV2Component implements OnInit {
[AccountingIntegrationApp.XERO]: '/integrations/xero'
};

private readonly tpaNameToIntegrationKeyMap: Record<string, IntegrationAppKey> = {
'Fyle Netsuite Integration': 'NETSUITE',
'Fyle Sage Intacct Integration': 'INTACCT',
'Fyle Quickbooks Integration': 'QBO',
'Fyle Xero Integration': 'XERO',
'Fyle Quickbooks Desktop (IIF) Integration': 'QBD',
'Fyle Quickbooks Desktop Integration': 'QBD_DIRECT',
'Fyle Sage 300 Integration': 'SAGE300',
'Fyle Business Central Integration': 'BUSINESS_CENTRAL',
'Fyle TravelPerk Integration': 'TRAVELPERK',
'Fyle BambooHR Integration': 'BAMBOO_HR'
};

readonly brandingConfig = brandingConfig;

readonly isINCluster = this.storageService.get('cluster-domain')?.includes('in1');
Expand Down Expand Up @@ -100,7 +118,8 @@ export class LandingV2Component implements OnInit {
private router: Router,
private siAuthService: SiAuthService,
private storageService: StorageService,
private orgService: OrgService
private orgService: OrgService,
private integrationService: IntegrationsService
) { }


Expand Down Expand Up @@ -146,6 +165,10 @@ export class LandingV2Component implements OnInit {
return false;
}

isAppConnected(appKey: IntegrationAppKey) {
return this.connectedApps?.includes(appKey);
}

openAccountingIntegrationApp(accountingIntegrationApp: AccountingIntegrationApp): void {

// For local dev, we perform auth via loginWithRefreshToken on Fyle login redirect (/auth/redirect)
Expand Down Expand Up @@ -215,7 +238,17 @@ export class LandingV2Component implements OnInit {
});
}

private storeConnectedApps() {
this.integrationService.getIntegrations().subscribe(integrations => {
const tpaNames = integrations.map(integration => integration.tpa_name);
const connectedApps = tpaNames.map(tpaName => this.tpaNameToIntegrationKeyMap[tpaName]);

this.connectedApps = connectedApps;
});
}
Comment on lines +241 to +248
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling and cleanup subscription.

The subscription to getIntegrations() should handle errors and be cleaned up to prevent memory leaks.

+  private destroy$ = new Subject<void>();

   private storeConnectedApps() {
-    this.integrationService.getIntegrations().subscribe(integrations => {
+    this.integrationService.getIntegrations().pipe(
+      takeUntil(this.destroy$),
+      catchError(error => {
+        console.error('Failed to fetch integrations:', error);
+        return of([]);
+      })
+    ).subscribe(integrations => {
       const tpaNames = integrations.map(integration => integration.tpa_name);
       const connectedApps = tpaNames.map(tpaName => this.tpaNameToIntegrationKeyMap[tpaName]);
 
       this.connectedApps = connectedApps;
     });
   }

Don't forget to implement ngOnDestroy:

ngOnDestroy(): void {
  this.destroy$.next();
  this.destroy$.complete();
}


ngOnInit(): void {
this.setupLoginWatcher();
this.storeConnectedApps();
}
}
5 changes: 5 additions & 0 deletions src/app/shared/components/core/badge/badge.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@
@apply tw-text-badge-dark-text-color;
}

.theme-success {
@apply tw-bg-bg-success-light tw-border tw-border-solid tw-border-border-success-light;
@apply tw-text-text-success;
}

.size-large {
@apply tw-min-w-24-px;
@apply tw-min-h-24-px;
Expand Down
Binary file added src/assets/logos/intacct-logo-new.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified src/assets/logos/intacct-logo.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added src/assets/logos/netsuite-logo-new.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified src/assets/logos/netsuite-logo.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added src/assets/logos/xero-logo-new.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified src/assets/logos/xero-logo.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading