Add onboarding extension requests data model and examples in different state #74
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 pull request introduces a README file for the onboarding extension request Firestore collection model. The document defines a JSON schema that outlines the structure for the collection, including fields such as Changes
Assessment against linked issues
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
onboarding-extension-requests/README.md (1)
42-101: 💡 Verification agent🧩 Analysis chain
Confirm JSON Example Consistency
The examples provided for the PENDING, APPROVED, and REJECTED states are clear and useful. However, note that these examples use"requestNumber"while the JSON schema defines the field as"requestedNumber". Please verify and align the field name across all sections of the document to ensure consistency.
Based on the verification, we found that the JSON schema uses
"requestedNumber"whereas the examples in the documentation use"requestNumber". This discrepancy should be resolved to maintain consistency across the codebase and documentation.Align JSON Field Naming for Consistency
- In
onboarding-extension-requests/README.md(lines 42-101), update the examples to use"requestedNumber"instead of"requestNumber"as defined by the JSON schema.- This change ensures consistency and reduces potential confusion during schema validation or integration testing.
🏁 Script executed:
#!/bin/bash # Check for occurrences of "requestedNumber" in the repository to verify the JSON schema definition. rg -n '"requestedNumber"' . # Check for occurrences of "requestNumber" in the repository, particularly in documentation or examples. rg -n '"requestNumber"' .Length of output: 332
Align JSON Field Naming for Consistency
We've confirmed a naming mismatch between the JSON schema and the examples. The schema defines the field as"requestedNumber"(e.g., inonboarding-extension-requests/README.mdline 18), while the JSON examples for the PENDING, APPROVED, and REJECTED states use"requestNumber". Please update the examples in the README file (lines 42–101) to use"requestedNumber"to match the schema definition.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
onboarding-extension-requests/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
onboarding-extension-requests/README.md
[grammar] ~31-~31: This is normally spelled as one word.
Context: ... | String | The message provided by super user while approving or rejecting request.| ...
(SUPER_COMPOUNDS)
[typographical] ~38-~38: Consider putting a comma before the abbreviation “i.e.”.
Context: ...type | String | The type of request i.e. ONBOARDING. ...
(IE_COMMA)
🪛 markdownlint-cli2 (0.17.2)
onboarding-extension-requests/README.md
2-2: First line in a file should be a top-level heading
null
(MD041, first-line-heading, first-line-h1)
28-28: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
31-31: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
40-40: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
41-41: Multiple consecutive blank lines
Expected: 1; Actual: 3
(MD012, no-multiple-blanks)
🔇 Additional comments (1)
onboarding-extension-requests/README.md (1)
17-18:Details
✅ Verification successful
Resolve Field Naming Inconsistency
There is an inconsistency between the JSON schema and the Fields table: the schema (line 18) uses"requestedNumber", whereas the Fields table (line 37) refers to it as"requestNumber". Please standardize the field name across the document to avoid confusion.Also applies to: 37-38
#!/bin/bash # Check occurrences of "requestedNumber" and "requestNumber" in the README.md file echo "Occurrences of 'requestedNumber' in onboarding-extension-requests/README.md:" rg "requestedNumber" -n onboarding-extension-requests/README.md echo "" echo "Occurrences of 'requestNumber' in onboarding-extension-requests/README.md:" rg "requestNumber" -n onboarding-extension-requests/README.mdCould you please run the above script to confirm that the JSON schema (around lines 17-18) uses
"requestedNumber"while the Fields table (around lines 37-38) refers to"requestNumber"? Once verified, the review comment can be rewritten as follows:Field Naming Inconsistency – Standardize Field Name
The JSON schema (line 18) declares"requestedNumber", whereas the Fields table (line 37) uses"requestNumber". To avoid confusion, please update the document so that the field name is consistent throughout (e.g., update the Fields table to use"requestedNumber").
|
|
||
| ### Onboarding Extension Request Firestore collection model | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider Adding a Top-Level Heading
While the file begins with a level-3 heading, markdown best practices (and tools such as markdownlint MD041) suggest that the first line should be a top-level (level-1) heading. For better structure and readability, consider replacing the current heading with a level-1 heading (e.g., # Onboarding Extension Request Firestore Collection Model).
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
2-2: First line in a file should be a top-level heading
null
(MD041, first-line-heading, first-line-h1)
| | newEndsOn | number | New deadline of the onboarding task in millisecond | | ||
| | message | String | The message provided by super user while approving or rejecting request.| | ||
| | createdAt | Timestamp | Unix timestamp for the creation time of the request. | |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Standardize Terminology: "Superuser" Instead of "Super user"
In the Fields table (line 31), the description reads "The message provided by super user while approving or rejecting request." For consistency and clarity, consider combining it into a single word: "superuser".
🧰 Tools
🪛 LanguageTool
[grammar] ~31-~31: This is normally spelled as one word.
Context: ... | String | The message provided by super user while approving or rejecting request.| ...
(SUPER_COMPOUNDS)
🪛 markdownlint-cli2 (0.17.2)
31-31: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
| | reason | String | The reason for the extension request. | | ||
| | requestedBy | String | The username of the user who created the request. | | ||
| | requestNumber | number | The current request number for onboarding task. | | ||
| | type | String | The type of request i.e. ONBOARDING. | |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enhance Punctuation in Field Description
In the description for the type field (line 38), consider adding commas around "i.e." for clarity. For example, use: "The type of request, i.e., ONBOARDING."
🧰 Tools
🪛 LanguageTool
[typographical] ~38-~38: Consider putting a comma before the abbreviation “i.e.”.
Context: ...type | String | The type of request i.e. ONBOARDING. ...
(IE_COMMA)
|
|
||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Clean Up Extra Blank Lines
There are multiple consecutive blank lines at lines 40–41. Removing the extra blank lines would improve the overall document formatting and readability.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
40-40: Multiple consecutive blank lines
Expected: 1; Actual: 2
(MD012, no-multiple-blanks)
41-41: Multiple consecutive blank lines
Expected: 1; Actual: 3
(MD012, no-multiple-blanks)
| ```json | ||
| { | ||
| "id": "String", | ||
| "type": "String" | ||
| "userId": "String", |
There was a problem hiding this comment.
Fix JSON Syntax: Missing Comma After "type" Field
In the JSON schema block, the "type" field on line 7 is missing a trailing comma. This omission makes the JSON example invalid. Please add a comma after the "type": "String" entry.
- "type": "String"
+ "type": "String",📝 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.
| ```json | |
| { | |
| "id": "String", | |
| "type": "String" | |
| "userId": "String", |
iamitprakash
left a comment
iamitprakash
left a comment
There was a problem hiding this comment.
I need to understand few more things, would love to talk in VC before approving this
Date: 29 March, 2025
Developer Name: @pankajjs
Issue Ticket Number
Description
This PR adds data model and examples of onboarding extension request in different state such as PENDING, APPROVED and REJECTED in readme file.
Doc- https://docs.google.com/document/d/17LAwNt_LijCR9HY5mHim5tGV70OkMkTETZEJ73n1q4w/edit?pli=1&tab=t.0
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
Test Coverage
Screenshot 1
Additional Notes