Daily Closure

[alessandrobellesia]

fix(daily-closure)!: align DailyClosure type with actual API response

BREAKING CHANGE: DailyClosure type now matches real API with shop_daily_closure and device_daily_closure objects. Removed incorrect payments_count field. Added Date object support to get() method.

Summary by CodeRabbit

• New Features

  • DailyClosure.get() now accepts Date objects in addition to YYYYMMDD strings for flexible date input
  • Added new utility function for date formatting

• Breaking Changes

  • DailyClosure response structure redesigned with nested shop and device closure objects containing detailed financial data

• Documentation

  • Updated examples to reflect new API structure and recommended Date object usage patterns

• Tests

  • Added comprehensive unit and end-to-end test coverage for new date handling and response structure

• Chores

  • Updated development dependencies

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The DailyClosure API type structure has been restructured to align with Satispay's response format, replacing flat fields with nested shop_daily_closure and device_daily_closure objects. The DailyClosure.get() method now accepts Date objects alongside strings, supported by a new DateUtils.formatToYYYYMMDD() utility. Tests and documentation updated accordingly.

Changes

Cohort / File(s)	Summary
Core API Structure src/types.ts	Introduced DailyClosureDetail type; restructured DailyClosure from flat fields (date, total_amount_unit, currency, payments_count) to nested shop_daily_closure and device_daily_closure objects
Daily Closure Implementation src/DailyClosure.ts	Updated get() method signature to accept Date | string; added internal dateString normalization logic using DateUtils.formatToYYYYMMDD() for Date object support
Utility Functions src/utils.ts	Added DateUtils.formatToYYYYMMDD(date: Date | string): string utility function for formatting dates to YYYYMMDD format
Unit Tests tests/utils.test.ts, tests/DailyClosure.test.ts	Added tests for new DateUtils.formatToYYYYMMDD() function; updated DailyClosure tests to validate Date object and string date inputs, reflecting new nested type structure
End-to-End Tests tests/e2e/daily-closure.e2e.test.ts	Added comprehensive e2e test suite validating daily closure fetching via Date objects and string dates, nested field structures, and amount calculation consistency
Documentation & Configuration README.md, CHANGELOG.md, package.json	Updated README examples to showcase Date object usage and nested response structure; added changelog entries; bumped devDependencies (Vitest, Prettier, TypeScript ESLint) to version 4.0.15

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Type structure changes: DailyClosure restructuring impacts implementation and test files; verify all references updated correctly
• Date handling logic: New Date object conversion in DailyClosure.get() and DateUtils utility; ensure zero-padding and edge cases (year boundaries) handled correctly
• Backward compatibility: Confirm string date format (YYYYMMDD) still functions as before
• Test coverage: Verify e2e tests account for timezone considerations and API response validation

Poem

A rabbit redoes closures with flair,
Nesting details with utmost care,
Date objects bloom where strings once lay,
Utils format the YYYYMMDD way,
Clean code hops forward, hip-hip-hooray! 🐰📅✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Daily Closure' is vague and generic, lacking specific information about the actual changes (type restructuring, breaking changes, date object support).	Consider a more descriptive title that captures the main change, such as 'Refactor DailyClosure type to match API response structure' or 'Fix DailyClosure type alignment with Satispay API'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/daily-closure

---

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

tests/utils.test.ts (1)

84-98: Potential timezone-related test flakiness.

The tests use UTC timestamps (e.g., '2024-01-15T10:30:00Z') but formatToYYYYMMDD uses local time methods (getFullYear, getMonth, getDate). In timezones with large negative UTC offsets, the local date may differ from the UTC date, causing test failures.

Consider using dates without the Z suffix or choosing timestamps that are robust across timezones (e.g., noon UTC):

 it('should format Date object to YYYYMMDD', () => {
-  const date = new Date('2024-01-15T10:30:00Z')
+  const date = new Date(2024, 0, 15) // Local date, no timezone ambiguity
   expect(DateUtils.formatToYYYYMMDD(date)).toBe('20240115')
 })

 it('should pad single digit months and days', () => {
-  const date = new Date('2024-03-05T10:30:00Z')
+  const date = new Date(2024, 2, 5) // Local date
   expect(DateUtils.formatToYYYYMMDD(date)).toBe('20240305')
 })

src/utils.ts (1)

98-104: Consider handling invalid date strings.

If an invalid string is passed (e.g., 'not-a-date'), new Date(date) creates an Invalid Date, and the function returns "NaNNaNNaN". This could cause confusing API errors downstream.

 formatToYYYYMMDD(date: Date | string): string {
   const d = typeof date === 'string' ? new Date(date) : date
+  if (isNaN(d.getTime())) {
+    throw new Error(`Invalid date: ${date}`)
+  }
   const year = d.getFullYear()
   const month = String(d.getMonth() + 1).padStart(2, '0')
   const day = String(d.getDate()).padStart(2, '0')
   return `${year}${month}${day}`
 },

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 318d3e5 and 7b05d60.

