Skip to content

Conversation

@devksingh4
Copy link
Member

@devksingh4 devksingh4 commented Nov 11, 2025

Summary by CodeRabbit

  • New Features

    • Organizations can create, list, and delete org-scoped short links with authorization, audit logging, and optimistic concurrency control.
    • Subdomain-based routing maps org subdomains to their redirects.
    • CDN now accepts wildcard subdomains and QA adds wildcard DNS records.
  • Tests

    • Added an org-scoped health-check test; one purchases test commented out.
  • Breaking Changes

    • Public create request schema no longer accepts an orgId field.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 11, 2025

Walkthrough

Adds organization-scoped link support: new org API routes (create/get/delete) with authorization, concurrency checks, transactional deletes and audit logging; new fetchOrgRecords helper and Org types; host-based subdomain→org slug resolution in the edge handler; CloudFront alias and Route53 wildcard updates; edge package dependency added.

Changes

Cohort / File(s) Summary
Organization Link API Functions
src/api/functions/linkry.ts
Adds fetchOrgRecords(orgId: string, tableName: string, dynamoClient: DynamoDBClient): Promise<OrgLinkRecord[]> to query the AccessIndex for OWNER#<orgId> items, unmarshall results, strip OWNER#/GROUP# prefixes from access, and return OrgLinkRecord[]. Import updated to include OrgLinkRecord.
Organization Link Routes
src/api/routes/linkry.ts
Adds org-scoped endpoints: POST /orgs/:orgId/redir (create/modify with preValidation, conditional PutItem, audit log), GET /orgs/:orgId/redir (list org links with auth), DELETE /orgs/:orgId/redir/:slug (transactional delete of owner and related group records, concurrency handling, audit log). Adds org-aware types, auth checks, and ConditionalCheckFailedException handling.
Types / Schema Updates
src/common/types/linkry.ts
Removes orgId from createRequest, adds createOrgLinkRequest = createRequest.omit({ access: true }), defines orgLinkRecord schema and OrgLinkRecord type, and exports org-related types.
Edge Handler — Host-based Slug Resolution
src/linkryEdgeFunction/index.ts
Adds BASE_DOMAINS, builds subdomain→org-code mapping from Organizations, implements getSlugToQuery(path, host) to map requests on recognized subdomains to OWNER#<org> slugs, and uses slugToQuery for DynamoDB lookups and logging.
Edge Function Dependencies / Package
src/linkryEdgeFunction/package.json
Adds dependency @acm-uiuc/js-shared (^3.2.1), reorders devDependencies (typescript, esbuild), and normalizes EOF newline.
Terraform — CloudFront Aliases
terraform/modules/frontend/main.tf
Updates aws_cloudfront_distribution.linkry_cloudfront_distribution aliases to include wildcard subdomains by concatenating var.LinkryPublicDomains with *.<domain> entries.
Terraform — QA Route53 Wildcard Record
terraform/envs/qa/main.tf
Adds aws_route53_record.linkry_wildcard (for_each over ["A","AAAA"]) creating *.${var.LinkryPublicDomain} alias records pointing to the frontend CloudFront domain.
Terraform — QA Certificate ARN
terraform/envs/qa/variables.tf
Updates default value of LinkryCertificateArn variable to a new ACM certificate ARN.
Terraform — Prod Certificate ARN
terraform/envs/prod/variables.tf
Updates default value of LinkryCertificateArn variable to a new ACM certificate ARN.
Tests — Live
tests/live/linkry.test.ts, tests/live/tickets.test.ts, tests/live/utils.ts
Adds a new infra health-check test and infra.go service value; comments out a purchases test (TODO).

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client
    participant API as API Route
    participant Auth as Authorization
    participant Dynamo as DynamoDB
    participant Log as Audit Log

    rect rgb(220,240,255)
    Note over Client,Log: POST /orgs/:orgId/redir (Create/Modify Org Link)
    end

    Client->>API: POST /orgs/:orgId/redir with slug, redirect
    API->>Auth: Verify org authorization
    Auth-->>API: Authorized / Not Authorized
    alt Authorized
        API->>Dynamo: PutItem with conditional expression (slug conflict check)
        alt Condition succeeds
            Dynamo-->>API: Success
            API->>Log: Write audit entry (CREATE/MODIFY)
            API-->>Client: 201 Created + Location
        else ConditionalCheckFailed
            Dynamo-->>API: ConditionalCheckFailedException
            API-->>Client: 409 Conflict
        end
    else Not Authorized
        API-->>Client: 403 Forbidden
    end
