Skip to content

feat: database migrations and model foundation for limits by period#60

Open
alexgarzao wants to merge 4 commits intodevelopfrom
feature/limits-by-period-pr1-migrations-model-foundation
Open

feat: database migrations and model foundation for limits by period#60
alexgarzao wants to merge 4 commits intodevelopfrom
feature/limits-by-period-pr1-migrations-model-foundation

Conversation

@alexgarzao
Copy link
Collaborator

PR 1: Database Migrations + Model Foundation

Schema changes for new limit types (WEEKLY, CUSTOM, PER_TRANSACTION), time window columns, custom period columns, and counter expires_at. Introduces TimeOfDay value type and new error constants.

Changes

Migrations:

  • 000006: Add WEEKLY, CUSTOM, PER_TRANSACTION enum values to limit_type
  • 000007: Add time window (active_time_start, active_time_end) and custom period (custom_start_date, custom_end_date) columns with constraints
  • 000008: Add expires_at column to usage_counters

Model:

  • TimeOfDay value object with JSON serialization, validation via time.Parse("15:04"), and comparison methods
  • Error constants TRC-0300 to TRC-0312 for limit validation errors
  • Limit type constants (LimitTypeWeekly, LimitTypeCustom, LimitTypePERTransaction)
  • FixedClock in pkg/clock for MOCK_TIME test infrastructure

Migration Rollback

All migrations have corresponding down.sql files:

  • 000006: RAISE NOTICE (enum values cannot be dropped if in use)
  • 000007: Drops nullable columns safely (no data loss)
  • 000008: Drops expires_at column

Notes

  • All new DB columns are nullable -- zero impact on existing rows
  • Part of the limits-by-period feature (PR 1 of 6)
  • Next: PR 2 (Limit Model -- WEEKLY, CUSTOM, Time Window)

@alexgarzao alexgarzao requested a review from a team as a code owner March 13, 2026 22:29
@coderabbitai
Copy link

coderabbitai bot commented Mar 13, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds DB migrations: appends WEEKLY and CUSTOM to limit_type_enum (UP) and provides a DOWN script that emits a NOTICE explaining removal must be manual; adds active_time_start/active_time_end and custom_start_date/custom_end_date to limits with multiple CHECK constraints and a partial index; adds expires_at to usage_counters with a partial index. Adds FixedClock for deterministic testing, 11 new limit-related error variables, three new constants (CounterRetentionDays, MaxMetadataEntries, MaxMetadataKeyLength), a TimeOfDay value object with parsing/comparison/JSON support, and tests for TimeOfDay and the extended error constants.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Migrator as "Migration Runner"
participant Postgres as "PostgreSQL"
participant Dev as "Developer / Operator"

Migrator->>Postgres: Run 000006_add_limit_type_enum_values.up.sql\nALTER TYPE ... ADD VALUE IF NOT EXISTS 'WEEKLY','CUSTOM'
Postgres-->>Migrator: Return success

Migrator->>Postgres: Run 000007_add_limit_period_columns.up.sql\nALTER TABLE limits ADD columns, constraints, partial index
Postgres-->>Migrator: Return success

Migrator->>Postgres: Run 000008_add_counter_expires_at.up.sql\nALTER TABLE usage_counters ADD expires_at, partial index
Postgres-->>Migrator: Return success

Note over Migrator,Postgres: On rollback of enum change:
Migrator->>Postgres: Run 000006_add_limit_type_enum_values.down.sql
Postgres-->>Migrator: NOTICE: "Enum values WEEKLY and CUSTOM cannot be automatically removed from limit_type_enum"
Migrator->>Dev: Emit NOTICE and manual removal instructions (create new enum, update tables, drop/rename)

🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is well-structured with clear sections for migrations, model changes, and rollback notes. However, it does not follow the provided template structure (missing pull request type checkbox, checklist items, etc.). Update the description to follow the repository template: include PR Type selection, complete the checklist items (testing, documentation, code standards, security, version), and maintain the structured format required.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: database migrations and model foundation for limits by period' clearly and concisely describes the main changes: database schema updates and model code foundations for a new limits feature.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

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

Tip

CodeRabbit can generate a title for your PR based on the changes.

