Skip to content

fix: Deprecate simplify report settings #3772

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

fix: Deprecate simplify report settings #3772

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b6f7f28
remove report_approvals
mvaishnavi Jul 15, 2025
994bfa1
remove ExtendedReport
mvaishnavi Jul 15, 2025
756a390
deprecate_simplify_report_settings
mvaishnavi Jul 15, 2025
e09110e
merge master
mvaishnavi Jul 15, 2025
f742c4f
test and datafixes
mvaishnavi Jul 15, 2025
9491a8e
Merge branch 'master' into deprecate_simplify_report_settings
mvaishnavi Jul 15, 2025
94da9fb
test and datafixes
mvaishnavi Jul 15, 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: 1 addition & 2 deletions src/app/core/mock-data/modal-controller.data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ export const teamReportsModalControllerParams = {
],
} as FilterOptions<string>,
],
simplifyReportsSettings$: undefined,

selectedFilterValues: selectedFilters4,
activeFilterInitialName: 'State',
},
Expand Down Expand Up @@ -388,7 +388,6 @@ export const addToReportModalControllerParams = {
subheader: 'All',
enableSearch: false,
autoSubmissionReportName: '#Aug 1',
isNewReportsFlowEnabled: false,
},
mode: 'ios' as Mode,
cssClass: 'fy-modal',
Expand Down
1 change: 0 additions & 1 deletion src/app/core/mock-data/my-reports.data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ export const filterPopoverParams = {
component: FyFiltersComponent,
componentProps: {
filterOptions: openFiltersOptions,
simplifyReportsSettings$: undefined,
nonReimbursableOrg$: undefined,
selectedFilterValues: [{ name: 'state', value: 'PENDING' }],
activeFilterInitialName: 'State',
Expand Down
7 changes: 0 additions & 7 deletions src/app/core/mock-data/org-settings.data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1240,10 +1240,6 @@ export const orgSettingsCCCEnabled: OrgSettings = deepFreeze({

export const orgSettingsParamsWithSimplifiedReport: OrgSettings = deepFreeze({
...orgSettingsRes,
simplified_report_closure_settings: {
allowed: true,
enabled: true,
},
});

export const orgSettingsParamsWithAdvanceWallet: OrgSettings = deepFreeze({
Expand Down Expand Up @@ -1293,9 +1289,6 @@ export const orgSettingsWoTax: OrgSettings = deepFreeze({
...orgSettingsData,
tax_settings: { ...orgSettingsData.tax_settings, enabled: false },
advances: null,
simplified_report_closure_settings: {
enabled: true,
},
corporate_credit_card_settings: {
allowed: true,
enabled: true,
Expand Down
2 changes: 0 additions & 2 deletions src/app/core/mock-data/report-stats-data.data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@ import { expectedReportStats } from './report-stats.data';

export const reportStatsData1: ReportStatsData = deepFreeze({
reportStats: expectedReportStats,
simplifyReportsSettings: { enabled: true },
homeCurrency: 'INR',
currencySymbol: '₹',
isNonReimbursableOrg: false,
});

export const reportStatsData2: ReportStatsData = deepFreeze({
...reportStatsData1,
simplifyReportsSettings: { enabled: undefined },
});
2 changes: 0 additions & 2 deletions src/app/core/models/org-settings.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,6 @@ export interface OrgSettingsResponse {
company_expenses_beta_settings?: CommonOrgSettings;
trip_request_settings?: CommonOrgSettings;
xe_provider_settings?: XeProviderSettings;
simplified_report_closure_settings?: CommonOrgSettings;
mobile_app_my_expenses_beta_enabled?: boolean;
amex_feed_enrollment_settings?: AmexFeedEnrollmentSettings;
pending_cct_expense_restriction?: CommonOrgSettings;
Expand Down Expand Up @@ -572,7 +571,6 @@ export interface OrgSettings {
visa_enrollment_settings?: CommonOrgSettings;
mastercard_enrollment_settings?: CommonOrgSettings;
company_expenses_beta_settings?: CommonOrgSettings;
simplified_report_closure_settings?: CommonOrgSettings;
mobile_app_my_expenses_beta_enabled?: boolean;
amex_feed_enrollment_settings?: AmexFeedEnrollmentSettings;
pending_cct_expense_restriction?: CommonOrgSettings;
Expand Down
1 change: 0 additions & 1 deletion src/app/core/models/report-stats-data.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { ReportStats } from './report-stats.model';

export interface ReportStatsData {
reportStats: ReportStats;
simplifyReportsSettings: { enabled: boolean };
homeCurrency: string;
currencySymbol: string;
isNonReimbursableOrg: boolean;
Expand Down
5 changes: 0 additions & 5 deletions src/app/core/services/org-settings.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,10 +414,6 @@ export class OrgSettingsService {
allowed: incoming.company_expenses_beta_settings && incoming.company_expenses_beta_settings.allowed,
enabled: incoming.company_expenses_beta_settings && incoming.company_expenses_beta_settings.enabled,
},
simplified_report_closure_settings: {
allowed: incoming?.simplified_report_closure_settings?.allowed,
enabled: incoming?.simplified_report_closure_settings?.enabled,
},
mobile_app_my_expenses_beta_enabled: incoming?.mobile_app_my_expenses_beta_enabled,
amex_feed_enrollment_settings: {
allowed: incoming?.amex_feed_enrollment_settings?.allowed,
Expand Down Expand Up @@ -584,7 +580,6 @@ export class OrgSettingsService {
company_expenses_beta_settings: outgoing.company_expenses_beta_settings,
visa_enrollment_settings: outgoing.visa_enrollment_settings,
mastercard_enrollment_settings: outgoing.mastercard_enrollment_settings,
simplified_report_closure_settings: outgoing.simplified_report_closure_settings,
mobile_app_my_expenses_beta_enabled: outgoing.mobile_app_my_expenses_beta_enabled,
amex_feed_enrollment_settings: outgoing.amex_feed_enrollment_settings,
};
Expand Down
8 changes: 0 additions & 8 deletions src/app/core/test-data/org-settings.service.spec.data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,10 +436,6 @@ export const orgSettingsGetData: OrgSettings = deepFreeze({
allowed: true,
enabled: true,
},
simplified_report_closure_settings: {
allowed: false,
enabled: false,
},
mobile_app_my_expenses_beta_enabled: false,
amex_feed_enrollment_settings: {
allowed: true,
Expand Down Expand Up @@ -878,10 +874,6 @@ export const orgSettingsPostData: OrgSettingsResponse = deepFreeze({
allowed: true,
cost_center_wise_split: true,
},
simplified_report_closure_settings: {
allowed: false,
enabled: false,
},
mobile_app_my_expenses_beta_enabled: false,
amex_feed_enrollment_settings: {
allowed: true,
Expand Down
16 changes: 16 additions & 0 deletions src/app/fyle/add-edit-expense/add-edit-expense-5.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
taxSettingsData2,
orgSettingsParamsWithAdvanceWallet,
orgSettingsWithProjectCategoryRestrictions,
orgSettingsParamsWithSimplifiedReport,
} from 'src/app/core/mock-data/org-settings.data';
import {
employeeSettingsData,
Expand Down Expand Up @@ -1978,5 +1979,20 @@ export function TestCases5(getTestBed) {
done();
});
});

describe('checkNewReportsFlow():', () => {
it('should check for new reports flow, if simplified report closure setting is not enabled', () => {
component.checkNewReportsFlow(orgSettings$);
tick(500);
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
});
Copy link

@coderabbitai coderabbitai bot Jul 15, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance test coverage to verify actual behavior, thalaiva!

Boss, these tests are too basic - they only verify that the service method is called but don't test what the checkNewReportsFlow method actually does. Since this is related to the deprecation of simplified report closure settings, the tests should verify the method's behavior more comprehensively.

Consider adding assertions to verify:

  • The method's return value or side effects
  • How it handles different organization settings
  • Any component state changes that occur
  • Error handling scenarios
       it('should check for new reports flow, if simplified report closure setting is not enabled', fakeAsync(() => {
+        const mockOrgSettings = orgSettingsData;
+        orgSettingsService.get.and.returnValue(of(mockOrgSettings));
+        
         component.checkNewReportsFlow(orgSettings$);
         tick(500);
+        
         expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
+        // Add assertions to verify the method's behavior based on the settings
       }));

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

🤖 Prompt for AI Agents 
In src/app/fyle/add-edit-expense/add-edit-expense-5.spec.ts around lines 1984 to
1988, the test only checks if orgSettingsService.get is called but does not
verify the actual behavior of checkNewReportsFlow. Enhance the test by adding
assertions to validate the method's return value or side effects, check how it
handles different organization settings inputs, confirm any changes to component
state, and include tests for error handling scenarios to ensure comprehensive
coverage.


it('should check for new reports flow, if simplified report closure setting is enabled', () => {
orgSettingsService.get.and.returnValue(of(orgSettingsParamsWithSimplifiedReport));
component.checkNewReportsFlow(orgSettings$);
tick(500);
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
});
});
Copy link

@coderabbitai coderabbitai bot Jul 15, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ayyo! These tests need some serious polishing, boss!

Several critical issues need fixing:

  1. Missing fakeAsync wrapper - You're using tick(500) without fakeAsync. This will cause runtime errors.

  2. Undefined variable - orgSettings$ is not defined in the test scope.

  3. Pipeline failure - Line 1582 references isNewReportsFlowEnabled property that no longer exists on the component.

  4. Incomplete test assertions - Tests only verify service calls but don't check the actual method behavior.

Apply these fixes:

-    describe('checkNewReportsFlow():', () => {
-      it('should check for new reports flow, if simplified report closure setting is not enabled', () => {
-        component.checkNewReportsFlow(orgSettings$);
-        tick(500);
-        expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
-      });
-
-      it('should check for new reports flow, if simplified report closure setting is enabled', () => {
-        orgSettingsService.get.and.returnValue(of(orgSettingsParamsWithSimplifiedReport));
-        component.checkNewReportsFlow(orgSettings$);
-        tick(500);
-        expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
-      });
-    });
+    describe('checkNewReportsFlow():', () => {
+      it('should check for new reports flow, if simplified report closure setting is not enabled', fakeAsync(() => {
+        const orgSettings$ = of(orgSettingsData);
+        orgSettingsService.get.and.returnValue(orgSettings$);
+        component.checkNewReportsFlow(orgSettings$);
+        tick(500);
+        expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
+      }));
+
+      it('should check for new reports flow, if simplified report closure setting is enabled', fakeAsync(() => {
+        const orgSettings$ = of(orgSettingsParamsWithSimplifiedReport);
+        orgSettingsService.get.and.returnValue(orgSettings$);
+        component.checkNewReportsFlow(orgSettings$);
+        tick(500);
+        expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
+      }));
+    });

Also, remove the reference to isNewReportsFlowEnabled property around line 1582 to fix the pipeline failure.

📝 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
describe('checkNewReportsFlow():', () => {
it('should check for new reports flow, if simplified report closure setting is not enabled', () => {
component.checkNewReportsFlow(orgSettings$);
tick(500);
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
});
it('should check for new reports flow, if simplified report closure setting is enabled', () => {
orgSettingsService.get.and.returnValue(of(orgSettingsParamsWithSimplifiedReport));
component.checkNewReportsFlow(orgSettings$);
tick(500);
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
});
});
describe('checkNewReportsFlow():', () => {
it('should check for new reports flow, if simplified report closure setting is not enabled', fakeAsync(() => {
const orgSettings$ = of(orgSettingsData);
orgSettingsService.get.and.returnValue(orgSettings$);
component.checkNewReportsFlow(orgSettings$);
tick(500);
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
}));
it('should check for new reports flow, if simplified report closure setting is enabled', fakeAsync(() => {
const orgSettings$ = of(orgSettingsParamsWithSimplifiedReport);
orgSettingsService.get.and.returnValue(orgSettings$);
component.checkNewReportsFlow(orgSettings$);
tick(500);
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
}));
});
🤖 Prompt for AI Agents 
In src/app/fyle/add-edit-expense/add-edit-expense-5.spec.ts around lines 1983 to
1996, the tests for checkNewReportsFlow are missing the fakeAsync wrapper needed
for tick(500), and the orgSettings$ observable is not defined. Wrap the test
functions in fakeAsync, define orgSettings$ as an observable of the appropriate
org settings data, and enhance assertions to verify the actual behavior of
checkNewReportsFlow beyond just service call counts. Additionally, remove any
references to the obsolete isNewReportsFlowEnabled property around line 1582 to
resolve the pipeline failure.

Copy link

@coderabbitai coderabbitai bot Jul 15, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add fakeAsync wrapper for proper async testing, superstar!

Boss, these test cases are using tick(500) but missing the fakeAsync wrapper, which is required for testing asynchronous behavior in Angular. Without this, the tests won't work properly.

-    describe('checkNewReportsFlow():', () => {
-      it('should check for new reports flow, if simplified report closure setting is not enabled', () => {
+    describe('checkNewReportsFlow():', () => {
+      it('should check for new reports flow, if simplified report closure setting is not enabled', fakeAsync(() => {
         component.checkNewReportsFlow(orgSettings$);
         tick(500);
         expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
-      });
+      }));

-      it('should check for new reports flow, if simplified report closure setting is enabled', () => {
+      it('should check for new reports flow, if simplified report closure setting is enabled', fakeAsync(() => {
         orgSettingsService.get.and.returnValue(of(orgSettingsParamsWithSimplifiedReport));
         component.checkNewReportsFlow(orgSettings$);
         tick(500);
         expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
-      });
+      }));
📝 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
describe('checkNewReportsFlow():', () => {
it('should check for new reports flow, if simplified report closure setting is not enabled', () => {
component.checkNewReportsFlow(orgSettings$);
tick(500);
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
});
it('should check for new reports flow, if simplified report closure setting is enabled', () => {
orgSettingsService.get.and.returnValue(of(orgSettingsParamsWithSimplifiedReport));
component.checkNewReportsFlow(orgSettings$);
tick(500);
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
});
});
describe('checkNewReportsFlow():', () => {
it('should check for new reports flow, if simplified report closure setting is not enabled', fakeAsync(() => {
component.checkNewReportsFlow(orgSettings$);
tick(500);
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
}));
it('should check for new reports flow, if simplified report closure setting is enabled', fakeAsync(() => {
orgSettingsService.get.and.returnValue(of(orgSettingsParamsWithSimplifiedReport));
component.checkNewReportsFlow(orgSettings$);
tick(500);
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
}));
});
🤖 Prompt for AI Agents 
In src/app/fyle/add-edit-expense/add-edit-expense-5.spec.ts around lines 1983 to
1996, the test cases use tick(500) for async timing but lack the required
fakeAsync wrapper. To fix this, wrap each it block's callback function with
fakeAsync to properly handle asynchronous operations in Angular tests, ensuring
tick works as intended.

});
}
1 change: 0 additions & 1 deletion src/app/fyle/add-edit-expense/add-edit-expense.page.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,6 @@
[label]="etxn.tx.report_id ? 'Report' : 'Add to report'"
[autoSubmissionReportName]="autoSubmissionReportName$|async"
[options]="reports"
[isNewReportsFlowEnabled]="isNewReportsFlowEnabled"
formControlName="report"
></app-fy-add-to-report>
</div>
Expand Down
5 changes: 2 additions & 3 deletions src/app/fyle/add-edit-expense/add-edit-expense.page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ export class AddEditExpensePage implements OnInit {

hardwareBackButtonAction: Subscription;

isNewReportsFlowEnabled = false;


onPageExit$: Subject<void>;

Expand Down Expand Up @@ -2865,7 +2865,6 @@ export class AddEditExpensePage implements OnInit {
}

initClassObservables(): void {
this.isNewReportsFlowEnabled = false;
this.onPageExit$ = new Subject();
this.projectDependentFieldsRef?.ngOnInit();
this.costCenterDependentFieldsRef?.ngOnInit();
Expand Down Expand Up @@ -3121,7 +3120,7 @@ export class AddEditExpensePage implements OnInit {
orgSettings$.subscribe((orgSettings) => {
this.isCorporateCreditCardEnabled = this.getCCCSettings(orgSettings);

this.isNewReportsFlowEnabled = orgSettings?.simplified_report_closure_settings?.enabled || false;


this.isProjectEnabled = orgSettings?.projects.enabled || false;
this.isCostCenterEnabled = orgSettings?.cost_centers.enabled || false;
Expand Down
1 change: 0 additions & 1 deletion src/app/fyle/add-edit-mileage/add-edit-mileage.page.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,6 @@
[options]="reports"
formControlName="report"
[autoSubmissionReportName]="autoSubmissionReportName$|async"
[isNewReportsFlowEnabled]="isNewReportsFlowEnabled"
></app-fy-add-to-report>
</div>
</ng-container>
Expand Down
5 changes: 2 additions & 3 deletions src/app/fyle/add-edit-mileage/add-edit-mileage.page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@

mode = 'add';

title = 'edit';

Check warning on line 131 in src/app/fyle/add-edit-mileage/add-edit-mileage.page.ts

View workflow job for this annotation

GitHub Actions / Run linters 

Hard-coded string "edit" should be replaced with a translation key. Refer to https://www.notion.so/fyleuniverse/i18n-translation-file-structure-1ea2ed8bfcb3803da113d3bfc2774ec1#1ea2ed8bfcb3809a9a0ddc05a548eb49 for more details

activeIndex: number;

Expand Down Expand Up @@ -256,7 +256,7 @@

hardwareBackButtonAction: Subscription;

isNewReportsFlowEnabled = false;


onPageExit$: Subject<void>;

Expand Down Expand Up @@ -969,7 +969,6 @@
}

initClassObservables(): void {
this.isNewReportsFlowEnabled = false;
this.onPageExit$ = new Subject();
this.projectDependentFieldsRef?.ngOnInit();
this.costCenterDependentFieldsRef?.ngOnInit();
Expand Down Expand Up @@ -1085,8 +1084,8 @@
}

checkNewReportsFlow(orgSettings$: Observable<OrgSettings>): void {
orgSettings$.subscribe((orgSettings) => {

Check failure on line 1087 in src/app/fyle/add-edit-mileage/add-edit-mileage.page.ts

View workflow job for this annotation

GitHub Actions / Run linters 

Unexpected empty arrow function

Check failure on line 1087 in src/app/fyle/add-edit-mileage/add-edit-mileage.page.ts

View workflow job for this annotation

GitHub Actions / Run linters 

'orgSettings' is defined but never used
this.isNewReportsFlowEnabled = orgSettings?.simplified_report_closure_settings.enabled || false;

Check failure on line 1088 in src/app/fyle/add-edit-mileage/add-edit-mileage.page.ts

View workflow job for this annotation

GitHub Actions / Run linters 

Trailing spaces not allowed
});
}

Expand Down
1 change: 0 additions & 1 deletion src/app/fyle/add-edit-per-diem/add-edit-per-diem.page.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,6 @@
[options]="reports"
formControlName="report"
[autoSubmissionReportName]="autoSubmissionReportName$|async"
[isNewReportsFlowEnabled]="isNewReportsFlowEnabled"
></app-fy-add-to-report>
</div>
</ng-container>
Expand Down
5 changes: 2 additions & 3 deletions src/app/fyle/add-edit-per-diem/add-edit-per-diem.page.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@

hardwareBackButtonAction: Subscription;

isNewReportsFlowEnabled = false;


onPageExit$: Subject<void>;

Expand Down Expand Up @@ -806,7 +806,6 @@

ionViewWillEnter(): void {
this.subCategories$ = this.getSubCategories().pipe(shareReplay(1));
this.isNewReportsFlowEnabled = false;
this.onPageExit$ = new Subject();
this.projectDependentFieldsRef?.ngOnInit();
this.costCenterDependentFieldsRef?.ngOnInit();
Expand Down Expand Up @@ -893,8 +892,8 @@
map((orgSettings) => orgSettings.advanced_per_diems_settings?.enable_employee_restriction)
);

orgSettings$.subscribe((orgSettings) => {

Check failure on line 895 in src/app/fyle/add-edit-per-diem/add-edit-per-diem.page.ts

View workflow job for this annotation

GitHub Actions / Run linters 

Unexpected empty arrow function

Check failure on line 895 in src/app/fyle/add-edit-per-diem/add-edit-per-diem.page.ts

View workflow job for this annotation

GitHub Actions / Run linters 

'orgSettings' is defined but never used
this.isNewReportsFlowEnabled = orgSettings?.simplified_report_closure_settings?.enabled || false;

});

this.setupNetworkWatcher();
Expand Down
35 changes: 10 additions & 25 deletions src/app/fyle/dashboard/stats/stats.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,7 @@
<app-stat-badge
[currency]="reportStatsData.homeCurrency"
[currencySymbol]="reportStatsData.currencySymbol"
[name]="
(reportStatsData.simplifyReportsSettings.enabled ? 'stats.submitted' : 'stats.reported') | transloco
"
[name]="'stats.submitted' | transloco"
[reportState]="ReportStates.APPROVER_PENDING"
[count]="reportStatsData.reportStats?.report?.count"
[value]="reportStatsData.reportStats?.report?.total_amount"
Expand All @@ -87,28 +85,15 @@
class="stats--report-badge-col stats--report-badge-col__right stats--report-badge-col__bottom"
size="6"
>
<ng-container *ngIf="reportStatsData.simplifyReportsSettings.enabled; else simplifyReportsDisabled">
<app-stat-badge
[currency]="reportStatsData.homeCurrency"
[currencySymbol]="reportStatsData.currencySymbol"
[name]="'stats.processing' | transloco"
[reportState]="ReportStates.PAYMENT_PROCESSING"
[count]="reportStatsData.reportStats?.processing?.count"
[value]="reportStatsData.reportStats?.processing?.total_amount"
(badgeClicked)="goToReportsPage($event)"
></app-stat-badge>
</ng-container>
<ng-template #simplifyReportsDisabled>
<app-stat-badge
[currency]="reportStatsData.homeCurrency"
[currencySymbol]="reportStatsData.currencySymbol"
[name]="'stats.paymentPending' | transloco"
[reportState]="ReportStates.PAYMENT_PENDING"
[count]="reportStatsData.reportStats?.paymentPending?.count"
[value]="reportStatsData.reportStats?.paymentPending?.total_amount"
(badgeClicked)="goToReportsPage($event)"
></app-stat-badge>
</ng-template>
<app-stat-badge
[currency]="reportStatsData.homeCurrency"
[currencySymbol]="reportStatsData.currencySymbol"
[name]="'stats.processing' | transloco"
[reportState]="ReportStates.PAYMENT_PROCESSING"
[count]="reportStatsData.reportStats?.processing?.count"
[value]="reportStatsData.reportStats?.processing?.total_amount"
(badgeClicked)="goToReportsPage($event)"
></app-stat-badge>
</ion-col>
</ion-row>
</ng-container>
Expand Down
26 changes: 2 additions & 24 deletions src/app/fyle/dashboard/stats/stats.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,32 +175,10 @@ describe('StatsComponent', () => {
});

it('should initialize reportStatsData$', (done) => {
component.initializeReportStats();

expect(component.reportStatsLoading).toBeTrue();
component.reportStatsData$.subscribe((res) => {
expect(res).toEqual(reportStatsData1);
expect(dashboardService.getReportsStats).toHaveBeenCalledTimes(1);
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
expect(paymentModeService.isNonReimbursableOrg).toHaveBeenCalledOnceWith(
orgSettingsParamsWithSimplifiedReport.payment_mode_settings
);
expect(component.reportStatsLoading).toBeFalse();
done();
});
});

it('should initialize reportStatsData$ with enabled as undefined if simplified_report_closure_settings is undefined', (done) => {
orgSettingsService.get.and.returnValue(of(orgSettingsRes));
component.initializeReportStats();

expect(component.reportStatsLoading).toBeTrue();
component.ngOnInit();
component.reportStatsData$.subscribe((res) => {
expect(res).toEqual(reportStatsData2);
expect(dashboardService.getReportsStats).toHaveBeenCalledTimes(1);
expect(orgSettingsService.get).toHaveBeenCalledTimes(1);
expect(paymentModeService.isNonReimbursableOrg).toHaveBeenCalledOnceWith(orgSettingsRes.payment_mode_settings);
expect(component.reportStatsLoading).toBeFalse();
expect(res).toEqual(reportStatsData1);
done();
});
});
Expand Down
7 changes: 1 addition & 6 deletions src/app/fyle/dashboard/stats/stats.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export class StatsComponent implements OnInit {

isIncompleteExpensesStatsLoading = true;

simplifyReportsSettings$: Observable<{ enabled: boolean }>;


reportStatsLoading = true;

Expand Down Expand Up @@ -91,17 +91,12 @@ export class StatsComponent implements OnInit {

const orgSettings$ = this.orgSettingsService.get().pipe(shareReplay(1));

const simplifyReportsSettings$ = orgSettings$.pipe(
map((orgSettings) => ({ enabled: orgSettings.simplified_report_closure_settings?.enabled }))
);

const isNonReimbursableOrg$ = orgSettings$.pipe(
map((orgSettings) => this.paymentModeService.isNonReimbursableOrg(orgSettings.payment_mode_settings))
);

this.reportStatsData$ = forkJoin({
reportStats: reportStats$,
simplifyReportsSettings: simplifyReportsSettings$,
homeCurrency: this.homeCurrency$,
currencySymbol: this.currencySymbol$,
isNonReimbursableOrg: isNonReimbursableOrg$,
Expand Down
2 changes: 1 addition & 1 deletion src/app/fyle/my-expenses/add-txn-to-report-dialog/add-txn-to-report-dialog.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ describe('AddTxnToReportDialogComponent', () => {
},
{
provide: MAT_BOTTOM_SHEET_DATA,
useValue: { openReports: expectedReportsSinglePage, isNewReportsFlowEnabled: true },
useValue: { openReports: expectedReportsSinglePage },
},
{
provide: TranslocoService,
Expand Down
3 changes: 2 additions & 1 deletion src/app/fyle/my-expenses/add-txn-to-report-dialog/add-txn-to-report-dialog.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ export class AddTxnToReportDialogComponent implements OnInit {

constructor(
private currencyService: CurrencyService,
@Inject(MAT_BOTTOM_SHEET_DATA) public data: { openReports: Report[]; isNewReportsFlowEnabled: boolean },
@Inject(MAT_BOTTOM_SHEET_DATA) public data: { openReports: Report[] },

Comment on lines +19 to +20
Copy link

@coderabbitai coderabbitai bot Jul 15, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Duplicated source of truth for openReports

You accept openReports via both @Input() and MAT_BOTTOM_SHEET_DATA. Two doors for the same guest can cause confusion.

Option 1 – lean on DI only:

-  @Input() openReports;
+  // @Input() openReports;  // removed, DI rules

Option 2 – keep the input but rename one of them to avoid clashes.

Pick one path, walk in style.

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

🤖 Prompt for AI Agents 
In
src/app/fyle/my-expenses/add-txn-to-report-dialog/add-txn-to-report-dialog.component.ts
around lines 19 to 20, you are receiving the openReports data both via @Input()
and MAT_BOTTOM_SHEET_DATA injection, causing duplication and potential
confusion. Choose one method to provide openReports: either rely solely on
dependency injection with MAT_BOTTOM_SHEET_DATA or keep the @Input() but rename
one of the properties to avoid naming conflicts. Remove the redundant source
accordingly to maintain a single source of truth.

private matBottomsheet: MatBottomSheet,
private router: Router
) {}
Expand Down
Loading
Loading