Loading
sequenceDiagram
    participant Client as Client
    participant API as API Route
    participant Auth as Authorization
    participant Dynamo as DynamoDB
    participant Log as Audit Log

    rect rgb(240,220,255)
    Note over Client,Log: DELETE /orgs/:orgId/redir/:slug (Delete Org Link)
    end

    Client->>API: DELETE /orgs/:orgId/redir/:slug
    API->>Auth: Verify org authorization
    Auth-->>API: Authorized / Not Authorized
    alt Authorized
        API->>Dynamo: TransactWriteItems (delete owner record + related group records)
        Dynamo-->>API: Transaction result or ConditionalCheckFailed
        API->>Log: Write audit entry (DELETE)
        API-->>Client: 204 No Content
    else Not Authorized
        API-->>Client: 403 Forbidden
    end
Loading
sequenceDiagram
    participant Request as Incoming Request
    participant Handler as Edge Handler
    participant DomainMap as Domain Mapper
    participant Dynamo as DynamoDB

    rect rgb(220,240,255)
    Note over Request,Dynamo: Host-based Slug Resolution in Edge Handler
    end

    Request->>Handler: HTTP request (path, Host header)
    Handler->>DomainMap: Normalize host, map subdomain → org code
    alt Recognized subdomain
        DomainMap-->>Handler: Org code
        Handler->>Handler: Transform path into org-scoped slug (OWNER#<org>)
    else Unrecognized
        DomainMap-->>Handler: No mapping
        Handler->>Handler: Use path as slug
    end
    Handler->>Dynamo: Query AccessIndex with computed slug
    Dynamo-->>Handler: Return link record(s)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45–60 minutes

Key areas requiring extra attention:

  • src/api/routes/linkry.ts — new org routes, conditional PutItem logic, TransactWriteItems, audit logging, and authorization checks.
  • src/api/functions/linkry.tsfetchOrgRecords unmarshalling and access prefix stripping.
  • src/linkryEdgeFunction/index.ts — host normalization and subdomain→org mapping; ensure host edge cases and slug construction.
  • src/common/types/linkry.ts — schema change removing orgId and new org-specific types.
  • Terraform changes affecting CloudFront aliases and QA Route53 wildcard records (verify wildcard behavior and zone IDs).

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Setup backend for org-managed short links' accurately captures the main purpose of this PR, which implements backend infrastructure for organization-scoped link management across multiple files.
✨ 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 dsingh14/org-managed-links

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 and usage tips.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 11, 2025

💰 Infracost report

Monthly estimate increased by $1 📈

Changed project Baseline cost Usage cost* Total change New monthly cost
acm-uiuc/core/terraform/envs/qa +$0 +$1 +$1 (+1%) $87

*Usage costs were estimated using infracost-usage.yml, see docs for other options.

Estimate details
Key: * usage cost, ~ changed, + added, - removed

──────────────────────────────────
Project: envs-qa
Module path: envs/qa

+ aws_route53_record.linkry_wildcard["A"]
  +$0.50

    + Standard queries (first 1B)
      +$0.50, +1.25 1M queries*

+ aws_route53_record.linkry_wildcard["AAAA"]
  +$0.50

    + Standard queries (first 1B)
      +$0.50, +1.25 1M queries*

Monthly cost change for acm-uiuc/core/terraform/envs/qa (Module path: envs/qa)
Amount:  +$1 ($86 → $87)
Percent: +1%

──────────────────────────────────
Key: * usage cost, ~ changed, + added, - removed
1 project has no cost estimate change.
Run the following command to see its breakdown: infracost breakdown --path=/path/to/code

──────────────────────────────────
*Usage costs were estimated using infracost-usage.yml, see docs for other options.

338 cloud resources were detected:
∙ 132 were estimated
∙ 206 were free

Infracost estimate: Monthly estimate increased by $1 ↑
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━┓
┃ Changed project                                    ┃ Baseline cost ┃ Usage cost* ┃ Total change ┃
┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━╋━━━━━━━━━━━━━━┫
┃ acm-uiuc/core/terraform/envs/qa                    ┃           +$0 ┃         +$1 ┃    +$1 (+1%) ┃
┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━┻━━━━━━━━━━━━━━┛
This comment will be updated when code changes.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 087b45e and 3483d92.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (5)
  • src/api/functions/linkry.ts (2 hunks)
  • src/api/routes/linkry.ts (4 hunks)
  • src/common/types/linkry.ts (1 hunks)
  • src/linkryEdgeFunction/index.ts (4 hunks)
  • src/linkryEdgeFunction/package.json (1 hunks)
🧰 Additional context used
🪛 ESLint
src/api/functions/linkry.ts

[error] 8-8: Unexpected use of file extension "js" for "common/types/linkry.js"

(import/extensions)

src/common/types/linkry.ts

[error] 30-30: Insert ;

(prettier/prettier)

src/api/routes/linkry.ts

[error] 22-22: Unexpected use of file extension "js" for "../../common/config.js"

(import/extensions)


[error] 24-24: Unexpected use of file extension "js" for "api/plugins/rateLimiter.js"

(import/extensions)


[error] 30-30: Unexpected use of file extension "js" for "common/types/linkry.js"

(import/extensions)


[error] 40-40: Unexpected use of file extension "js" for "api/functions/linkry.js"

(import/extensions)


[error] 41-41: Unexpected use of file extension "js" for "api/plugins/auth.js"

(import/extensions)


[error] 45-45: Unexpected use of file extension "js" for "api/functions/auditLog.js"

(import/extensions)


[error] 46-46: Unexpected use of file extension "js" for "common/modules.js"

(import/extensions)


[error] 48-48: Unexpected use of file extension "js" for "api/components/index.js"

(import/extensions)


[error] 50-50: Unexpected use of file extension "js" for "api/functions/authorization.js"

(import/extensions)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
tests/live/linkry.test.ts (1)

14-19: Consider adding tests for org-specific behavior.

While this health check validates basic connectivity to the infra.go endpoint, it doesn't verify any org-scoped functionality mentioned in the PR objectives (e.g., subdomain→org slug resolution, org-specific link records). Consider adding tests that verify:

  • Subdomain-based org routing
  • Creating/retrieving org-scoped links
  • Authorization for org-specific operations
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between 4ea6c84 and b8052e7.

📒 Files selected for processing (3)
  • tests/live/linkry.test.ts (2 hunks)
  • tests/live/tickets.test.ts (1 hunks)
  • tests/live/utils.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build Application
  • GitHub Check: Run Unit Tests
🔇 Additional comments (3)
tests/live/utils.ts (1)

75-75: LGTM!

The addition of "infra.go" to the Service type union is consistent with the existing pattern and properly enables testing of the new infra endpoint.

tests/live/linkry.test.ts (1)

5-5: LGTM!

The constant declaration properly utilizes the new "infra.go" service type to create a base endpoint for org-scoped testing.

tests/live/tickets.test.ts (1)

33-54: Update the TODO comment with clear next step for writing the live test.

The API migration from email-based to UIN-based lookup is already complete and thoroughly tested at the unit level. Comprehensive unit tests exist in tests/unit/tickets.test.ts (10+ test cases covering happy and sad paths for the POST /api/v1/tickets/getPurchasesByUser endpoint).

The TODO just needs a live test written calling the new endpoint with UIN instead of the old email-based GET endpoint. Update the comment to:

// TODO: write live test for POST /api/v1/tickets/getPurchasesByUser endpoint
// Should send { uin: "123456789", productId: "..." } and verify response contains merch and tickets arrays

Likely an incorrect or invalid review comment.

@devksingh4 devksingh4 merged commit 57e8a32 into main Nov 11, 2025
11 of 12 checks passed
@devksingh4 devksingh4 deleted the dsingh14/org-managed-links branch November 11, 2025 03:40
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.

2 participants