Add @coderabbitai placeholder anywhere in the title of your PR and CodeRabbit will replace it with a title based on the changes in the PR. You can change the placeholder by changing the reviews.auto_title_placeholder setting.

lerian-studio-midaz-push-bot[bot]

This comment was marked as resolved.

@lerian-studio
Copy link

Consider updating CHANGELOG.md to document this change. If this change doesn't need a changelog entry, add the skip-changelog label.

@lerian-studio
Copy link

This PR is very large (303 files, 46408 lines changed). Consider breaking it into smaller PRs for easier review.

@github-actions
Copy link

github-actions bot commented Mar 13, 2026

🔒 Security Scan Results — tracer

Filesystem Scan

✅ No vulnerabilities or secrets found.

Docker Image Scan

✅ No vulnerabilities found.

All security checks passed.

@blacksmith-sh

This comment has been minimized.

@alexgarzao alexgarzao force-pushed the feature/limits-by-period-pr1-migrations-model-foundation branch from 6520921 to 3fb9861 Compare March 13, 2026 22:46
coderabbitai[bot]

This comment was marked as resolved.

@lerian-studio
Copy link

lerian-studio commented Mar 13, 2026

📊 Unit Test Coverage Report: tracer

Metric Value
Overall Coverage 83.8% ⚠️ BELOW THRESHOLD
Threshold 85%

Coverage by Package

Package Coverage
tracer/internal/adapters/cel 81.9%
tracer/internal/adapters/http/in/middleware 62.0%
tracer/internal/adapters/http/in 81.8%
tracer/internal/adapters/postgres/db 0.0%
tracer/internal/adapters/postgres 75.1%
tracer/internal/services/cache 95.6%
tracer/internal/services/command 81.6%
tracer/internal/services/query 78.3%
tracer/internal/services/workers 79.7%
tracer/internal/services 40.2%
tracer/pkg/clock 50.0%
tracer/pkg/contextutil 100.0%
tracer/pkg/logging 100.0%
tracer/pkg/migration 89.0%
tracer/pkg/model 97.5%
tracer/pkg/net/http 88.3%
tracer/pkg/resilience 97.8%
tracer/pkg/sanitize 87.1%
tracer/pkg/validation 50.0%
tracer/pkg 96.6%

Generated by Go PR Analysis workflow

Add schema changes for WEEKLY, CUSTOM, PER_TRANSACTION limit types:
- Migration 000006: add WEEKLY, CUSTOM, PER_TRANSACTION enum values
- Migration 000007: add time window and custom period columns
- Migration 000008: add counter expires_at column
- TimeOfDay value object with JSON serialization and validation
- Error constants TRC-0300 to TRC-0312 for new limit validations
- FixedClock for MOCK_TIME support in test infrastructure
- Limit type constants (LimitTypeWeekly, LimitTypeCustom, etc.)
@alexgarzao alexgarzao force-pushed the feature/limits-by-period-pr1-migrations-model-foundation branch from 3fb9861 to ddc7d1c Compare March 13, 2026 22:53
gandalf-at-lerian

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

- Remove ErrTimeOfDayInvalidHour/ErrTimeOfDayInvalidMinute (unused after
  NewTimeOfDay simplification with time.Parse)
- Reorganize error codes: TRC-0308~0310 (no gaps in range)
- Fix FixedClock.NewTicker stop to be a no-op (matches time.Ticker.Stop
  which does not close its channel)
- Rename TestErrorConstants_DescriptiveMessages to NonEmptyMessages
- Update NewTimeOfDay docstring to reflect single-digit hour acceptance
coderabbitai[bot]

This comment was marked as resolved.

Add ErrTimeOfDayInvalidFormat, ErrLimitUnknownType, ErrLimitCustomPeriodExpired,
ErrLimitInvalidCustomStartFormat, ErrLimitInvalidCustomEndFormat to cover all 11
TRC-0300 range constants.
Copy link

@gandalf-at-lerian gandalf-at-lerian left a comment

Choose a reason for hiding this comment

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

Both points addressed — docstring clarifies the single-digit hour → zero-padded String() contract, and FixedClock.NewTicker stop is a clean no-op now. LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants