Conversation
Pm 4877 payments reports
PM-4922 Completeness optimised query
What was broken Reports app CSV downloads for SFDC reports, including Payments, saved JSON response bodies into .csv files. Root cause The SFDC reports controller did not use the existing CSV response interceptor, so Accept: text/csv requests still returned the controller data as JSON. What was changed Enabled application/json and text/csv production on the SFDC reports controller, attached the existing CsvResponseInterceptor, and registered the CSV serializer/interceptor providers in the SFDC reports module. Any added/updated tests Added SFDC controller and module coverage for CSV interceptor wiring. Updated the BA fees optional groupBy assertion to accept an omitted optional property while still verifying the value is undefined.
PM-4934: Fix SFDC payments CSV export
PM-4949: ChallengeCreateAt data
There was a problem hiding this comment.
Pull request overview
Production release updates for reports API, adding categorized payment accrual reporting endpoints, optimizing member search filtering/count queries, and enabling CSV serialization for SFDC report downloads.
Changes:
- Introduce a new Payment Reports module with admin-only member payment accrual endpoints (overall + by category) backed by updated SQL.
- Update the reports directory structure/access rules to expose the new payment report group and hide it from non-admin users.
- Optimize member search by applying lightweight filters first via a
filtered_membersCTE and optionally applying a deeperprofileCompletefilter without impacting count-query pagination params.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/reports/topcoder/topcoder-reports.service.ts | Removes member payment accrual logic from Topcoder reports service. |
| src/reports/topcoder/topcoder-reports.controller.ts | Removes the old /admin/member-payment-accrual endpoint. |
| src/reports/sfdc/sfdc-reports.module.ts | Registers CSV serializer + interceptor in the SFDC reports module. |
| src/reports/sfdc/sfdc-reports.module.spec.ts | Updates module test wiring and asserts CSV providers are available. |
| src/reports/sfdc/sfdc-reports.controller.ts | Enables CSV response serialization at controller level + documents produces types. |
| src/reports/sfdc/sfdc-reports.controller.spec.ts | Adds a test asserting the CSV interceptor is enabled; refines an assertion. |
| src/reports/report-directory.data.ts | Adds adminOnly report support and introduces the new payment report group/paths. |
| src/reports/report-directory.data.spec.ts | Updates directory access tests for the new payment group behavior. |
| src/reports/payment/payment-reports.service.ts | New service to query/map member payment accrual data with optional payment-type filtering. |
| src/reports/payment/payment-reports.service.spec.ts | Adds mapping/query parameter unit test for the new payment accrual service. |
| src/reports/payment/payment-reports.module.ts | New module wiring for payment reports (service/guard/CSV deps). |
| src/reports/payment/payment-reports.controller.ts | New admin-only payment accrual endpoints (overall + per category). |
| src/reports/payment/guards/admin-payment-reports.guard.ts | New guard restricting payment report access to human admins only. |
| src/reports/payment/dto/member-payment-accrual.dto.ts | New query DTO for payment accrual date range filtering. |
| src/reports/member/member-search.service.ts | Refactors SQL generation to use filtered_members and optional profile_complete_filtered. |
| src/reports/member/member-search.service.spec.ts | Adds coverage for the new profileComplete query behavior and param slicing. |
| src/reports/member/dto/member-search.dto.ts | Adds profileComplete request option documentation/validation. |
| src/app.module.ts | Registers PaymentReportsModule in the application module imports. |
| sql/reports/payment/member-payment-accrual.sql | Adds derived payment_type categorization + optional filter param. |
| .circleci/config.yml | Adjusts dev build branch filters (develop + PM-4949). |
Comments suppressed due to low confidence (1)
sql/reports/payment/member-payment-accrual.sql:90
- In member-payment-accrual.sql the CASE expression for challenge_created_date and the cp.gross_amount column are currently written on the same line, and the CASE indentation is inconsistent. This is valid SQL but makes the SELECT list harder to read/maintain; consider formatting the CASE block and separating cp.gross_amount onto its own line.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| payment: { | ||
| label: "Payment Reports", | ||
| basePath: "/payment", | ||
| reports: [ |
There was a problem hiding this comment.
Changing the report directory group key from the previous "admin" group to the new "payment" group (and basePath from "/admin" to "/payment") is an API-breaking change for any consumers that rely on the directory JSON shape (e.g., expecting an "admin" section). If backward compatibility is required, consider keeping an "admin" alias group (or returning both keys) for at least one release, or coordinating/annotating the change explicitly for all clients consuming /v6/reports(/directory).
| @ApiTags("Payment Reports") | ||
| @ApiBearerAuth() | ||
| @UseGuards(AdminPaymentReportsGuard) | ||
| @UseInterceptors(CsvResponseInterceptor) | ||
| @Controller() | ||
| export class PaymentReportsController { | ||
| constructor(private readonly reports: PaymentReportsService) {} | ||
|
|
||
| @Get("/payment/member-payment-accrual") | ||
| @ApiOperation({ | ||
| summary: | ||
| "Member payment accruals for the provided date range (defaults to last 3 months)", | ||
| }) | ||
| getMemberPaymentAccrual(@Query() query: MemberPaymentAccrualQueryDto) { | ||
| const { startDate, endDate } = query; | ||
| return this.reports.getMemberPaymentAccrual(startDate, endDate); | ||
| } |
There was a problem hiding this comment.
PaymentReportsController is new and introduces multiple admin-only routes plus a custom guard + CSV interceptor, but there are no controller-level tests validating routing/guard behavior (unlike other report controllers such as SFDC). Adding a payment-reports.controller.spec.ts (at minimum asserting the guard is applied and that requests are delegated to PaymentReportsService with the expected paymentTypes per endpoint) would help prevent regressions.
| # Development builds are executed on "develop" branch only. | ||
| - "build-dev": | ||
| context: org-global | ||
| filters: | ||
| branches: | ||
| only: | ||
| - develop | ||
| - pm-1127_1 | ||
| - PM-4305 | ||
| - PM-4490 | ||
| - PM-4491-fix | ||
| - PM-3497_talent-search | ||
| - PM-4886 | ||
| - PM-4949 |
There was a problem hiding this comment.
The build-dev workflow branch allowlist was changed to only run on develop and PM-4949. This will disable dev builds for all other feature/hotfix branches, which can be an operational surprise if CI is expected on arbitrary branches. If the intent is to allow any PM-* branch, consider using a regex filter or removing the per-branch allowlist rather than updating this list per release branch.
PM-4877 - payment reports by category
PM-4922 - profile completeness query optimization
PM-4934 - SFDC payments report CSV