Skip to content

Implemented tenants display based on NYU identity API#1273

Open
interfinityOfficial wants to merge 4 commits intomainfrom
867-call-the-nyu-identity-api-for-which-tenants-to-display
Open

Implemented tenants display based on NYU identity API#1273
interfinityOfficial wants to merge 4 commits intomainfrom
867-call-the-nyu-identity-api-for-which-tenants-to-display

Conversation

@interfinityOfficial
Copy link
Collaborator

@interfinityOfficial interfinityOfficial commented Mar 8, 2026

Summary of Changes

Implemented tenants display based on NYU identity API response

Checklist

  • I checked for existing implementations and confirmed there is no duplication
  • I thoroughly tested this feature locally
  • I added or updated unit tests (or explained why not in the PR description)
  • I attached screenshots or a video demonstrating the feature
  • I incorporated Copilot's feedback (or explained why not in the PR description), and marked conversation as resolved
  • I confirmed my PR passed all unit and end-to-end (E2E) tests
  • I confirmed there are no conflicts
  • I requested a code review from at least one other teammate

Screenshots / Video

Screen.Recording.2026-03-07.at.10.51.17.PM.mov

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an NYU Identity–backed “entitled tenants” lookup and updates the app’s root page to render tenant cards based on that entitlement response.

Changes:

  • Introduces /api/nyu/entitlements/[netId] to call NYU Identity API and derive entitled tenants (MC always, plus ITP based on department keywords).
  • Replaces the static / landing page with an entitlement-driven tenant selection UI.
  • Adds unit tests for the new entitlements API route.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
booking-app/app/api/nyu/entitlements/[netId]/route.ts New API route to compute entitled tenants from NYU Identity API response
booking-app/app/page.tsx New client UI that fetches entitlements and displays tenant cards
booking-app/tests/unit/nyu-entitlements-api.unit.test.ts Unit coverage for success/fallback/error cases of the new entitlements route

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +8 to +16
// Keywords matched case-insensitively against reporting_dept_name to identify
// ITP / IMA / Low Res affiliated users. Mirrors the keyword approach in
// components/src/server/admin.ts (itpDeptKeywords).
const ITP_DEPT_NAME_KEYWORDS = [
"interactive telecommunications", // ITP
"interactive media arts", // IMA (e.g. "Interactive Media Arts UG Program")
"low res", // Low Residence program
"low-res",
];
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

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

The comment says this keyword list “mirrors” components/src/server/admin.ts (itpDeptKeywords), but the actual keywords there are different (and itpDeptKeywords is currently defined locally inside a function). To avoid divergence, consider extracting a shared constant (or update the comment to reflect the intentional difference).

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

@interfinityOfficial
Please don't ignore this Copilot's comment. Redefining similar things can lead to missed fixes later on and create bugs.
Use the existing itpDeptKeywords.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rlho
I think the existing keywords don't really work, at least for IMA students, here's what I got from the identity API for my nyu account:

{
"school_abbr": "TSOA",
"school_name": "TSOA",
"reporting_dept_code": "I_MEDIA_ARTS_UGP",
"reporting_dept_name": "Interactive Media Arts UG Program",
"affiliation": "student",
"affiliation_sub_type": "degree",
"dept_code": "UTIMNY",
"preferred_first_name": "Charles",
"preferred_last_name": "Zhang",
"dept_name": null,
"primary_affiliation": "student"
}

which the "ima" keyword doesn't really match any fields. So do you think I should update the existing keyword list as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@interfinityOfficial
Yes! update the existing keywords too and extract them as a shared constant.
"ima" doesn't match "interactive media arts" so it's been broken.
Please fix both in one place.

…, added an error text, updated the unit tests
@interfinityOfficial interfinityOfficial requested a review from rlho March 9, 2026 17:00
import admin from "@/lib/firebase/server/firebaseAdmin";
import { NextRequest, NextResponse } from "next/server";

const NYU_API_BASE = "https://api.nyu.edu/identity-v2-sys";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please use this instead of redefining what's already defined.

const NYU_API_BASE = "https://api.nyu.edu/identity-v2-sys";

Comment on lines +8 to +16
// Keywords matched case-insensitively against reporting_dept_name to identify
// ITP / IMA / Low Res affiliated users. Mirrors the keyword approach in
// components/src/server/admin.ts (itpDeptKeywords).
const ITP_DEPT_NAME_KEYWORDS = [
"interactive telecommunications", // ITP
"interactive media arts", // IMA (e.g. "Interactive Media Arts UG Program")
"low res", // Low Residence program
"low-res",
];
Copy link
Collaborator

Choose a reason for hiding this comment

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

@interfinityOfficial
Please don't ignore this Copilot's comment. Redefining similar things can lead to missed fixes later on and create bugs.
Use the existing itpDeptKeywords.

}

const url = new URL(
`${NYU_API_BASE}/identity/unique-id/primary-affil/${netId}`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's already an API that calls this Identity API, so please use that one without redefining it.

…lidating department keyword handling. Updated identity fetching to use a base URL from environment variables. Improved code organization by exporting constants for department keywords.
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.

As a User I want to call the NYU Identity API to determine which tenants I'm entitled to see on the dashboard.

3 participants