Update Feedback Unit ID field lookup to show saved responses#969
Conversation
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UnitFeedbackComponent
participant API
participant AirtableDB
User->>UnitFeedbackComponent: Open Unit Feedback UI
UnitFeedbackComponent->>API: GET existing feedback
API->>AirtableDB: Query feedback by unit
AirtableDB-->>API: Return feedback data
API-->>UnitFeedbackComponent: Return feedback data
User->>UnitFeedbackComponent: Submit feedback
UnitFeedbackComponent->>UnitFeedbackComponent: Set isSubmitting = true
UnitFeedbackComponent->>API: PUT feedback (with unit field)
API->>AirtableDB: Upsert feedback with unit field
AirtableDB-->>API: Confirm save
API-->>UnitFeedbackComponent: Return result
UnitFeedbackComponent->>UnitFeedbackComponent: Set isSubmitting = false
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Updates the feedback system to use lookup fields for unit references instead of direct record links, while adding loading states for better UX during submission.
- Inconsistency in
apps/website/src/pages/api/courses/[courseSlug]/[unitId]/feedback.tsbetween scan operation using 'unitId' and insert using 'unit' field could cause feedback retrieval issues - Added submission states in
apps/website/src/components/courses/UnitFeedback.tsxwith disabled button and 'Sending...' text - Removed 'userFullName' field and added new 'unit' field in
apps/website/src/lib/api/db/tables.tsfor lookup-based unit references
3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/website/src/components/courses/UnitFeedback.tsx(4 hunks)apps/website/src/lib/api/db/tables.ts(2 hunks)apps/website/src/pages/api/courses/[courseSlug]/[unitId]/feedback.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/website/src/components/courses/UnitFeedback.tsx (1)
apps/website/src/pages/api/courses/[courseSlug]/[unitId]/feedback.ts (1)
GetUnitFeedbackResponse(11-14)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci
🔇 Additional comments (8)
apps/website/src/lib/api/db/tables.ts (3)
134-135: LGTM! Field mapping updates align with data model changes.The field mappings correctly reflect the new
unitandunitIdfield distinction. The changes appear to repurpose existing Airtable fields for the new lookup functionality.
143-144: Verify schema consistency with field mappings.The schema correctly includes both
unitandunitIdas string fields, matching the type definition and field mappings.
120-120:Details
✅ Verification successful
Verify the purpose and usage of the new
unitfield.The addition of the
unit: stringfield alongside the existingunitId: stringsuggests different purposes for these fields. Based on the PR objectives,unitappears to be a lookup field displaying the Unit ID, whileunitIdmight be the actual linked record ID.Ensure this dual-field approach is used consistently across the codebase:
🏁 Script executed:
#!/bin/bash # Description: Verify how unit and unitId fields are used in the codebase # Expected: Consistent usage patterns for both fields # Search for usage of both unit and unitId fields in TypeScript files echo "=== Searching for 'unit:' field usage ===" rg -A 2 -B 2 "unit:" --type ts echo -e "\n=== Searching for 'unitId:' field usage ===" rg -A 2 -B 2 "unitId:" --type ts echo -e "\n=== Searching for UnitFeedback type usage ===" rg -A 5 "UnitFeedback" --type tsLength of output: 23677
🏁 Script executed:
#!/bin/bash # Verify db.update usage for UnitFeedback echo "=== Looking for db.update(unitFeedbackTable) usage ===" rg -n "db.update(unitFeedbackTable" -A 10 -B 5 apps/website/src/pages/api/coursesLength of output: 290
🏁 Script executed:
#!/bin/bash # Verify insert and update calls for UnitFeedback echo "=== Searching for db.insert(unitFeedbackTable) usage ===" rg -F "db.insert(unitFeedbackTable" -n apps/website/src echo -e "\n=== Searching for db.update(unitFeedbackTable) usage ===" rg -F "db.update(unitFeedbackTable" -n apps/website/srcLength of output: 639
🏁 Script executed:
#!/bin/bash # Get context around insert and update calls for UnitFeedback to verify both fields echo "=== Context for db.insert(unitFeedbackTable) ===" rg -A5 -B5 "db.insert(unitFeedbackTable" -n apps/website/src/pages/api/courses echo -e "\n=== Context for db.update(unitFeedbackTable) ===" rg -A5 -B5 "db.update(unitFeedbackTable" -n apps/website/src/pages/api/coursesLength of output: 557
Dual-field usage confirmed as consistent
- In the API route (
feedback.ts), only theunitfield is explicitly set (as the linked-record key); theunitIdlookup field is auto‐populated by Airtable.- Front-end components and tests reference only the
unitproperty (or the returnedUnitFeedback.overallRating/anythingElse), never theunitIdlookup field.- The
UnitFeedbacktype and table mappings correctly include both fields for schema completeness; no code references tounitIdneed updating.All usages align with the intended distinction—no further changes required.
apps/website/src/components/courses/UnitFeedback.tsx (5)
23-23: LGTM! Proper state management for submission tracking.The addition of
isSubmittingstate provides necessary control for preventing duplicate submissions and improving user experience.
27-27: Good practice: Extracting loading state for comprehensive button control.Destructuring the
loadingstate from the useAxios hook allows for proper button state management during both initial data fetching and form submission.
44-44: LGTM! Proper async operation state management.Setting
isSubmittingto true before the API call prevents multiple concurrent submissions and provides immediate user feedback.
63-65: Excellent: Proper cleanup in finally block.Using the finally block ensures
isSubmittingis reset regardless of success or failure, preventing the UI from getting stuck in a submitting state.
116-118: Perfect UX implementation for submit button state.The button correctly:
- Disables during both loading (initial fetch) and submitting states
- Provides clear visual feedback with "Sending..." text
- Prevents user confusion and duplicate submissions
| // If the feedback does NOT exist, create it | ||
| upsertedFeedback = await db.insert(unitFeedbackTable, { | ||
| unitId, | ||
| unit: unitId, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify consistency between GET filtering and PUT field assignment.
The PUT operation now assigns the unitId value to the unit field, but the GET operation (line 52) still filters by the unitId field. This inconsistency could lead to issues where newly created feedback records cannot be retrieved by subsequent GET requests.
Please verify this is intentional behavior. If both fields should contain the same value, consider updating the creation logic:
upsertedFeedback = await db.insert(unitFeedbackTable, {
unit: unitId,
+ unitId: unitId,
createdAt: new Date().toISOString(),
lastModified: new Date().toISOString(),
userEmail: auth.email,
overallRating,
anythingElse,
});Or update the GET filter to use the unit field instead.
Run this script to check for existing feedback records and their field values:
🏁 Script executed:
#!/bin/bash
# Description: Check how existing feedback records use unit vs unitId fields
# Expected: Understanding of field usage patterns
# Look for other API endpoints that interact with UnitFeedback
echo "=== Searching for UnitFeedback API usage ==="
rg -A 5 -B 5 "unitFeedbackTable" --type ts
echo -e "\n=== Searching for feedback filtering patterns ==="
rg -A 3 -B 3 "filterByFormula.*unit" --type tsLength of output: 8812
🏁 Script executed:
#!/bin/bash
echo "=== export type UnitFeedback ==="
rg -C 5 "export type UnitFeedback" -n apps/website/src/lib/api/db/tables.ts
echo -e "\n=== unitFeedbackTable mapping ==="
rg -C 10 "export const unitFeedbackTable" -n apps/website/src/lib/api/db/tables.ts
echo -e "\n=== unitFeedbackTable mappings block ==="
sed -n '/export const unitFeedbackTable/,/};/p' apps/website/src/lib/api/db/tables.ts | sed -n '/mappings:/,/}/p'
echo -e "\n=== Search for 'unitId' references in tables.ts ==="
rg -n "unitId" apps/website/src/lib/api/db/tables.tsLength of output: 2208
Missing unitId in feedback insertion
The UnitFeedback type and Airtable mapping include both unit and unitId, but the insert call only sets unit. Since the GET scan filters by unitId, newly created feedback won’t be found.
• File: apps/website/src/pages/api/courses/[courseSlug]/[unitId]/feedback.ts (else branch around line 85)
Suggested fix:
upsertedFeedback = await db.insert(unitFeedbackTable, {
unit: unitId,
+ unitId: unitId,
createdAt: new Date().toISOString(),
lastModified: new Date().toISOString(),
userEmail: auth.email,
overallRating,
anythingElse,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| unit: unitId, | |
| upsertedFeedback = await db.insert(unitFeedbackTable, { | |
| unit: unitId, | |
| unitId: unitId, | |
| createdAt: new Date().toISOString(), | |
| lastModified: new Date().toISOString(), | |
| userEmail: auth.email, | |
| overallRating, | |
| anythingElse, | |
| }); |
🤖 Prompt for AI Agents
In apps/website/src/pages/api/courses/[courseSlug]/[unitId]/feedback.ts at line
85, the PUT operation assigns the unitId value only to the 'unit' field but the
GET operation filters by 'unitId'. To fix this inconsistency, update the
feedback insertion logic to include both 'unit' and 'unitId' fields with the
unitId value. This ensures that newly created feedback records can be retrieved
correctly by the GET requests filtering on 'unitId'.
1 participant
Description
Issue
https://bluedotimpact.slack.com/archives/C08DC9SSA66/p1748294763877639
Developer checklist
Screenshot
Screen.Recording.2025-05-27.at.15.28.19.mov
Summary by CodeRabbit
New Features
Bug Fixes
Refactor