diff --git a/memory-bank/archive/archive-TargetUnit-Effective-Params-20250811.md b/memory-bank/archive/archive-TargetUnit-Effective-Params-20250811.md new file mode 100644 index 00000000..17603e18 --- /dev/null +++ b/memory-bank/archive/archive-TargetUnit-Effective-Params-20250811.md @@ -0,0 +1,76 @@ +# Enhancement Archive: Add Effective Financial Parameters to Target Units + +## Summary +Implemented integration of effective financial parameters (Effective Revenue, Effective Margin, Effective Marginality) into the Weekly Financial Reports system through QuickBooks Online (QBO) integration. + +## Date Completed +2025-08-11 + +## Key Files Modified +- `workers/main/src/activities/weeklyFinancialReports/fetchFinancialAppData.ts` - QBO integration for effective revenue fetching +- `workers/main/src/configs/qbo.ts` - QBO configuration with effective revenue parameters +- `workers/main/src/services/FinApp/types.ts` - added effectiveRevenue field to Project interface +- `workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportFormatter.ts` - formatting new metrics in reports +- `workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts` - core business logic for calculations +- `workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.test.ts` - extended testing +- `workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportSorting.test.ts` - new sorting tests + +## Requirements Addressed +- Add Effective Revenue as Target Unit parameter +- Add Effective Margin as Target Unit parameter +- Add Effective Marginality as Target Unit parameter +- Integrate data from QuickBooks Online for accurate calculations +- Implement group sorting by marginality levels (High → Medium → Low) +- Ensure display of new metrics in financial reports + +## Implementation Details +**QBO Integration:** +- Added `qboRepo.getEffectiveRevenue()` call in `fetchFinancialAppData` +- Configured with `effectiveRevenueMonths` parameter (default 4 months) +- Effective revenue linked to projects through `quick_books_id` + +**Calculations and Sorting:** +- Implemented effectiveRevenue, effectiveMargin, effectiveMarginality calculations in `aggregateGroupData` +- Added advanced sorting: first by marginality levels, then by effective marginality +- `compareMarginalityLevels` method for sorting order determination (High: 3, Medium: 2, Low: 1) + +**Formatting:** +- Updated `formatDetail` to display Effective Revenue, Effective Margin, Effective Marginality +- Added explanations in footer about effective revenue calculation period +- Support for marginality indicators in reports + +## Testing Performed +- Unit tests for new `aggregateGroupData` method with effective metrics calculations +- Comprehensive sorting tests in `WeeklyFinancialReportSorting.test.ts` (100+ lines) +- Verification of correct sorting by marginality levels: High → Medium → Low +- Testing secondary sorting by effective marginality within same level +- Validation of new field formatting in reports + +## Lessons Learned +- **QBO integration**: Repository pattern is effective for external services and scales well +- **Financial calculations**: Require particularly detailed testing due to critical importance of accuracy +- **Level 2 complexity**: Tasks with external service integration can be more complex than expected (+401 lines for Level 2) +- **Code organization**: Proper separation of responsibilities between data, business, and presentation layers is critical +- **Optimization**: The `compareMarginalityLevels` method can be inlined for simplification (~15 lines savings) + +## Related Work +- Related to general Weekly Financial Reports system +- Based on existing QBORepository infrastructure +- Complements marginality system (MarginalityCalculator, MarginalityLevel) +- PR #95: https://github.com/speedandfunction/automatization/pull/95 + +## Notes +**Technical Architecture:** +- Used Repository pattern for QBO integration +- Preserved backward compatibility when adding new fields +- Efficient design: minimal changes in types, focused changes in business logic + +**Potential Improvements:** +- Inline `compareMarginalityLevels` method +- Extract marginality thresholds to configuration constants +- More strict typing for financial calculations + +**Time Estimates:** +- Planned: 1-2 days (Level 2) +- Actual: 2-3 days +- Variance reason: underestimation of QBO integration complexity and testing volume required diff --git a/workers/main/src/activities/weeklyFinancialReports/fetchFinancialAppData.test.ts b/workers/main/src/activities/weeklyFinancialReports/fetchFinancialAppData.test.ts index 25564b21..00d5bff5 100644 --- a/workers/main/src/activities/weeklyFinancialReports/fetchFinancialAppData.test.ts +++ b/workers/main/src/activities/weeklyFinancialReports/fetchFinancialAppData.test.ts @@ -5,9 +5,14 @@ import { AppError } from '../../common/errors'; import * as fileUtils from '../../common/fileUtils'; import * as mongoPoolModule from '../../common/MongoPool'; import type { TargetUnit } from '../../common/types'; -import type { Employee, Project } from '../../services/FinApp'; -import type { IFinAppRepository } from '../../services/FinApp'; +import type { + Employee, + IFinAppRepository, + Project, +} from '../../services/FinApp'; import * as finAppService from '../../services/FinApp'; +import type { CustomerRevenueByRef } from '../../services/QBO'; +import * as qboService from '../../services/QBO'; import { fetchFinancialAppData } from './fetchFinancialAppData'; type MongoPoolMock = { @@ -30,6 +35,9 @@ vi.mock('../../common/MongoPool', () => ({ vi.mock('../../services/FinApp', () => ({ FinAppRepository: vi.fn(), })); +vi.mock('../../services/QBO', () => ({ + QBORepository: vi.fn(), +})); const mockTargetUnits: TargetUnit[] = [ { @@ -48,12 +56,21 @@ const mockEmployees: Employee[] = [ ]; const mockProjects: Project[] = [ { + name: 'Test Project', redmine_id: 2, quick_books_id: 10, history: { rate: { '2024-01-01': 200 } }, }, ]; +const mockEffectiveRevenue: CustomerRevenueByRef = { + '10': { + customerName: 'Test Customer', + totalAmount: 5000, + invoiceCount: 3, + }, +}; + function createRepoInstance( overrides: Partial = {}, ): IFinAppRepository { @@ -83,8 +100,10 @@ describe('getFinAppData', () => { let connect: Mock; let disconnect: Mock; let FinAppRepository: Mock; + let qboRepository: Mock; let dateSpy: ReturnType; let repoInstance: IFinAppRepository; + let qboRepoInstance: { getEffectiveRevenue: Mock }; let mongoPoolInstance: MongoPoolMock; const fileLink = 'input.json'; @@ -100,6 +119,7 @@ describe('getFinAppData', () => { (repoInstance.getProjectsByRedmineIds as Mock).mockResolvedValue( mockProjects, ); + qboRepoInstance.getEffectiveRevenue.mockResolvedValue(mockEffectiveRevenue); } async function expectAppError(promise: Promise, msg: string) { @@ -113,10 +133,16 @@ describe('getFinAppData', () => { readJsonFile = vi.mocked(fileUtils.readJsonFile); writeJsonFile = vi.mocked(fileUtils.writeJsonFile); FinAppRepository = vi.mocked(finAppService.FinAppRepository); + qboRepository = vi.mocked(qboService.QBORepository); repoInstance = createRepoInstance(); FinAppRepository.mockImplementation(() => repoInstance); + qboRepoInstance = { + getEffectiveRevenue: vi.fn().mockResolvedValue(mockEffectiveRevenue), + }; + qboRepository.mockImplementation(() => qboRepoInstance); + connect = vi.fn().mockResolvedValue(undefined); disconnect = vi.fn().mockResolvedValue(undefined); mongoPoolInstance = createMongoPoolInstance(connect, disconnect); @@ -141,8 +167,16 @@ describe('getFinAppData', () => { expect(readJsonFile).toHaveBeenCalledWith(fileLink); expect(writeJsonFile).toHaveBeenCalledWith(expectedFilename, { employees: mockEmployees, - projects: mockProjects, + projects: [ + { + ...mockProjects[0], + effectiveRevenue: 5000, + }, + ], + effectiveRevenue: mockEffectiveRevenue, }); + expect(qboRepository).toHaveBeenCalledTimes(1); + expect(qboRepoInstance.getEffectiveRevenue).toHaveBeenCalledTimes(1); }); it('always disconnects the mongo pool', async () => { diff --git a/workers/main/src/activities/weeklyFinancialReports/fetchFinancialAppData.ts b/workers/main/src/activities/weeklyFinancialReports/fetchFinancialAppData.ts index 025de73b..160f8e53 100644 --- a/workers/main/src/activities/weeklyFinancialReports/fetchFinancialAppData.ts +++ b/workers/main/src/activities/weeklyFinancialReports/fetchFinancialAppData.ts @@ -3,6 +3,7 @@ import { readJsonFile, writeJsonFile } from '../../common/fileUtils'; import { MongoPool } from '../../common/MongoPool'; import { TargetUnit } from '../../common/types'; import { FinAppRepository } from '../../services/FinApp'; +import { QBORepository } from '../../services/QBO'; interface GetTargetUnitsResult { fileLink: string; @@ -21,17 +22,30 @@ export const fetchFinancialAppData = async ( try { await mongoPool.connect(); const repo = new FinAppRepository(); + const qboRepo = new QBORepository(); const targetUnits = await readJsonFile(fileLink); const employeeIds = getUniqueIds(targetUnits, 'user_id'); const projectIds = getUniqueIds(targetUnits, 'project_id'); - const [employees, projects] = await Promise.all([ - repo.getEmployeesByRedmineIds(employeeIds), - repo.getProjectsByRedmineIds(projectIds), - ]); - - await writeJsonFile(filename, { employees, projects }); + const [employees, projects, effectiveRevenueByCustomerRef] = + await Promise.all([ + repo.getEmployeesByRedmineIds(employeeIds), + repo.getProjectsByRedmineIds(projectIds), + qboRepo.getEffectiveRevenue(), + ]); + + await writeJsonFile(filename, { + employees, + projects: projects.map((project) => ({ + ...project, + effectiveRevenue: project.quick_books_id + ? effectiveRevenueByCustomerRef[String(project.quick_books_id)] + ?.totalAmount || 0 + : 0, + })), + effectiveRevenue: effectiveRevenueByCustomerRef, + }); return { fileLink: filename }; } catch (err) { diff --git a/workers/main/src/configs/qbo.ts b/workers/main/src/configs/qbo.ts index 3dd24144..0cf3be73 100644 --- a/workers/main/src/configs/qbo.ts +++ b/workers/main/src/configs/qbo.ts @@ -11,9 +11,11 @@ export const qboConfig = { tokenHost: 'https://oauth.platform.intuit.com', tokenPath: '/oauth2/v1/tokens/bearer', tokenExpirationWindowSeconds: 300, - effectiveRevenueMonths: parseInt( - process.env.QBO_EFFECTIVE_REVENUE_MONTHS || '3', - ), + effectiveRevenueMonths: (() => { + const raw = Number(process.env.QBO_EFFECTIVE_REVENUE_MONTHS); + + return Number.isFinite(raw) ? Math.trunc(raw) : 4; + })(), }; export const qboSchema = z.object({ diff --git a/workers/main/src/services/FinApp/FinAppRepository.test.ts b/workers/main/src/services/FinApp/FinAppRepository.test.ts index 0dc0f58c..071be91b 100644 --- a/workers/main/src/services/FinApp/FinAppRepository.test.ts +++ b/workers/main/src/services/FinApp/FinAppRepository.test.ts @@ -120,7 +120,7 @@ describe('FinAppRepository', () => { expect(result).toEqual(mockProjects); expect(vi.mocked(ProjectModel).find).toHaveBeenCalledWith( { redmine_id: { $in: [550] } }, - { 'redmine_id': 1, 'quick_books_id': 1, 'history.rate': 1 }, + { 'name': 1, 'redmine_id': 1, 'quick_books_id': 1, 'history.rate': 1 }, ); }); diff --git a/workers/main/src/services/FinApp/FinAppRepository.ts b/workers/main/src/services/FinApp/FinAppRepository.ts index 538fd793..fc65dc5e 100644 --- a/workers/main/src/services/FinApp/FinAppRepository.ts +++ b/workers/main/src/services/FinApp/FinAppRepository.ts @@ -21,7 +21,7 @@ export class FinAppRepository implements IFinAppRepository { try { return await ProjectModel.find( { redmine_id: { $in: redmineIds } }, - { 'redmine_id': 1, 'quick_books_id': 1, 'history.rate': 1 }, + { 'name': 1, 'redmine_id': 1, 'quick_books_id': 1, 'history.rate': 1 }, ).lean(); } catch (error) { throw new FinAppRepositoryError( diff --git a/workers/main/src/services/FinApp/FinAppSchemas.test.ts b/workers/main/src/services/FinApp/FinAppSchemas.test.ts index 699c59b8..6bea5c4d 100644 --- a/workers/main/src/services/FinApp/FinAppSchemas.test.ts +++ b/workers/main/src/services/FinApp/FinAppSchemas.test.ts @@ -59,6 +59,7 @@ describe('FinApp Schemas', () => { it('should accept valid project', async () => { const doc = new ProjectModel({ + name: 'Test Project', redmine_id: 456, quick_books_id: 789, history: { rate: { '2024-01-01': 200 } }, diff --git a/workers/main/src/services/FinApp/FinAppSchemas.ts b/workers/main/src/services/FinApp/FinAppSchemas.ts index 36e24a4a..f03d2dc1 100644 --- a/workers/main/src/services/FinApp/FinAppSchemas.ts +++ b/workers/main/src/services/FinApp/FinAppSchemas.ts @@ -24,6 +24,7 @@ export const EmployeeModel = mongoose.model( // Project schema: represents a project with Redmine and QuickBooks IDs, and a history of rates export const projectSchema = new mongoose.Schema({ + name: { type: String, required: true }, redmine_id: { type: Number, required: true, index: true }, quick_books_id: Number, history: historySchema, diff --git a/workers/main/src/services/FinApp/types.ts b/workers/main/src/services/FinApp/types.ts index b37ee101..b2cd0070 100644 --- a/workers/main/src/services/FinApp/types.ts +++ b/workers/main/src/services/FinApp/types.ts @@ -12,6 +12,10 @@ export interface Employee { } export interface Project { + /** + * Project name + */ + name: string; /** * Redmine project ID (links to the corresponding project in Redmine) */ @@ -21,6 +25,7 @@ export interface Project { */ quick_books_id?: number; history?: History; + effectiveRevenue?: number; [key: string]: unknown; } diff --git a/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportFormatter.ts b/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportFormatter.ts index debae357..57399f25 100644 --- a/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportFormatter.ts +++ b/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportFormatter.ts @@ -1,10 +1,12 @@ import { formatCurrency } from '../../common/formatUtils'; +import { formatDateToISOString } from '../../common/utils'; +import { qboConfig } from '../../configs/qbo'; import { HIGH_MARGINALITY_THRESHOLD, MEDIUM_MARGINALITY_THRESHOLD, } from '../../configs/weeklyFinancialReport'; -export interface formatSummaryInput { +export interface FormatSummaryInput { reportTitle: string; highGroups: string[]; mediumGroups: string[]; @@ -13,13 +15,16 @@ export interface formatSummaryInput { export interface FormatDetailInput { groupName: string; - groupTotalHours: number; currentQuarter: string; + groupTotalHours: number; groupTotalRevenue: number; groupTotalCogs: number; marginAmount: number; marginalityPercent: number; indicator: string; + effectiveRevenue: number; + effectiveMargin: number; + effectiveMarginality: number; } const spacer = ' '.repeat(4); @@ -27,58 +32,108 @@ const spacer = ' '.repeat(4); export class WeeklyFinancialReportFormatter { static formatDetail = ({ groupName, - groupTotalHours, currentQuarter, + groupTotalHours, groupTotalRevenue, groupTotalCogs, marginAmount, marginalityPercent, indicator, + effectiveRevenue, + effectiveMargin, + effectiveMarginality, }: FormatDetailInput) => - `${indicator} *${groupName}* (${groupTotalHours}h)\n` + - `${spacer}*Period*: ${currentQuarter}\n` + - `${spacer}*Revenue*: ${formatCurrency(groupTotalRevenue)}\n` + - `${spacer}*COGS*: ${formatCurrency(groupTotalCogs)}\n` + - `${spacer}*Margin*: ${formatCurrency(marginAmount)}\n` + - `${spacer}*Marginality*: ${marginalityPercent.toFixed(0)}%\n\n`; + `*${groupName}*\n` + + `${spacer}period: ${currentQuarter}\n` + + `${spacer}total hours: ${groupTotalHours.toFixed(1)}\n` + + `${spacer}revenue: ${formatCurrency(groupTotalRevenue)}\n` + + `${spacer}COGS: ${formatCurrency(groupTotalCogs)}\n` + + `${spacer}margin: ${formatCurrency(marginAmount)}\n` + + `${spacer}marginality: ${marginalityPercent.toFixed(0)}%\n` + + `${spacer}effective revenue: ${formatCurrency(effectiveRevenue)}\n` + + `${spacer}effective margin: ${formatCurrency(effectiveMargin)}\n` + + `${spacer}effective marginality: ${indicator} ${effectiveMarginality.toFixed(0)}%\n\n\n`; static formatSummary = ({ reportTitle, highGroups, mediumGroups, lowGroups, - }: formatSummaryInput) => { + }: FormatSummaryInput) => { let summary = `${reportTitle}\n`; if (highGroups.length) { - summary += '________________________________\n'; + summary += '\n_______________________\n\n\n'; summary += `:arrowup: *Marginality is ${HIGH_MARGINALITY_THRESHOLD}% or higher*:\n`; - summary += `${spacer}${highGroups.join(`\n${spacer}`)}\n`; + summary += `${spacer}${spacer}${highGroups.join(`\n${spacer}${spacer}`)}\n`; } if (mediumGroups.length) { - summary += '__________________________________\n'; + summary += '\n_______________________\n\n\n'; summary += ` :large_yellow_circle: *Marginality is between ${MEDIUM_MARGINALITY_THRESHOLD}-${HIGH_MARGINALITY_THRESHOLD}%*:\n`; - summary += `${spacer}${mediumGroups.join(`\n${spacer}`)}\n`; + summary += `${spacer}${spacer}${mediumGroups.join(`\n${spacer}${spacer}`)}\n`; } if (lowGroups.length) { - summary += '__________________________________\n'; + summary += '\n_______________________\n\n\n'; summary += `:arrowdown: *Marginality is under ${MEDIUM_MARGINALITY_THRESHOLD}%*:\n`; - summary += `${spacer}${lowGroups.join(`\n${spacer}`)}\n`; + summary += `${spacer}${spacer}${lowGroups.join(`\n${spacer}${spacer}`)}\n`; } - summary += ' -------------------------------------------\n'; + summary += '\n_______________________\n\n\n'; summary += 'The specific figures will be available in the thread'; return summary; }; - static formatFooter = (totalHours: number) => - `\n*Total hours*: ${totalHours}h\n\n` + - '*Notes:*\n' + - '1. *Contract Type* is not implemented\n' + - '2. *Effective Revenue* is not implemented\n' + - '3. *Dept Tech* hours are not implemented\n\n' + - `*Legend*: Marginality :arrowup: ≥${HIGH_MARGINALITY_THRESHOLD}% :large_yellow_circle: ${MEDIUM_MARGINALITY_THRESHOLD}-${HIGH_MARGINALITY_THRESHOLD - 1}% :arrowdown: <${MEDIUM_MARGINALITY_THRESHOLD}%`; + private static calculateDateWindow() { + const endDate = new Date(); + + const currentYear = endDate.getFullYear(); + const currentMonth = endDate.getMonth(); + const monthsToSubtract = qboConfig.effectiveRevenueMonths; + + let targetYear = currentYear; + let targetMonth = currentMonth - monthsToSubtract; + + while (targetMonth < 0) { + targetMonth += 12; + targetYear -= 1; + } + + const daysInTargetMonth = new Date( + targetYear, + targetMonth + 1, + 0, + ).getDate(); + + const clampedDay = Math.min(endDate.getDate(), daysInTargetMonth); + + const startDate = new Date( + targetYear, + targetMonth, + clampedDay, + endDate.getHours(), + endDate.getMinutes(), + endDate.getSeconds(), + endDate.getMilliseconds(), + ); + + return { + startDate: formatDateToISOString(startDate), + endDate: formatDateToISOString(endDate), + }; + } + + static formatFooter = () => { + const { startDate, endDate } = this.calculateDateWindow(); + + return ( + '\n*Notes:*\n' + + '1. *Contract Type* is not implemented\n' + + `2. *Effective Revenue* calculated for the last ${qboConfig.effectiveRevenueMonths} months (${startDate} - ${endDate})\n` + + '3. *Dept Tech* hours are not implemented\n\n' + + `*Legend*: Marginality :arrowup: ≥${HIGH_MARGINALITY_THRESHOLD}% :large_yellow_circle: ${MEDIUM_MARGINALITY_THRESHOLD}-${HIGH_MARGINALITY_THRESHOLD - 1}% :arrowdown: <${MEDIUM_MARGINALITY_THRESHOLD}%` + ); + }; } diff --git a/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.test.ts b/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.test.ts index 96b090fb..23af2fa6 100644 --- a/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.test.ts +++ b/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.test.ts @@ -2,10 +2,8 @@ import { describe, expect, it } from 'vitest'; import { WeeklyFinancialReportRepository } from './WeeklyFinancialReportRepository'; -describe('WeeklyFinancialReportRepository', () => { - const repo = new WeeklyFinancialReportRepository(); - - const targetUnits = [ +const createBasicTestData = () => ({ + targetUnits: [ { group_id: 1, group_name: 'Group A', @@ -56,26 +54,47 @@ describe('WeeklyFinancialReportRepository', () => { spent_on: '2024-06-01', total_hours: 10, }, - ]; - const employees = [ + ], + employees: [ { redmine_id: 100, history: { rate: { '2024-01-01': 100 } } }, { redmine_id: 101, history: { rate: { '2024-01-01': 200 } } }, { redmine_id: 102, history: { rate: { '2024-01-01': 300 } } }, { redmine_id: 103, history: { rate: { '2024-01-01': 900 } } }, { redmine_id: 104, history: { rate: { '2024-01-01': 700 } } }, - ]; - const projects = [ - { redmine_id: 10, history: { rate: { '2024-01-01': 500 } } }, - { redmine_id: 20, history: { rate: { '2024-01-01': 1000 } } }, - { redmine_id: 30, history: { rate: { '2024-01-01': 1500 } } }, - { redmine_id: 40, history: { rate: { '2024-01-01': 1300 } } }, - ]; + ], + projects: [ + { + redmine_id: 10, + name: 'Project X', + history: { rate: { '2024-01-01': 500 } }, + }, + { + redmine_id: 20, + name: 'Project Y', + history: { rate: { '2024-01-01': 1000 } }, + }, + { + redmine_id: 30, + name: 'Project Z', + history: { rate: { '2024-01-01': 1500 } }, + }, + { + redmine_id: 40, + name: 'Project W', + history: { rate: { '2024-01-01': 1300 } }, + }, + ], +}); + +describe('WeeklyFinancialReportRepository', () => { + const repo = new WeeklyFinancialReportRepository(); it('generates a report with summary and details', async () => { + const testData = createBasicTestData(); const { summary, details } = await repo.generateReport({ - targetUnits, - employees, - projects, + targetUnits: testData.targetUnits, + employees: testData.employees, + projects: testData.projects, }); expect(typeof summary).toBe('string'); @@ -83,7 +102,6 @@ describe('WeeklyFinancialReportRepository', () => { expect(summary.length).toBeGreaterThan(0); expect(details.length).toBeGreaterThan(0); - // Check summary content expect(summary).toContain('Weekly Financial Summary for Target Units'); expect(summary).toContain('Marginality is 55% or higher'); expect(summary).toContain('Marginality is between 45-55%'); @@ -91,19 +109,17 @@ describe('WeeklyFinancialReportRepository', () => { expect(summary).toContain( 'The specific figures will be available in the thread', ); - // Group names should appear in summary expect(summary).toContain('Group A'); expect(summary).toContain('Group B'); expect(summary).toContain('Group C'); expect(summary).toContain('Group D'); - // Check details content - expect(details).toContain('Total hours'); + expect(details).toContain('total hours'); expect(details).toContain('Group A'); expect(details).toContain('Group B'); expect(details).toContain('Group C'); expect(details).toContain('Group D'); - expect(details).toMatch(/\*Period\*: Q\d/); + expect(details).toMatch(/period: Q\d/); expect(details).toContain('Revenue'); expect(details).toContain('COGS'); expect(details).toContain('Margin'); @@ -128,9 +144,86 @@ describe('WeeklyFinancialReportRepository', () => { expect(typeof summary).toBe('string'); expect(typeof details).toBe('string'); - expect(details).toContain('*Total hours*: 0h'); + // No total hours output when there are no groups expect(details).toContain('Notes:'); expect(details).toContain('Legend'); expect(summary).toContain('Weekly Financial Summary for Target Units'); }); + + it('sorts groups alphabetically within the same marginality level', async () => { + // Create test data with groups that have the same marginality level + // but different names to test alphabetical sorting + const testData = { + targetUnits: [ + { + group_id: 1, + group_name: 'Zebra Group', + project_id: 10, + project_name: 'Project X', + user_id: 100, + username: 'Alice', + spent_on: '2024-06-01', + total_hours: 10, + }, + { + group_id: 2, + group_name: 'Alpha Group', + project_id: 20, + project_name: 'Project Y', + user_id: 101, + username: 'Bob', + spent_on: '2024-06-01', + total_hours: 10, + }, + { + group_id: 3, + group_name: 'Beta Group', + project_id: 30, + project_name: 'Project Z', + user_id: 102, + username: 'Charlie', + spent_on: '2024-06-01', + total_hours: 10, + }, + ], + employees: [ + { redmine_id: 100, history: { rate: { '2024-01-01': 50 } } }, + { redmine_id: 101, history: { rate: { '2024-01-01': 50 } } }, + { redmine_id: 102, history: { rate: { '2024-01-01': 50 } } }, + ], + projects: [ + // All projects have same marginality level (High) with rate 200 and cogs 50*10=500 + // (2000-500)/2000 = 75% marginality + { + redmine_id: 10, + name: 'Project X', + history: { rate: { '2024-01-01': 200 } }, + }, + { + redmine_id: 20, + name: 'Project Y', + history: { rate: { '2024-01-01': 200 } }, + }, + { + redmine_id: 30, + name: 'Project Z', + history: { rate: { '2024-01-01': 200 } }, + }, + ], + }; + const { details } = await repo.generateReport({ + targetUnits: testData.targetUnits, + employees: testData.employees, + projects: testData.projects, + }); + + // Since all groups have the same marginality level, they should be sorted alphabetically + const alphaGroupIndex = details.indexOf('Alpha Group'); + const betaGroupIndex = details.indexOf('Beta Group'); + const zebraGroupIndex = details.indexOf('Zebra Group'); + + // Alpha should come first, then Beta, then Zebra (alphabetical order) + expect(alphaGroupIndex).toBeLessThan(betaGroupIndex); + expect(betaGroupIndex).toBeLessThan(zebraGroupIndex); + }); }); diff --git a/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts b/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts index cebfe680..f9dbd5de 100644 --- a/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts +++ b/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts @@ -10,21 +10,19 @@ import { import { MarginalityCalculator, MarginalityLevel, + MarginalityResult, } from './MarginalityCalculator'; import { WeeklyFinancialReportFormatter } from './WeeklyFinancialReportFormatter'; -interface ProcessTargetUnitInput { - targetUnit: TargetUnit; - targetUnits: TargetUnit[]; - employees: Employee[]; - projects: Project[]; - processedGroupIds: Set; - currentQuarter: string; - highMarginalityGroups: string[]; - mediumMarginalityGroups: string[]; - lowMarginalityGroups: string[]; - updateReportDetails: (detail: string) => void; - updateTotalReportedHours: (hours: number) => void; +interface GroupData { + groupName: string; + groupTotalHours: number; + groupTotalRevenue: number; + groupTotalCogs: number; + effectiveRevenue: number; + effectiveMargin: number; + effectiveMarginality: number; + marginality: MarginalityResult; } export class WeeklyFinancialReportRepository @@ -37,32 +35,26 @@ export class WeeklyFinancialReportRepository }: GenerateReportInput) { const currentDate = new Date(); const reportTitle = this.composeWeeklyReportTitle(currentDate); - const processedGroupIds = new Set(); - let reportDetails = ''; - let totalReportedHours = 0; const currentQuarter = `Q${Math.floor(currentDate.getMonth() / 3) + 1}`; - const highGroups: string[] = []; - const mediumGroups: string[] = []; - const lowGroups: string[] = []; - for (const targetUnit of targetUnits) { - this.processTargetUnit({ - targetUnit, - targetUnits, - employees, - projects, - processedGroupIds, - currentQuarter, - highMarginalityGroups: highGroups, - mediumMarginalityGroups: mediumGroups, - lowMarginalityGroups: lowGroups, - updateReportDetails: (detail) => (reportDetails += detail), - updateTotalReportedHours: (hours) => (totalReportedHours += hours), - }); - } + const { groupData } = this.collectGroupData( + targetUnits, + employees, + projects, + ); + + this.sortGroupData(groupData); + + const { reportDetails: initialDetails } = this.formatGroupDetails( + groupData, + currentQuarter, + ); + + const reportDetails = + initialDetails + WeeklyFinancialReportFormatter.formatFooter(); - reportDetails += - WeeklyFinancialReportFormatter.formatFooter(totalReportedHours); + const { highGroups, mediumGroups, lowGroups } = + this.createSortedGroups(groupData); const reportSummary = WeeklyFinancialReportFormatter.formatSummary({ reportTitle, @@ -77,55 +69,125 @@ export class WeeklyFinancialReportRepository }; } - private processTargetUnit({ - targetUnit, - targetUnits, - employees, - projects, - processedGroupIds, - currentQuarter, - highMarginalityGroups, - mediumMarginalityGroups, - lowMarginalityGroups, - updateReportDetails, - updateTotalReportedHours, - }: ProcessTargetUnitInput) { - if (!processedGroupIds.has(targetUnit.group_id)) { - processedGroupIds.add(targetUnit.group_id); - - const { groupUnits, groupTotalHours } = GroupAggregator.aggregateGroup( - targetUnits, - targetUnit.group_id, - ); - const { groupTotalCogs, groupTotalRevenue } = this.aggregateGroupData({ - groupUnits, - employees, - projects, + private collectGroupData( + targetUnits: TargetUnit[], + employees: Employee[], + projects: Project[], + ) { + const processedGroupIds = new Set(); + const groupData: GroupData[] = []; + + for (const targetUnit of targetUnits) { + if (!processedGroupIds.has(targetUnit.group_id)) { + processedGroupIds.add(targetUnit.group_id); + const groupDataItem = this.processSingleGroup( + targetUnit, + targetUnits, + employees, + projects, + ); + + groupData.push(groupDataItem); + } + } + + return { groupData }; + } + + private sortGroupData(groupData: GroupData[]) { + const levelOrder = { + [MarginalityLevel.High]: 3, + [MarginalityLevel.Medium]: 2, + [MarginalityLevel.Low]: 1, + }; + + // Sort by marginality level (High -> Medium -> Low), + // then within each level by groupName alphabetically + groupData.sort((a, b) => { + const levelComparison = + levelOrder[b.marginality.level] - levelOrder[a.marginality.level]; + + if (levelComparison !== 0) { + return levelComparison; + } + + // Sort by groupName alphabetically within each level + return a.groupName.localeCompare(b.groupName); + }); + } + + private processSingleGroup( + targetUnit: TargetUnit, + targetUnits: TargetUnit[], + employees: Employee[], + projects: Project[], + ): GroupData { + const { groupUnits, groupTotalHours } = GroupAggregator.aggregateGroup( + targetUnits, + targetUnit.group_id, + ); + const { + groupTotalCogs, + groupTotalRevenue, + effectiveRevenue, + effectiveMargin, + effectiveMarginality, + } = this.aggregateGroupData({ groupUnits, employees, projects }); + const marginality = MarginalityCalculator.calculate( + groupTotalRevenue, + groupTotalCogs, + ); + + return { + groupName: targetUnit.group_name, + groupTotalHours, + groupTotalRevenue, + groupTotalCogs, + effectiveRevenue, + effectiveMargin, + effectiveMarginality, + marginality, + }; + } + + private createSortedGroups(groupData: GroupData[]) { + const highGroups: string[] = []; + const mediumGroups: string[] = []; + const lowGroups: string[] = []; + + // Distribute groups by marginality level + for (const group of groupData) { + this.pushGroupByMarginality(group.marginality.level, group.groupName, { + highMarginalityGroups: highGroups, + mediumMarginalityGroups: mediumGroups, + lowMarginalityGroups: lowGroups, }); - const marginality = MarginalityCalculator.calculate( - groupTotalRevenue, - groupTotalCogs, - ); - - this.pushGroupByMarginality(marginality.level, targetUnit.group_name, { - highMarginalityGroups, - mediumMarginalityGroups, - lowMarginalityGroups, + } + // Preserve the order established in sortGroupData + + return { highGroups, mediumGroups, lowGroups }; + } + + private formatGroupDetails(groupData: GroupData[], currentQuarter: string) { + let reportDetails = ''; + + for (const group of groupData) { + reportDetails += WeeklyFinancialReportFormatter.formatDetail({ + groupName: group.groupName, + currentQuarter, + groupTotalHours: group.groupTotalHours, + groupTotalRevenue: group.groupTotalRevenue, + groupTotalCogs: group.groupTotalCogs, + marginAmount: group.marginality.marginAmount, + marginalityPercent: group.marginality.marginalityPercent, + indicator: group.marginality.indicator, + effectiveRevenue: group.effectiveRevenue, + effectiveMargin: group.effectiveMargin, + effectiveMarginality: group.effectiveMarginality, }); - updateReportDetails( - WeeklyFinancialReportFormatter.formatDetail({ - groupName: targetUnit.group_name, - groupTotalHours, - currentQuarter, - groupTotalRevenue, - groupTotalCogs, - marginAmount: marginality.marginAmount, - marginalityPercent: marginality.marginalityPercent, - indicator: marginality.indicator, - }), - ); - updateTotalReportedHours(groupTotalHours); } + + return { reportDetails }; } private pushGroupByMarginality( @@ -182,6 +244,8 @@ export class WeeklyFinancialReportRepository }: AggregateGroupDataInput) { let groupTotalCogs = 0; let groupTotalRevenue = 0; + let effectiveRevenue = 0; + const processedProjects = new Set(); // Отслеживаем обработанные проекты for (const unit of groupUnits) { const employee = employees.find((e) => e.redmine_id === unit.user_id); @@ -192,8 +256,23 @@ export class WeeklyFinancialReportRepository groupTotalCogs += employeeRate * unit.total_hours; groupTotalRevenue += projectRate * unit.total_hours; + + if (project && !processedProjects.has(project.redmine_id)) { + effectiveRevenue += project.effectiveRevenue || 0; + processedProjects.add(project.redmine_id); + } } - return { groupTotalCogs, groupTotalRevenue }; + const effectiveMargin = effectiveRevenue - groupTotalCogs; + const effectiveMarginality = + effectiveRevenue > 0 ? (effectiveMargin / effectiveRevenue) * 100 : 0; + + return { + groupTotalCogs, + groupTotalRevenue, + effectiveRevenue, + effectiveMargin, + effectiveMarginality, + }; } } diff --git a/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportSorting.test.ts b/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportSorting.test.ts new file mode 100644 index 00000000..97826a40 --- /dev/null +++ b/workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportSorting.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, it } from 'vitest'; + +import { WeeklyFinancialReportRepository } from './WeeklyFinancialReportRepository'; + +const createLevelTestData = () => ({ + targetUnits: [ + { + group_id: 1, + group_name: 'Low Group A', + project_id: 10, + project_name: 'Project X', + user_id: 100, + username: 'Alice', + spent_on: '2024-06-01', + total_hours: 10, + }, + { + group_id: 2, + group_name: 'High Group B', + project_id: 20, + project_name: 'Project Y', + user_id: 101, + username: 'Bob', + spent_on: '2024-06-01', + total_hours: 10, + }, + { + group_id: 3, + group_name: 'Medium Group C', + project_id: 30, + project_name: 'Project Z', + user_id: 102, + username: 'Charlie', + spent_on: '2024-06-01', + total_hours: 10, + }, + { + group_id: 4, + group_name: 'High Group D', + project_id: 40, + project_name: 'Project W', + user_id: 103, + username: 'David', + spent_on: '2024-06-01', + total_hours: 10, + }, + ], + employees: [ + { redmine_id: 100, history: { rate: { '2024-01-01': 50 } } }, + { redmine_id: 101, history: { rate: { '2024-01-01': 50 } } }, + { redmine_id: 102, history: { rate: { '2024-01-01': 50 } } }, + { redmine_id: 103, history: { rate: { '2024-01-01': 50 } } }, + ], + projects: [ + { redmine_id: 10, history: { rate: { '2024-01-01': 100 } } }, // 50% marginality (Low) + { redmine_id: 20, history: { rate: { '2024-01-01': 200 } } }, // 75% marginality (High) + { redmine_id: 30, history: { rate: { '2024-01-01': 150 } } }, // 67% marginality (Medium) + { redmine_id: 40, history: { rate: { '2024-01-01': 180 } } }, // 72% marginality (High) + ], +}); + +describe('WeeklyFinancialReportRepository Sorting', () => { + const repo = new WeeklyFinancialReportRepository(); + + it('sorts groups by marginality level (High -> Medium -> Low) then by groupName alphabetically', async () => { + const testData = createLevelTestData(); + const { details, summary } = await repo.generateReport({ + targetUnits: testData.targetUnits, + employees: testData.employees, + projects: testData.projects, + }); + + const highGroupBIndex = details.indexOf('High Group B'); + const highGroupDIndex = details.indexOf('High Group D'); + const mediumGroupCIndex = details.indexOf('Medium Group C'); + const lowGroupAIndex = details.indexOf('Low Group A'); + + // High groups should be first + expect(highGroupBIndex).toBeLessThan(mediumGroupCIndex); + expect(highGroupDIndex).toBeLessThan(mediumGroupCIndex); + + // Medium groups should be after High + expect(mediumGroupCIndex).toBeLessThan(lowGroupAIndex); + + // Low groups should be last + expect(lowGroupAIndex).toBeGreaterThan(highGroupBIndex); + expect(lowGroupAIndex).toBeGreaterThan(highGroupDIndex); + expect(lowGroupAIndex).toBeGreaterThan(mediumGroupCIndex); + + // Within same marginality level, groups should be sorted alphabetically + // "High Group B" should come before "High Group D" alphabetically + expect(highGroupBIndex).toBeLessThan(highGroupDIndex); + + const highGroupBIndexSummary = summary.indexOf('High Group B'); + const mediumGroupCIndexSummary = summary.indexOf('Medium Group C'); + const lowGroupAIndexSummary = summary.indexOf('Low Group A'); + + expect(highGroupBIndexSummary).toBeLessThan(mediumGroupCIndexSummary); + expect(mediumGroupCIndexSummary).toBeLessThan(lowGroupAIndexSummary); + }); +});