Skip to content

Fix redirect correctness and analytics tracking - #48

Merged
AlexKitipov merged 1 commit into
mainfrom
codex/fix-redirect-logic-and-improve-analytics
Jun 9, 2026
Merged

Fix redirect correctness and analytics tracking#48
AlexKitipov merged 1 commit into
mainfrom
codex/fix-redirect-logic-and-improve-analytics

Conversation

@AlexKitipov

Copy link
Copy Markdown
Owner

Motivation

  • Make short-link redirects deterministic and ensure analytics are recorded only for valid, non-expired, active links.
  • Prevent client/browser caching from skipping analytics by avoiding permanent 301 redirects.
  • Persist and index expires_at so expiration checks are reliable during redirect handling.

Description

  • Split redirect logic into focused helpers: _get_redirectable_link, _enforce_click_limit, and _record_click in app/api/v1/redirect.py.
  • Enforce expires_at and is_active before recording analytics and enforce free-tier click limits prior to writes.
  • Switch redirect responses from 301 to a temporary 302 via REDIRECT_STATUS_CODE to avoid cache bypassing analytics.
  • Persist expires_at when creating links in app/api/v1/links.py and add an index on expires_at in app/models/link.py.
  • Add tests/test_redirect.py with coverage for analytics insertion, temporary redirect status, free click limit blocking, inactive links, and expired links.

Testing

  • Ran pytest -q and all tests passed.
  • Ran flake8 on modified files and there were no lint failures.
  • Verified git diff --check produced no whitespace/errors related issues.

Codex Task

@AlexKitipov
AlexKitipov merged commit 4f6f29e into main Jun 9, 2026
2 checks passed

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e5655f401

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/models/link.py
nullable=False,
)
expires_at = Column(DateTime, nullable=True)
expires_at = Column(DateTime, nullable=True, index=True)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Add the missing Alembic migration for the new index

This model-only index does not get applied to existing Alembic-managed databases: I checked migrations/versions and the only links.expires_at definition is in 0001_initial_schema.py, which creates the column without an ix_links_expires_at index, and there is no later migration for it. In deployments that use alembic upgrade, the database schema will drift from the model and expiration-related queries/maintenance will still lack the intended index; please add a migration that creates/drops the links.expires_at index.

Useful? React with 👍 / 👎.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant