Skip to content

feat: notification fan-out + portal bell, rent-payments completion, Stripe period-end fix - #225

Open
MouhannadKhodor wants to merge 6 commits into
hadisaiibi-mouhannadfrom
feat/notification-fanout
Open

feat: notification fan-out + portal bell, rent-payments completion, Stripe period-end fix#225
MouhannadKhodor wants to merge 6 commits into
hadisaiibi-mouhannadfrom
feat/notification-fanout

Conversation

@MouhannadKhodor

@MouhannadKhodor MouhannadKhodor commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Lands the work that was already deployed to prorentallb.cloud but had never been committed, plus one commit that was on the branch but incomplete.

Commits

1. feat(rent-payments) — org-wide tenant rent-payment register
Previously committed as 0faeee9 but incomplete: the page imported useListRentPaymentsQuery / useGetRentPaymentSummaryQuery, which existed only in an uncommitted file, so that commit did not type-check on its own. Its RTK layer is folded back in here, matching what the commit message already claimed.

There was nowhere in the admin nav to register a tenant's rent payment — /dashboard/payments is the platform's own Stripe subscription billing, while tenant rent money (InvoicePayment) was reachable only by drilling Invoices → detail → "Record payment". Adds /dashboard/rent-payments with KPI tiles, filters, pagination, a record dialog and delete confirm; a new rentPayments permission area (org_admin/finance full, supervisor readonly); and GET /invoice-payments/summary computed over the same where-clause as the list. A supervisor's building scope intersects an explicit buildingId filter, never widens it.

2. feat(notifications) — admin fan-out, portal bell, ticket follow-through
Three partner-reported bugs, all downstream of one gap: every notification went to a single hard-coded recipient, and the tenant portal had no bell at all.

  • New OrgRecipientsService. Keycloak's two lookups each leak — a client-role lookup spans the whole realm, an org lookup spans every role — so neither alone is a tenancy boundary. Recipients are searchUsersByOrg(orgId) ∩ getUsersWithClientRole(ORG_ADMIN); the intersection is the boundary.
  • Tenant maintenance requests and support tickets now notify every org admin. They previously notified nobody — no admin fan-out existed anywhere in the codebase.
  • Maintenance status changes notify the tenant, and TenantMaintenanceRequestView gains updatedAt so the portal can show when staff last touched a request.
  • The portal gains a notifications bell (unread notifications existed but were unreachable) and its nav collapses to a tab bar instead of squashing at tablet widths.
  • Bell and history rows are fully clickable, deep-linking to the request/ticket.
  • i18n: notifications are stored in English with render params in data and translated at render time. Rows written before this change carry no params, so they fall back to the stored English text rather than rendering a broken template.

3. fix(billing)current_period_end from the subscription item
Stripe's basil API moved current_period_end onto each subscription item. The old fallback was billing_cycle_anchor, which is the period start — so every renewal stored a currentPeriodEnd of roughly "now". Now read from the item, with the legacy top-level tried first, and null stored when absent (a missing period end beats a wrong one).

4. chore(deploy) — build fixes the prod deploys already run with
api image 2.36GB → ~950MB via turbo prune + npm prune --omit=dev, keeping the generated Prisma client. @repo/contracts becomes a declared dep of apps/api (it was imported but undeclared, letting turbo race the shared-package build in a clean checkout); tsconfig-paths moves to prod deps since start:prod loads it at runtime. The web image now verifies Tailwind v4's native oxide binding after npm ci and reinstalls it if npm skipped the optional dep (npm/cli#4828) — the lockfile is correct, the install is not, and this repair does fire in practice.

Verification

  • check-types — 4/4 tasks green. This matters: it's what proves commit 1 is no longer broken.
  • api jest — 364 passed, 4 skipped, 30 suites.
  • web vitest — 46 passed (5 files, including new notification-content / notification-target specs).
  • Verified live in-browser on prod, both locales, before this PR: tenant opened a request → admin bell showed "New maintenance request — Layla Hassan (unit G-01) reported: …" (previously "You're all caught up") → click deep-linked to /en/dashboard/tasks/<id> → admin set Resolved → tenant portal showed تم الحل with a last-updated stamp, and the portal bell rendered the translated Arabic title/body. Nav checked at 880px and 1280px with no overlaps.
  • No migrations.

Note on the diff

Local hadisaiibi-mouhannad had diverged (19 ahead / 3 behind) because #174/#186/#188 were squash-merged. This branch is cut fresh from origin/hadisaiibi-mouhannad with only the un-landed work cherry-picked, so the diff contains nothing that was already merged.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added a rent payments dashboard with filtering, pagination, summaries, payment recording, and deletion.
    • Added invoice shortcuts for recording outstanding payments.
    • Added role-based access controls and localized English and Arabic content.
    • Notifications now deep-link to relevant maintenance, support, invoice, and lease pages.
    • Organization administrators receive alerts for new support tickets and tenant maintenance requests.
  • Bug Fixes
    • Improved Stripe subscription period handling.
    • Improved notification navigation, localization fallbacks, and portal display updates.
  • Improvements
    • Maintenance requests now show recent staff updates and refresh automatically.

Mouhannad and others added 4 commits July 27, 2026 01:19
There was nowhere in the admin nav to register a tenant's rent payment.
/dashboard/payments is the platform's own Stripe SUBSCRIPTION billing; tenant
rent money (InvoicePayment) was only reachable by drilling Invoices -> invoice
detail -> "Record payment", with no org-wide list and no nav item.

New surface
- /[lang]/dashboard/rent-payments + sidebar "Rent Payments" + topbar title.
- New permission area `rentPayments`: org_admin/finance full, supervisor
  readonly, maintenance/tenant none. The route path is kebab-case while the
  area key is camelCase, so proxy.ts SUBAREA_ALIASES maps it — otherwise the
  edge treats it as an unknown area and skips gating layer #1.5.

Backend (invoice-payments)
- findAll -> PaginatedResponse<InvoicePaymentListItem> (was {data:[]}), with
  invoiceId/buildingId/renterId/method/from/to/q/page/limit (limit clamped to
  100) and server-side enrichment: renter, unit, building name, and the parent
  invoice's derived status/totals.
- New GET /invoice-payments/summary: collected, MTD, outstanding balance and a
  per-method breakdown, computed over the SAME where-clause as the list so the
  tiles always describe the rows shown. Declared before @get(':id') — Nest
  matches routes in declaration order. outstandingTotal reuses
  computeInvoiceSummary so it reconciles with invoices and reports.