⛔ Files ignored due to path filters (1)

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (9)

• CHANGELOG.md (2 hunks)
• README.md (1 hunks)
• package.json (1 hunks)
• src/DailyClosure.ts (2 hunks)
• src/types.ts (1 hunks)
• src/utils.ts (1 hunks)
• tests/DailyClosure.test.ts (2 hunks)
• tests/e2e/daily-closure.e2e.test.ts (1 hunks)
• tests/utils.test.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (5)

tests/e2e/daily-closure.e2e.test.ts (3)

tests/setup.ts (2)

• canRunE2ETests (49-55)
• hasAuthenticationKeys (61-63)

src/DailyClosure.ts (1)

• DailyClosure (8-50)

src/types.ts (1)

• DailyClosure (211-214)

tests/DailyClosure.test.ts (2)

src/DailyClosure.ts (1)

• DailyClosure (8-50)

src/types.ts (1)

• DailyClosure (211-214)

src/types.ts (2)

src/DailyClosure.ts (1)

• DailyClosure (8-50)

src/index.ts (1)

• DailyClosure (26-26)

src/DailyClosure.ts (3)

src/types.ts (1)

• DailyClosureQueryParams (246-249)

src/utils.ts (1)

• DateUtils (80-195)

src/index.ts (1)

• DateUtils (36-36)

tests/utils.test.ts (1)

src/utils.ts (1)

• DateUtils (80-195)

🔇 Additional comments (16)

package.json (1)

66-76: LGTM!

DevDependency updates look reasonable. The Vitest-related packages are properly synchronized at version 4.0.15.

src/types.ts (1)

196-214: LGTM!

The restructured DailyClosure type with nested shop_daily_closure and device_daily_closure objects aligns with the actual Satispay API response. Good use of DRY by extracting the common DailyClosureDetail type.

README.md (1)

234-245: LGTM!

Documentation is well-updated to reflect the new API structure. Good examples showing both Date object (recommended) and YYYYMMDD string inputs, along with the nested response structure access pattern.

src/DailyClosure.ts (4)

3-3: LGTM: DateUtils import added.

The import is necessary for the new Date object support and is correctly placed.

---

14-17: LGTM: Documentation updated correctly.

The JSDoc accurately describes the new Date object support and the expected string format.

---

33-33: LGTM: Path construction updated correctly.

The path construction properly uses the normalized dateString variable.

---

19-31: Add input validation and verify timezone behavior.

The date normalization logic handles multiple input types, but several gaps should be addressed:

1. Invalid Date object handling: Missing validation for invalid Date objects created by new Date('invalid'). Add a check: if (isNaN(date.getTime())) throw new Error('Invalid Date object provided')

2. String format validation: No check that string inputs follow the expected YYYYMMDD format. Add: /^\d{8}$/.test(date)

3. Timezone consistency: Ensure DateUtils.formatToYYYYMMDD() handles timezones consistently. JavaScript Date objects can produce different results depending on the user's local timezone when using getFullYear(), getMonth(), and getDate(). Document whether the implementation uses local or UTC methods, and ensure this aligns with API expectations.

Add explicit validation for both invalid Date objects and improperly formatted strings.

tests/e2e/daily-closure.e2e.test.ts (5)

1-13: LGTM: E2E test setup is well-structured.

The test suite properly checks for required authentication keys and includes clear documentation about test requirements.

---

15-44: LGTM: Comprehensive validation of today's daily closure.

The test thoroughly validates the nested structure and field types, ensuring the API response matches the expected DailyClosure type.

---

46-61: LGTM: Date object support tested correctly.

The test validates that Date objects are properly accepted and formatted. The manual date string construction at lines 55-56 duplicates the logic in DateUtils.formatToYYYYMMDD(), but this is acceptable in tests to independently verify the behavior.

---

63-95: LGTM: String date and date arithmetic tested correctly.

The tests validate both string date input (YYYYMMDD format) and date arithmetic (yesterday's closure), ensuring the API handles various date input scenarios correctly.

---

97-110: LGTM: Amount calculation validation is excellent.

This test ensures the API response data is internally consistent by verifying that amount_unit = gross_amount_unit - refund_amount_unit for both shop and device closures.

tests/DailyClosure.test.ts (3)

15-33: LGTM: Mock data updated to reflect new API structure.

The mock data correctly represents the nested shop_daily_closure and device_daily_closure structure with all required fields.

---

81-91: LGTM: Date object test is well-structured.

The test validates that Date objects are properly formatted to YYYYMMDD format and used in the API path.

---

93-103: LGTM: String date test validates correct behavior.

The test ensures string dates in YYYYMMDD format are used directly in the API path without transformation.

CHANGELOG.md (1)

8-31: LGTM: Comprehensive changelog entry.

The changelog entry thoroughly documents all breaking changes, additions, and fixes, with clear migration guidance for users upgrading from previous versions. The BREAKING CHANGE markers are appropriately used, and the format follows Keep a Changelog standards.