fix: api to get OOO req and ack OOO req #2445
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Summary by CodeRabbit
WalkthroughThis update introduces additional logging across several controllers and models, refines data handling for out-of-office (OOO) requests by updating field names and response mapping, modifies validation schemas, and adds a utility function for OOO data transformation. Some fallback logic for GitHub authentication is commented out, and compatibility adjustments are made for legacy data formats. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Model
participant Utils
Client->>Controller: Update OOO Request
Controller->>Model: getRequests({ type: 'OOO', ... })
Model->>Model: Fetch OOO requests from DB
Model->>Utils: mapOOOResponseData(requests)
Utils-->>Model: Mapped legacy format data
Model-->>Controller: Return mapped data
Controller-->>Client: Respond with data
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 (
|
coderabbitai
bot
added
backend
bug
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Missing Email Verification in GitHub Auth ▹ view | ✅ Fix detected |
 |
Missing GitHub Email Fallback ▹ view | ✅ Fix detected |
 |
Data Transform Logic Violates SRP and OCP ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| utils/mapOOOResponseData.ts | ✅ |
| models/userFutureStatus.ts | ✅ |
| controllers/requests.ts | ✅ |
| middlewares/validators/requests.ts | ✅ |
| models/requests.ts | ✅ |
| controllers/oooRequests.ts | ✅ |
| controllers/auth.js | ✅ |
| models/userStatus.js | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (1)
middlewares/validators/requests.ts (1)
61-71: Clarify schema design with both 'state' and 'status' fields.The schema now includes both
state(required) andstatus(optional) fields with the same validation rules. This could lead to confusion about which field should be used by API consumers. Consider:
- Adding clear documentation about when to use each field
- Planning a migration path to consolidate to a single field
- Adding validation to ensure both fields don't conflict when provided
Consider adding a custom validation to prevent conflicting values:
schema = schema.custom((value, helpers) => { if (value.state && value.status && value.state !== value.status) { return helpers.error('custom.conflictingFields'); } return value; }).messages({ 'custom.conflictingFields': 'state and status fields cannot have different values' });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
controllers/auth.js(1 hunks)controllers/oooRequests.ts(3 hunks)controllers/requests.ts(1 hunks)middlewares/validators/requests.ts(1 hunks)models/requests.ts(3 hunks)models/userFutureStatus.ts(1 hunks)models/userStatus.js(2 hunks)utils/mapOOOResponseData.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: build (22.10.0)
🔇 Additional comments (6)
controllers/auth.js (1)
175-188: Let’s locate the auth controller and verify whether that fallback block still exists under a different path:#!/bin/bash # 1. Find any file named auth.js echo "=== Finding auth.js ===" find . -type f -name auth.js # 2. If found, dump lines where GitHub email fallback would live (around line 175) echo echo "=== Searching for GitHub email fallback references ===" grep -R "githubApi.baseUrl" -n . echo grep -R "/user/emails" -n .controllers/oooRequests.ts (2)
123-123: Good addition of type filter for specificity.Adding the type filter
REQUEST_TYPE.OOOmakes the query more specific and prevents potential data inconsistencies.
126-126: ```shell
#!/bin/bashLocate and inspect the destructuring in controllers/oooRequests.ts
echo "Finding requestData usage in controllers/oooRequests.ts..."
rg -n "requestData" controllers/oooRequests.tsShow context around the destructuring (20 lines before and after)
LINE=$(rg -n "const { from, until, userId, reason }" controllers/oooRequests.ts | cut -d: -f1)
sed -n "$((LINE-10)),$((LINE+10))p" controllers/oooRequests.ts</details> <details> <summary>models/requests.ts (3)</summary> `3-3`: **Approve new imports.** The addition of `REQUEST_TYPE` constant, `mapOOOResponseData` utility, and logger are appropriate for the new functionality. Also applies to: 11-12 --- `154-157`: **Good backward compatibility handling.** The conditional transformation using `mapOOOResponseData` when `dev` flag is false provides good backward compatibility for the dashboard site expecting the old data format. The comment clearly explains the reasoning, and the implementation properly handles the data format transformation based on the request type and dev flag. --- `42-42`: ```shell #!/bin/bash # Locate controllers/oooRequests.ts and print around createRequest usage FILE=$(fd -e ts "controllers/oooRequests.ts" | head -1) if [ -n "$FILE" ]; then grep -R -n "createRequest" -n "$FILE" | head -5 echo "------- Context --------" sed -n '1,200p' "$FILE" else echo "controllers/oooRequests.ts not found" fi
tejaskh3
added
the
hacktoberfest
Date:
Developer Name:
Issue Ticket Number
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Test Coverage
Screenshot 1
Additional Notes
Description by Korbit AI
What change is being made?
Update and fix the API for handling Out-Of-Office (OOO) requests, including changes in handling data structures and logging.
Why are these changes being made?
These changes address issues with the existing OOO request handling by improving the mapping of response data to align with expected formats, updating the logging for better traceability, and correcting function parameters and data attributes to align with expected API behavior. Additionally, deprecated or non-functional code related to email retrieval from GitHub has been commented out for potential refactoring or removal in the future.