- A supervisor's building scope INTERSECTS an explicit buildingId filter
  (out-of-scope -> {in: []}, never widens) and survives the free-text OR.

Frontend
- listRentPayments + getRentPaymentSummary; listInvoicePayments now peels
  `.items` so invoice-detail needed no changes. Create/delete invalidate the
  register, summary, invoice list and reports.
- KPI tiles, filters with debounced search, enriched table, pagination, record
  dialog (payable-invoice picker + live remaining balance) and delete confirm.
- Invoices row action "Record payment" deep-links with ?recordFor=<invoiceId>.

i18n: new rentPayments namespace, parity 1181 -> 1253 == 1253; payment-method
and invoice-status labels are reused from the invoices namespace.

No migrations. api jest 353 pass/4 skip (this suite 12 -> 29); web vitest 30/30;
check-types and next build clean. Verified against real data in-browser: the
full record -> invoice flips to Paid -> delete -> baseline cycle, supervisor
read-only in UI and 403 on the server.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rough

Three partner-reported bugs, all downstream of the same gap: every notification
was addressed to a single hard-coded recipient, and the tenant portal had no
bell at all.

Admin fan-out (new OrgRecipientsService)
- Keycloak is the source of truth for users, and its two lookups each leak: a
  client-role lookup spans the WHOLE realm, an org lookup spans every role. So
  neither alone is a tenancy boundary. Recipients = searchUsersByOrg(orgId) n
  getUsersWithClientRole(ORG_ADMIN) — the INTERSECTION is the boundary.
- Tenant maintenance requests and support tickets now notify every org admin.
  Previously they notified nobody: no admin fan-out existed anywhere.

Tenant-facing
- Maintenance status changes now notify the tenant, who until now had no signal
  that a request had moved. TenantMaintenanceRequestView gains updatedAt so the
  portal can show when staff last touched it.
- The portal gets a notifications bell — unread notifications existed but were
  unreachable, with no bell on that shell — and its nav collapses to a tab bar
  instead of squashing at tablet widths.
- Rows in both the bell and the history page are fully clickable and deep-link
  to the underlying request/ticket (notification-target.ts), replacing a
  near-untappable link.

i18n
- Notifications are stored in English with their render params in `data`, then
  translated at render time (notification-content.ts). Rows written before this
  change carry no params, so rendering falls back to the stored English text
  rather than showing a broken template.

No migrations.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the anchor

Stripe's basil API (2025-08) moved `current_period_end` off the top-level
Subscription onto each subscription ITEM. The old code fell back to
`billing_cycle_anchor`, but the anchor is the period START — so every renewal
stored a currentPeriodEnd of roughly "now", silently misreporting when the
subscription actually ends.

Read it from the item, keep the legacy top-level read first for accounts still
pinned to an older API version, and store null when neither is present: the
schema allows null, and an absent period end is far better than a wrong one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
These are the build fixes the production Docker deploys have been running with.

api image: 2.36GB -> ~950MB
- `turbo prune property-manager-be --docker` carves out only the api and its
  internal workspace deps, so the web workspace's ~700MB of dependencies stop
  entering an image that never runs them; a final `npm prune --omit=dev` drops
  the build toolchain, restoring the generated Prisma client afterwards.
- `@repo/contracts` is now a declared dependency of apps/api. It was imported
  but undeclared, which let turbo race the shared-package build in a clean
  checkout. `tsconfig-paths` moves to prod deps because `start:prod` loads it
  at runtime.

web image
- npm intermittently skips platform-specific OPTIONAL deps (npm/cli#4828),
  dropping Tailwind v4's native oxide engine and failing `next build` with
  "Cannot find native binding". The lockfile is correct; the install is not.
  The layer now verifies the binding resolves and reinstalls it if missing,
  with the version read from the lockfile's own resolution. This repair does
  fire in practice.

Also refreshes the GitNexus index counts in AGENTS.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@MouhannadKhodor, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 30 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1504416f-c198-4134-92d1-eb9dd258ede0

📥 Commits

Reviewing files that changed from the base of the PR and between 036032d and fc22dbd.

📒 Files selected for processing (15)
  • AGENTS.md
  • apps/api/src/modules/invoice-payments/invoice-payments.service.spec.ts
  • apps/api/src/modules/invoice-payments/invoice-payments.service.ts
  • apps/api/src/modules/notifications/notifications.service.ts
  • apps/api/src/modules/tenant/tenant.service.spec.ts
  • apps/web/src/components/dashboard/notifications-page.tsx
  • apps/web/src/components/dashboard/rent-payments-page.tsx
  • apps/web/src/components/layout/notifications-bell.tsx
  • apps/web/src/components/portal/portal-dict.fallback.ts
  • apps/web/src/components/portal/portal-shell.tsx
  • apps/web/src/components/portal/portal-support.tsx
  • apps/web/src/lib/notification-content.ts
  • apps/web/src/lib/notification-target.ts
  • apps/web/src/store/api/endpoints/invoice-payments.api.ts
  • apps/web/src/tests/unit/notifications/notification-target.test.ts
📝 Walkthrough

Walkthrough

This PR adds a rent-payment register with filtering, summaries, pagination, recording, and deletion; expands notification delivery and routing; exposes tenant request update timestamps; corrects Stripe subscription period mapping; and updates API and web container builds.

Changes

Rent payment register

Layer / File(s) Summary
Contracts and API
packages/contracts/src/index.ts, apps/api/src/modules/invoice-payments/...
Adds enriched payment contracts, filtered list and summary endpoints, pagination, aggregation, scope enforcement, and validation coverage.
Client integration
apps/web/src/store/api/endpoints/invoice-payments.api.ts, apps/web/src/components/dashboard/rent-payments-page.tsx
Adds register and summary queries, cache invalidation, payment filters, pagination, recording, deletion, and invoice deep-linking.
Access and navigation
apps/web/src/auth/permissions.ts, apps/web/src/components/layout/..., apps/web/src/i18n/dictionaries/*
Adds the rent-payments permission area, navigation links, route gating, invoice actions, labels, and localized UI text.

Notification delivery and navigation

Layer / File(s) Summary
Recipient resolution and event delivery
apps/api/src/modules/notifications/..., apps/api/src/modules/{maintenance-requests,support-tickets,tenant}/...
Adds cached organization-admin resolution, bulk enqueueing, and structured notifications for maintenance requests and support tickets.
Localized rendering and routing
apps/web/src/lib/notification-*.ts, apps/web/src/components/{dashboard,layout}/..., apps/web/src/i18n/dictionaries/*
Adds localized notification templates, target resolution, dashboard and portal navigation, and clickable notification rows.

Tenant request progress

Layer / File(s) Summary
Updated request timestamps and portal display
packages/contracts/src/index.ts, apps/api/src/modules/tenant/..., apps/web/src/components/portal/...
Exposes updatedAt, polls tenant requests, and displays localized staff-update progress indicators.

Stripe subscription period handling

Layer / File(s) Summary
Period-end mapping
apps/api/src/modules/webhooks/webhooks.service.ts, apps/api/src/modules/webhooks/webhooks.service.spec.ts
Uses item-level or legacy top-level current_period_end values and stores null when unavailable.

Container builds

Layer / File(s) Summary
Build pipeline
apps/api/Dockerfile, apps/api/package.json, apps/web/Dockerfile
Builds the API from Turbo-pruned artifacts, preserves Prisma output after pruning, validates the API artifact, and repairs missing Tailwind native bindings.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant RentPaymentsPage
  participant API
  participant Database
  User->>RentPaymentsPage: open rent payments
  RentPaymentsPage->>API: request filtered register and summary
  API->>Database: query payments, invoices, and buildings
  Database-->>API: return register rows and aggregates
  API-->>RentPaymentsPage: return payment data
  User->>RentPaymentsPage: record or delete payment
  RentPaymentsPage->>API: submit mutation
  API->>Database: persist payment change
Loading
sequenceDiagram
  participant DomainService
  participant OrgRecipientsService
  participant NotificationsService
  participant DashboardOrPortal
  DomainService->>OrgRecipientsService: resolve notification recipients
  OrgRecipientsService-->>DomainService: return user IDs
  DomainService->>NotificationsService: enqueue notification jobs
  NotificationsService-->>DashboardOrPortal: expose notification data
  DashboardOrPortal->>DashboardOrPortal: localize content and resolve destination
Loading

Possibly related PRs

Suggested labels: enhancement

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is detailed, but it does not follow the required template and omits the Link, QA, and Screenshots sections. Rewrite the PR description using the template headings and add the missing Link to issue or ticket, Steps to QA, and Screenshots sections.
Docstring Coverage ⚠️ Warning Docstring coverage is 22.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main changes: notification fan-out/portal bell, rent-payments, and Stripe billing fixes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/notification-fanout

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

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

CI runs `prettier --check "**/*.{ts,tsx,md}"` and it was failing. Formats only
the 7 files this branch is responsible for: the 4 it adds, plus the 3 whose
edits made them non-compliant.

Whitespace only — api jest 364 pass/4 skip and web vitest 46/46 unchanged after.

The remaining offenders on this branch are pre-existing: they are byte-identical
to `origin/hadisaiibi-mouhannad`, so `format:check` already failed on the base
and is not fixed here. A repo-wide pass belongs in its own commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 13

🧹 Nitpick comments (5)
apps/api/src/modules/invoice-payments/invoice-payments.service.ts (1)

38-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Derive these from their sources instead of hand-maintaining them.

Two small drift risks:

  • PAYMENT_METHODS duplicates the contract's invoicePaymentMethodSchema enum. The InvoicePaymentMethod[] annotation catches removals, but a newly added method would silently be rejected with a 400 here. Export/reuse the zod enum options instead.
  • InvoicePaymentEnrichedRow mirrors ENRICHED_INCLUDE by hand; Prisma.InvoicePaymentGetPayload<{ include: typeof ENRICHED_INCLUDE }> keeps them in lockstep.
♻️ Sketch
-type InvoicePaymentEnrichedRow = InvoicePaymentRow & {
-  invoice: { /* … hand-mirrored … */ };
-};
+type InvoicePaymentEnrichedRow = Prisma.InvoicePaymentGetPayload<{
+  include: typeof ENRICHED_INCLUDE;
+}>;

(requires moving ENRICHED_INCLUDE above the type alias)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/src/modules/invoice-payments/invoice-payments.service.ts` around
lines 38 - 94, Derive both contracts from their existing sources: move
ENRICHED_INCLUDE above InvoicePaymentEnrichedRow and replace the hand-written
row shape with Prisma.InvoicePaymentGetPayload using that include definition.
Replace PAYMENT_METHODS with the exported options from
invoicePaymentMethodSchema so newly added schema methods are accepted
automatically, while preserving the existing validation behavior.
apps/web/src/components/dashboard/rent-payments-page.tsx (1)

73-115: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Derive the zod enum from PAYMENT_METHODS to avoid two sources of truth.

The literal list in z.enum([...]) (Line 104) duplicates PAYMENT_METHODS; adding a method later requires editing both. Also note todayInputValue() uses toISOString(), so the default "paid at" is the UTC date, which can be a day ahead/behind for users in non-UTC zones.

♻️ Proposed refactor
-const PAYMENT_METHODS: InvoicePaymentMethod[] = [
+const PAYMENT_METHODS = [
   'cash',
   'check',
   'bank_transfer',
   'card',
   'other',
-];
+] as const satisfies readonly InvoicePaymentMethod[];
@@
-    method: z.enum(['cash', 'check', 'bank_transfer', 'card', 'other']),
+    method: z.enum(PAYMENT_METHODS),
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/components/dashboard/rent-payments-page.tsx` around lines 73 -
115, Update buildPaymentSchema to derive its method enum from the shared
PAYMENT_METHODS constant, preserving the same validation values without
duplicating the literal list. Also update todayInputValue to compute the user’s
local calendar date rather than slicing toISOString(), so the paid-at default is
not shifted by timezone.
apps/web/src/components/dashboard/notifications-page.tsx (1)

186-192: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Keyboard focus on notification rows is only a background tint. Both row components strip the outline with focus-visible:outline-none and replace it with a low-contrast focus-visible:bg-*, so keyboard users get a weak focus cue on now-navigable rows.

  • apps/web/src/components/dashboard/notifications-page.tsx#L186-L192: add a visible ring (e.g. focus-visible:ring-2 focus-visible:ring-ring) to the card button classes.
  • apps/web/src/components/layout/notifications-bell.tsx#L252-L257: apply the same ring to the sheet row button classes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/components/dashboard/notifications-page.tsx` around lines 186 -
192, Strengthen keyboard focus indicators on both notification row buttons by
adding a visible focus ring alongside the existing focus styles. Update the card
button classes in apps/web/src/components/dashboard/notifications-page.tsx lines
186-192 and the sheet row button classes in
apps/web/src/components/layout/notifications-bell.tsx lines 252-257 with the
same ring treatment, while preserving their existing background and outline
behavior.
apps/api/src/modules/notifications/org-recipients.service.ts (1)

46-76: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Cache stampede on concurrent cold-cache calls.

On a cache miss, resolveOrgAdminUserIds always issues fresh Keycloak calls; the cache is only populated after both awaits resolve, so N concurrent callers for the same orgId (e.g. a burst of tickets arriving before the first lookup completes) will each fire their own pair of Keycloak requests — exactly the "hammering" scenario the docstring says this cache exists to prevent.

♻️ Dedup concurrent lookups with an in-flight promise map
 export class OrgRecipientsService {
   private readonly logger = new Logger(OrgRecipientsService.name);
   private readonly cache = new Map<string, CacheEntry>();
+  private readonly inFlight = new Map<string, Promise<string[]>>();

   ...

   private async resolveOrgAdminUserIds(orgId: string): Promise<string[]> {
     const cached = this.cache.get(orgId);
     if (cached && cached.expiresAt > Date.now()) {
       return cached.userIds;
     }
+    const pending = this.inFlight.get(orgId);
+    if (pending) return pending;

-    try {
-      const [orgUsers, admins] = await Promise.all([...]);
-      ...
-      return userIds;
-    } catch (error) {
-      ...
-      return [];
-    }
+    const promise = (async () => {
+      try {
+        const [orgUsers, admins] = await Promise.all([...]);
+        ...
+        return userIds;
+      } catch (error) {
+        ...
+        return [];
+      } finally {
+        this.inFlight.delete(orgId);
+      }
+    })();
+    this.inFlight.set(orgId, promise);
+    return promise;
   }
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/src/modules/notifications/org-recipients.service.ts` around lines 46
- 76, Update resolveOrgAdminUserIds to deduplicate concurrent cold-cache lookups
per orgId using an in-flight promise map. Reuse the existing promise for callers
while a lookup is pending, populate the cache from the shared lookup, and always
remove the map entry when it settles so later calls can retry after failures.
apps/api/src/modules/support-tickets/support-tickets.service.ts (1)

159-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated "notify org admins" sequence. Both files independently call orgRecipients.getOrgAdminUserIds(orgId, actorId) then build a payload and call notifications.enqueueMany, an identical three-step sequence that will need to be kept in sync as more event types are added.

  • apps/api/src/modules/support-tickets/support-tickets.service.ts#L159-L172: replace this block with a shared helper (e.g. OrgRecipientsService.notifyOrgAdmins(orgId, excludeUserId, notifications, payload) or a small method on NotificationsService) that both this file and tenant.service.ts call.
  • apps/api/src/modules/tenant/tenant.service.ts#L337-L355: same — replace with the shared helper once extracted.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/src/modules/support-tickets/support-tickets.service.ts` around lines
159 - 172, The org-admin notification sequence is duplicated across both
services. Add a shared helper, such as OrgRecipientsService.notifyOrgAdmins or
an equivalent NotificationsService method, that resolves admin recipients,
excludes the actor, and enqueues the supplied payload; replace the inline blocks
in apps/api/src/modules/support-tickets/support-tickets.service.ts lines 159-172
and apps/api/src/modules/tenant/tenant.service.ts lines 337-355 with calls to
that helper while preserving each event’s payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/api/src/modules/invoice-payments/invoice-payments.controller.ts`:
- Around line 55-56: Prevent non-finite query values from reaching Prisma: in
apps/api/src/modules/invoice-payments/invoice-payments.controller.ts lines
55-56, only pass parsed page and limit values when Number.isFinite() succeeds,
otherwise use undefined; in
apps/api/src/modules/invoice-payments/invoice-payments.service.ts lines 311-315,
guard the pagination clamp so unusable page and limit values resolve to 1 and
DEFAULT_LIMIT respectively.

In `@apps/api/src/modules/invoice-payments/invoice-payments.service.ts`:
- Around line 390-414: Optimize the invoice summary query used by the
surrounding summary method instead of loading every invoice and calculating
balances in JavaScript. Add database-level narrowing to exclude settled
invoices, or replace the per-invoice scan with a grouped aggregate that computes
outstanding totals and counts; preserve the existing organization/building
scoping and summary results.
- Around line 195-202: Update the date-range filtering that consumes parseDate
for the to parameter so date-only input includes the entire specified day,
preferably by using an exclusive lt bound at the start of the following day
instead of lte at midnight. Apply this consistently to both payment listing and
summary calculations, and update the affected invoice-payments service spec to
assert the inclusive end-date behavior.

In `@apps/api/src/modules/notifications/org-recipients.service.ts`:
- Line 25: Update the logger import and construction in OrgRecipientsService to
use nestjs-pino’s Logger instead of `@nestjs/common`’s Logger, while preserving
the existing OrgRecipientsService context name and logger field.

In `@apps/api/src/modules/webhooks/webhooks.service.spec.ts`:
- Around line 139-191: The current spec incorrectly expects top-level
current_period_end to take precedence. Update the second test around
WebhooksService.handleSubscriptionUpsert to include both top-level and
item-level current_period_end values, assert that the persisted currentPeriodEnd
uses the item value, and rename the test to describe item-level precedence.

In `@apps/api/src/modules/webhooks/webhooks.service.ts`:
- Around line 201-204: The period-end fallback in the webhook service must
prioritize subscription.items.data[0].current_period_end over the legacy
subscription.current_period_end value. Update the periodEndRaw selection
accordingly, and add a test in
apps/api/src/modules/webhooks/webhooks.service.spec.ts covering both values and
asserting the item-level timestamp wins.

In `@apps/web/Dockerfile`:
- Around line 15-27: Update the RUN command containing npm ci and the
`@tailwindcss/oxide` check so the binding verification and repair execute only
after npm ci succeeds, without allowing a failed install into the repair block.
In the repair command, derive the Oxide package suffix from Node’s runtime
platform and architecture, using process.platform and process.arch, instead of
hard-coding linux-x64-gnu; preserve the lockfile-derived Oxide version and final
binding verification.

In `@apps/web/src/app/`[lang]/dashboard/rent-payments/page.tsx:
- Around line 16-20: Update the rent-payments route around requireSession to
call requireActiveOrg and requireRole using the shared allowed rent-payment
roles. Retain canAccess for access handling and use canWrite only for read-only
UI behavior; do not rely on canWrite or normalized role checks as a substitute
for the required guards.

In `@apps/web/src/components/dashboard/invoices-page.tsx`:
- Around line 901-912: The payment menu item near the invoice action currently
checks only invoice status; additionally require canWrite(role, 'rentPayments')
before rendering it. Preserve the existing non-paid condition and routing
behavior, using the existing role access check so users without rent-payment
write permission cannot access the action.

In `@apps/web/src/components/dashboard/rent-payments-page.tsx`:
- Around line 147-172: Run the project formatter across the changed files,
including the rent-payments page containing dateFormatter and the debounced
setSearch effect, and commit the resulting Prettier formatting changes without
altering behavior.

In `@apps/web/src/components/portal/portal-dict.fallback.ts`:
- Around line 78-81: Update the canonical locale catalogs under
i18n/dictionaries to define portal.support.updated in both locales, using the
same “Updated” value currently mirrored in portal-dict.fallback.ts. Keep the
fallback entries only as the documented emergency mirror and ensure the UI
resolves this text through the dictionary keys.

In `@apps/web/src/i18n/dictionaries/ar.json`:
- Around line 1261-1268: Update the Arabic payment-delete confirmation strings
in the "delete" dictionary entry so the assembled message has consistent
punctuation and intent: either make confirmPrefix an explicit question like the
invoices dialog or remove the question mark beginning confirmSuffix. Preserve
the existing deletion and balance-recalculation meaning.

In `@apps/web/src/lib/notification-content.ts`:
- Around line 84-114: Run Prettier on the affected notification files. In
apps/web/src/lib/notification-content.ts lines 84-114, reformat the over-width
conditions; in apps/web/src/lib/notification-target.ts lines 66-74, reformat the
over-width return; and in
apps/web/src/tests/unit/notifications/notification-target.test.ts lines 32-36
(including the additional expectation at line 98), reformat the over-width
expectations. No behavioral changes are needed.

---

Nitpick comments:
In `@apps/api/src/modules/invoice-payments/invoice-payments.service.ts`:
- Around line 38-94: Derive both contracts from their existing sources: move
ENRICHED_INCLUDE above InvoicePaymentEnrichedRow and replace the hand-written
row shape with Prisma.InvoicePaymentGetPayload using that include definition.
Replace PAYMENT_METHODS with the exported options from
invoicePaymentMethodSchema so newly added schema methods are accepted
automatically, while preserving the existing validation behavior.

In `@apps/api/src/modules/notifications/org-recipients.service.ts`:
- Around line 46-76: Update resolveOrgAdminUserIds to deduplicate concurrent
cold-cache lookups per orgId using an in-flight promise map. Reuse the existing
promise for callers while a lookup is pending, populate the cache from the
shared lookup, and always remove the map entry when it settles so later calls
can retry after failures.

In `@apps/api/src/modules/support-tickets/support-tickets.service.ts`:
- Around line 159-172: The org-admin notification sequence is duplicated across
both services. Add a shared helper, such as OrgRecipientsService.notifyOrgAdmins
or an equivalent NotificationsService method, that resolves admin recipients,
excludes the actor, and enqueues the supplied payload; replace the inline blocks
in apps/api/src/modules/support-tickets/support-tickets.service.ts lines 159-172
and apps/api/src/modules/tenant/tenant.service.ts lines 337-355 with calls to
that helper while preserving each event’s payload.

In `@apps/web/src/components/dashboard/notifications-page.tsx`:
- Around line 186-192: Strengthen keyboard focus indicators on both notification
row buttons by adding a visible focus ring alongside the existing focus styles.
Update the card button classes in
apps/web/src/components/dashboard/notifications-page.tsx lines 186-192 and the
sheet row button classes in
apps/web/src/components/layout/notifications-bell.tsx lines 252-257 with the
same ring treatment, while preserving their existing background and outline
behavior.

In `@apps/web/src/components/dashboard/rent-payments-page.tsx`:
- Around line 73-115: Update buildPaymentSchema to derive its method enum from
the shared PAYMENT_METHODS constant, preserving the same validation values
without duplicating the literal list. Also update todayInputValue to compute the
user’s local calendar date rather than slicing toISOString(), so the paid-at
default is not shifted by timezone.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 644e9644-9a03-4290-9298-4ea4a37faf73

📥 Commits

Reviewing files that changed from the base of the PR and between b15037e and 036032d.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (43)
  • AGENTS.md
  • apps/api/Dockerfile
  • apps/api/package.json
  • apps/api/src/modules/invoice-payments/invoice-payments.controller.ts
  • apps/api/src/modules/invoice-payments/invoice-payments.service.spec.ts
  • apps/api/src/modules/invoice-payments/invoice-payments.service.ts
  • apps/api/src/modules/maintenance-requests/maintenance-requests.module.ts
  • apps/api/src/modules/maintenance-requests/maintenance-requests.service.spec.ts
  • apps/api/src/modules/maintenance-requests/maintenance-requests.service.ts
  • apps/api/src/modules/notifications/notifications.module.ts
  • apps/api/src/modules/notifications/notifications.service.ts
  • apps/api/src/modules/notifications/org-recipients.service.spec.ts
  • apps/api/src/modules/notifications/org-recipients.service.ts
  • apps/api/src/modules/support-tickets/support-tickets.service.spec.ts
  • apps/api/src/modules/support-tickets/support-tickets.service.ts
  • apps/api/src/modules/tenant/tenant.module.ts
  • apps/api/src/modules/tenant/tenant.service.spec.ts
  • apps/api/src/modules/tenant/tenant.service.ts
  • apps/api/src/modules/webhooks/webhooks.service.spec.ts
  • apps/api/src/modules/webhooks/webhooks.service.ts
  • apps/web/Dockerfile
  • apps/web/src/app/[lang]/dashboard/rent-payments/page.tsx
  • apps/web/src/app/api/invoice-payments/summary/route.ts
  • apps/web/src/auth/permissions.ts
  • apps/web/src/components/dashboard/invoices-page.tsx
  • apps/web/src/components/dashboard/notifications-page.tsx
  • apps/web/src/components/dashboard/rent-payments-page.tsx
  • apps/web/src/components/layout/dashboard-sidebar.tsx
  • apps/web/src/components/layout/dashboard-topbar.tsx
  • apps/web/src/components/layout/notifications-bell.tsx
  • apps/web/src/components/portal/portal-dict.fallback.ts
  • apps/web/src/components/portal/portal-dict.ts
  • apps/web/src/components/portal/portal-shell.tsx
  • apps/web/src/components/portal/portal-support.tsx
  • apps/web/src/i18n/dictionaries/ar.json
  • apps/web/src/i18n/dictionaries/en.json
  • apps/web/src/lib/notification-content.ts
  • apps/web/src/lib/notification-target.ts
  • apps/web/src/proxy.ts
  • apps/web/src/store/api/endpoints/invoice-payments.api.ts
  • apps/web/src/tests/unit/notifications/notification-content.test.ts
  • apps/web/src/tests/unit/notifications/notification-target.test.ts
  • packages/contracts/src/index.ts

Comment on lines +55 to +56
page: page ? Number(page) : undefined,
limit: limit ? Number(limit) : undefined,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Non-numeric page/limit become NaN and reach Prisma as skip/take. The root cause is that the query string is converted with Number() without a finiteness check, and the service clamp propagates NaN (Math.max(1, NaN) is NaN), so ?page=abc returns a 500 instead of falling back to the defaults.

  • apps/api/src/modules/invoice-payments/invoice-payments.controller.ts#L55-L56: only pass the parsed value through when Number.isFinite() holds, otherwise leave it undefined.
  • apps/api/src/modules/invoice-payments/invoice-payments.service.ts#L311-L315: guard the clamp with a finite check so page/limit always resolve to 1/DEFAULT_LIMIT for unusable input.
📍 Affects 2 files
  • apps/api/src/modules/invoice-payments/invoice-payments.controller.ts#L55-L56 (this comment)
  • apps/api/src/modules/invoice-payments/invoice-payments.service.ts#L311-L315
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/src/modules/invoice-payments/invoice-payments.controller.ts` around
lines 55 - 56, Prevent non-finite query values from reaching Prisma: in
apps/api/src/modules/invoice-payments/invoice-payments.controller.ts lines
55-56, only pass parsed page and limit values when Number.isFinite() succeeds,
otherwise use undefined; in
apps/api/src/modules/invoice-payments/invoice-payments.service.ts lines 311-315,
guard the pagination clamp so unusable page and limit values resolve to 1 and
DEFAULT_LIMIT respectively.

Comment on lines +195 to +202
private parseDate(value: string | undefined, field: string): Date | undefined {
if (!value) return undefined;
const parsed = new Date(value);
if (Number.isNaN(parsed.getTime())) {
throw new BadRequestException(`Invalid "${field}" date.`);
}
return parsed;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

to is exclusive of the end date in practice — payments on the final day are dropped.

paidAt is a timestamp, but parseDate('2026-01-31') yields 2026-01-31T00:00:00Z, so lte: to excludes every payment recorded during that day. The UI feeds <input type="date"> values, so this is the normal path, and both the list and the summary tiles inherit the miss.

Make the upper bound end-of-day for date-only input (an exclusive lt on the next day is the cleanest). The spec at invoice-payments.service.spec.ts lines 262-265 currently asserts the truncating behavior and would need updating too.

🐛 Proposed fix
-  private parseDate(value: string | undefined, field: string): Date | undefined {
-    if (!value) return undefined;
-    const parsed = new Date(value);
-    if (Number.isNaN(parsed.getTime())) {
-      throw new BadRequestException(`Invalid "${field}" date.`);
-    }
-    return parsed;
-  }
+  private parseDate(
+    value: string | undefined,
+    field: string,
+    // A bare `YYYY-MM-DD` upper bound means "through the end of that day".
+    endOfDay = false,
+  ): Date | undefined {
+    if (!value) return undefined;
+    const parsed = new Date(value);
+    if (Number.isNaN(parsed.getTime())) {
+      throw new BadRequestException(`Invalid "${field}" date.`);
+    }
+    if (endOfDay && /^\d{4}-\d{2}-\d{2}$/.test(value)) {
+      return new Date(parsed.getTime() + 24 * 60 * 60 * 1000 - 1);
+    }
+    return parsed;
+  }
     const to = opts.ignoreDateRange
       ? undefined
-      : this.parseDate(filters.to, 'to');
+      : this.parseDate(filters.to, 'to', true);
🧰 Tools
🪛 GitHub Actions: CI / 0_check.txt

[warning] Prettier --check reported formatting issues.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/src/modules/invoice-payments/invoice-payments.service.ts` around
lines 195 - 202, Update the date-range filtering that consumes parseDate for the
to parameter so date-only input includes the entire specified day, preferably by
using an exclusive lt bound at the start of the following day instead of lte at
midnight. Apply this consistently to both payment listing and summary
calculations, and update the affected invoice-payments service spec to assert
the inclusive end-date behavior.

Comment on lines +390 to +414
this.prisma.invoice.findMany({
where: invoiceWhere,
select: {
dueDate: true,
lineItems: { select: { amount: true } },
payments: { select: { amount: true } },
},
}),
]);

let outstandingTotal = 0;
let outstandingInvoices = 0;
for (const invoice of invoices) {
const { totalAmount, paidAmount } = computeInvoiceSummary(
invoice.lineItems.map((li) => ({ amount: li.amount.toNumber() })),
invoice.payments.map((p) => ({ amount: p.amount.toNumber() })),
invoice.dueDate,
now,
);
const balance = totalAmount - paidAmount;
if (balance > 0) {
outstandingTotal += balance;
outstandingInvoices += 1;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift

Unbounded invoice scan on every summary request.

invoice.findMany here has no pagination and no status narrowing: it loads every invoice in the org (or in the supervisor's buildings) plus all of their line items and payments, then sums in JS. The web page calls this on every filter change (debounced search included), so the cost grows linearly with the org's invoice history on a request thread.

At minimum exclude already-settled invoices at the DB level or compute the balance with a grouped aggregate; a cached/periodically-recomputed outstanding figure would be better still since it ignores the method/date/q filters anyway.

🧰 Tools
🪛 GitHub Actions: CI / 0_check.txt

[warning] Prettier --check reported formatting issues.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/src/modules/invoice-payments/invoice-payments.service.ts` around
lines 390 - 414, Optimize the invoice summary query used by the surrounding
summary method instead of loading every invoice and calculating balances in
JavaScript. Add database-level narrowing to exclude settled invoices, or replace
the per-invoice scan with a grouped aggregate that computes outstanding totals
and counts; preserve the existing organization/building scoping and summary
results.

*/
@Injectable()
export class OrgRecipientsService {
private readonly logger = new Logger(OrgRecipientsService.name);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how logging is done elsewhere in the API and whether nestjs-pino is configured.
rg -n "nestjs-pino" apps/api/src -l
rg -n "new Logger\(" apps/api/src/modules -g '*.ts'

Repository: chrisshaddad/bootcamp-starter

Length of output: 2930


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== app.module.ts relevant imports/config =="
sed -n '1,220p' apps/api/src/app.module.ts | rg -n "PinoLogger|LoggerModule|Logger|nestjs-pino|ModuleMetadata|imports" -C 3 || true

echo
echo "== main.ts logging setup =="
sed -n '1,180p' apps/api/src/main.ts | rg -n "PinoLogger|Logger|nestjs-pino|useLogger|LoggerModule" -C 5 || true

echo
echo "== package references to nestjs-pino/common packages =="
rg -n '"`@nestjs` (?:platform-express|common)"|nestjs-pino|`@nestjs/common`' package.json apps/api/package.json 2>/dev/null || true

Repository: chrisshaddad/bootcamp-starter

Length of output: 2377


Route logging through nestjs-pino.

LoggerModule is registered in AppModule, and main.ts installs the nestjs-pino Logger as the app logger. Construct the injected Logger from the nestjs-pino import here instead of @nestjs/common’s Logger to keep this path using the configured pino logger.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/src/modules/notifications/org-recipients.service.ts` at line 25,
Update the logger import and construction in OrgRecipientsService to use
nestjs-pino’s Logger instead of `@nestjs/common`’s Logger, while preserving the
existing OrgRecipientsService context name and logger field.

Source: Path instructions

Comment on lines +139 to +191
describe('WebhooksService.handleSubscriptionUpsert (currentPeriodEnd)', () => {
it('reads current_period_end from the subscription ITEM, never billing_cycle_anchor', async () => {
const { service, prisma } = makeService();
prisma.organization.findFirst.mockResolvedValue({ id: 'org-1' });

await service.handleEvent(
subscriptionEvent({
id: 'sub_1',
status: 'active',
customer: 'cus_1',
billing_cycle_anchor: ANCHOR,
items: {
data: [{ price: { id: 'price_1' }, current_period_end: ITEM_END }],
},
}),
);

expect(prisma.subscription.upsert).toHaveBeenCalledWith(
expect.objectContaining({
where: { orgId: 'org-1' },
create: expect.objectContaining({
currentPeriodEnd: new Date(ITEM_END * 1000),
}),
update: expect.objectContaining({
currentPeriodEnd: new Date(ITEM_END * 1000),
}),
}),
);
});

it('prefers a legacy top-level current_period_end when present', async () => {
const { service, prisma } = makeService();
prisma.organization.findFirst.mockResolvedValue({ id: 'org-1' });

await service.handleEvent(
subscriptionEvent({
id: 'sub_1',
status: 'active',
customer: 'cus_1',
current_period_end: ITEM_END,
billing_cycle_anchor: ANCHOR,
items: { data: [{ price: { id: 'price_1' } }] },
}),
);

expect(prisma.subscription.upsert).toHaveBeenCalledWith(
expect.objectContaining({
create: expect.objectContaining({
currentPeriodEnd: new Date(ITEM_END * 1000),
}),
}),
);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate file =="
git ls-files | rg 'apps/api/src/modules/webhooks/webhooks.service.spec.ts|apps/api/src/modules/webhooks/webhooks.service.ts' || true

echo "== spec relevant lines =="
if [ -f apps/api/src/modules/webhooks/webhooks.service.spec.ts ]; then
  sed -n '1,240p' apps/api/src/modules/webhooks/webhooks.service.spec.ts | cat -n
fi

echo "== service outline/implementation relevant =="
if [ -f apps/api/src/modules/webhooks/webhooks.service.ts ]; then
  ast-grep outline apps/api/src/modules/webhooks/webhooks.service.ts || true
  echo "== relevant service implementation =="
  rg -n "currentPeriodEnd|current_period_end|billing_cycle_anchor|handleSubscription|handleEvent" apps/api/src/modules/webhooks/webhooks.service.ts -C 4
fi

Repository: chrisshaddad/bootcamp-starter

Length of output: 12445


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== parse deterministic precedence model from source =="
python3 - <<'PY'
from pathlib import Path
import re

p = Path('apps/api/src/modules/webhooks/webhooks.service.ts')
if p.exists():
    s = p.read_text()
    for i, line in enumerate(s.splitlines(), 1):
        if re.search(r'current_period_end|currentPeriodEnd|billing_cycle_anchor', line):
            start=max(1,i-8); end=min(len(s.splitlines()), i+8)
            print(f"-- around line {i} --")
            for n in range(start, end+1):
                print(f"{n}: {s.splitlines()[n-1]}")
PY

echo "== fixture definitions =="
python3 - <<'PY'
from pathlib import Path
p=Path('apps/api/src/modules/webhooks/webhooks.service.spec.ts')
if p.exists():
    for token in ['ITEM_END','ANCHOR','subscriptionEvent']:
        print(f"-- {token} --")
        for i,line in enumerate(p.read_text().splitlines(),1):
            if token in line:
                print(f"{i}: {line}")
PY

Repository: chrisshaddad/bootcamp-starter

Length of output: 8168


Fail the spec when top-level precedence is left in place.

The implementation reads current_period_end at the subscription level before falling back to subscription.items.data[0].current_period_end, but the fixture tests only single-field payload shapes. Add one with both top-level and item-level values to assert item precedence, and rename the fallback case.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/api/src/modules/webhooks/webhooks.service.spec.ts` around lines 139 -
191, The current spec incorrectly expects top-level current_period_end to take
precedence. Update the second test around
WebhooksService.handleSubscriptionUpsert to include both top-level and
item-level current_period_end values, assert that the persisted currentPeriodEnd
uses the item value, and rename the test to describe item-level precedence.

Comment on lines +901 to +912
{invoice.status !== 'paid' && (
<DropdownMenuItem
onClick={() =>
router.push(
`/${locale}/dashboard/rent-payments?recordFor=${encodeURIComponent(invoice.id)}`,
)
}
>
<WalletIcon className="size-3.5 mr-1.5" />
{t.list.recordPayment}
</DropdownMenuItem>
)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the supervisor row's invoices/rentPayments access levels.
rg -n -A 20 "supervisor: \{" apps/web/src/auth/permissions.ts
# Check how canWrite is derived for the invoices page.
rg -n -C 5 "InvoicesPage" apps/web/src/app --glob '*.tsx'

Repository: chrisshaddad/bootcamp-starter

Length of output: 2663


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== invoices-page relevant section =="
sed -n '870,930p' apps/web/src/components/dashboard/invoices-page.tsx

echo
echo "== canAccess/canWrite definitions =="
sed -n '1,140p' apps/web/src/auth/permissions.ts

echo
echo "== rent-payments route gating =="
rg -n -C 4 "rent payments|rentPayments|role|canWrite|canAccess|dialog" apps/web/src/app/[lang]/dashboard/rent-payments -g '*.tsx'

echo
echo "== read-only guards in rent-payments =="
rg -n -C 2 "readonly|dialog|open|status|recordFor|query|setRecord" apps/web/src/app/[lang]/dashboard/rent-payments -g '*.tsx'

Repository: chrisshaddad/bootcamp-starter

Length of output: 10321


Gate the payment action on rent-payment write access.

canWrite here is canWrite(role, 'invoices'), so users with invoices write but only readonly rentPayments access (for example, a future custom role with this split) can get routed to /dashboard/rent-payments?recordFor=... without the payment dialog opening. Gate this menu item on canWrite(role, 'rentPayments').

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/components/dashboard/invoices-page.tsx` around lines 901 - 912,
The payment menu item near the invoice action currently checks only invoice
status; additionally require canWrite(role, 'rentPayments') before rendering it.
Preserve the existing non-paid condition and routing behavior, using the
existing role access check so users without rent-payment write permission cannot
access the action.

Comment thread apps/web/src/components/dashboard/rent-payments-page.tsx Outdated
Comment on lines 78 to +81
created: 'Request submitted.',
createError: 'Something went wrong. Please try again.',
unit: 'Unit',
updated: 'Updated',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Keep the new copy in the canonical dictionaries.

updated is user-facing text added to portal-dict.fallback.ts, while the web guideline requires user-facing strings to come from i18n/dictionaries/. Add or verify portal.support.updated in both canonical locale catalogs, keeping these values only as the documented emergency mirror.

As per coding guidelines, every user-facing string must come from i18n/dictionaries/, and new strings must be added to both locales.

Also applies to: 186-189

🧰 Tools
🪛 GitHub Actions: CI / 0_check.txt

[warning] Prettier --check reported formatting issues.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/components/portal/portal-dict.fallback.ts` around lines 78 - 81,
Update the canonical locale catalogs under i18n/dictionaries to define
portal.support.updated in both locales, using the same “Updated” value currently
mirrored in portal-dict.fallback.ts. Keep the fallback entries only as the
documented emergency mirror and ensure the UI resolves this text through the
dictionary keys.

Source: Coding guidelines

Comment on lines +1261 to +1268
"delete": {
"title": "حذف الدفعة",
"confirmPrefix": "سيتم حذف دفعة بمبلغ",
"confirmSuffix": "؟ سيُعاد احتساب رصيد الفاتورة.",
"confirming": "جارٍ الحذف…",
"success": "تم حذف الدفعة.",
"error": "تعذّر حذف الدفعة."
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Delete-confirm copy reads oddly: declarative prefix, interrogative suffix.

confirmPrefix ("سيتم حذف دفعة بمبلغ") is a statement, but confirmSuffix opens with "؟", producing "سيتم حذف دفعة بمبلغ X؟ سيُعاد احتساب رصيد الفاتورة." Either phrase the prefix as a question (as the invoices dialog does at Line 1174, "هل أنت متأكد…") or drop the question mark.

✏️ Proposed wording
-        "confirmPrefix": "سيتم حذف دفعة بمبلغ",
-        "confirmSuffix": "؟ سيُعاد احتساب رصيد الفاتورة.",
+        "confirmPrefix": "هل أنت متأكد من حذف دفعة بمبلغ",
+        "confirmSuffix": "؟ سيُعاد احتساب رصيد الفاتورة.",
📝 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
"delete": {
"title": "حذف الدفعة",
"confirmPrefix": "سيتم حذف دفعة بمبلغ",
"confirmSuffix": "؟ سيُعاد احتساب رصيد الفاتورة.",
"confirming": "جارٍ الحذف…",
"success": "تم حذف الدفعة.",
"error": "تعذّر حذف الدفعة."
}
"delete": {
"title": "حذف الدفعة",
"confirmPrefix": "هل أنت متأكد من حذف دفعة بمبلغ",
"confirmSuffix": "؟ سيُعاد احتساب رصيد الفاتورة.",
"confirming": "جارٍ الحذف…",
"success": "تم حذف الدفعة.",
"error": "تعذّر حذف الدفعة."
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/i18n/dictionaries/ar.json` around lines 1261 - 1268, Update the
Arabic payment-delete confirmation strings in the "delete" dictionary entry so
the assembled message has consistent punctuation and intent: either make
confirmPrefix an explicit question like the invoices dialog or remove the
question mark beginning confirmSuffix. Preserve the existing deletion and
balance-recalculation meaning.

Comment thread apps/web/src/lib/notification-content.ts
Formats the 8 files this PR modifies that were already non-compliant on the base
branch, so every file it touches is now clean. Whitespace only — api jest 364
pass/4 skip, web vitest 46/46, check-types 4/4 all unchanged after.

`format:check` still fails on 30 files this PR does not touch (byte-identical to
`origin/hadisaiibi-mouhannad` — the base was already red). Clearing those is a
repo-wide pass and deliberately not bundled here.

Note: AGENTS.md's reformat falls inside the `gitnexus:start` block, which
`gitnexus analyze` regenerates — expect it to drift non-compliant again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant