Open
Conversation
📝 WalkthroughWalkthroughA new comprehensive onboarding documentation file is added to guide engineers through the LRDG Portal system. The document covers the product's three-stage mental model, layered .NET architecture, request flow, domain-specific business logic for Contracts, Learner Program, and Booking Features, local development setup, contribution workflow, and token-evaluation guidance for LLM usage. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
onboarding.md (4)
371-382: Document how to obtain team-managed secrets.Line 373 mentions "team-managed secrets are not yet available" but doesn't explain how new engineers should obtain these secrets or who to contact. This could block onboarding.
📋 Suggested enhancement
Add guidance on obtaining secrets:
- Update local configuration values in `PortalMVC/Web.config` and `PortalAPI/Web.config`. Use placeholder values like these where team-managed secrets are not yet available: + Update local configuration values in `PortalMVC/Web.config` and `PortalAPI/Web.config`. Contact your team lead or check the team's password manager for production-like connection strings and API keys. Use placeholder values like these for initial setup:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@onboarding.md` around lines 371 - 382, Update the "Configure local application settings." section (the block showing keys like FeatureFlags:ConfigPath, Redis:ConnectionString, Cosmos:Endpoint, Cosmos:Key, AzureBlob:ConnectionString, Notifications:FromEmail) to include a short, actionable paragraph explaining how to obtain team-managed secrets: list the team or role to contact (e.g., infra/DevOps), the preferred channels (Slack channel name or email), the formal request process (ticketing system or access request form), required approvals or onboarding steps, and any ephemeral/local dev alternatives (e.g., use Azure Storage emulator or development keys) plus a note to rotate secrets after access is granted.
393-395: Strengthen the database updater safety warning.The warning about the database updater is crucial for preventing accidental schema changes to shared databases. Consider making this warning more prominent.
⚠️ Suggested enhancement for safety- Confirm the database contains the required portal schema and the `PortalDBVersion` table. The built-in database updater lives in the portal startup path and applies scripts from `PortalMVC/SQLScriptFix/` when the application starts. Before running the app, verify that this updater points only to a safe local database instance so startup scripts cannot touch a shared environment by mistake. + Confirm the database contains the required portal schema and the `PortalDBVersion` table. + + **⚠️ CRITICAL:** The built-in database updater automatically applies scripts from `PortalMVC/SQLScriptFix/` when the application starts. Before running the app for the first time, double-check that your connection string points ONLY to a local development database. Accidentally pointing to a shared or production database can cause irreversible schema changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@onboarding.md` around lines 393 - 395, Make the database-updater warning much more prominent in onboarding.md: emphasize the PortalDBVersion check and the updater in the portal startup path that applies scripts from PortalMVC/SQLScriptFix/, and add a short actionable safety checklist (verify the updater’s connection string points only to a local/dev instance, confirm connection string patterns do not match shared/production hosts, require an explicit env flag or confirmation before running the updater, and instruct how to disable the updater in production). Reference the PortalDBVersion table, the PortalMVC/SQLScriptFix/ folder, and the updater/startup path so readers can locate the updater, and convert the text into a clear, bolded warning block with the checklist steps to prevent accidental schema changes.
491-508: Consider separating meta-analysis from user documentation.The token evaluation section provides useful guidance for LLM usage but is unconventional in end-user documentation. While not problematic, this meta-content might be better suited for a separate document (e.g.,
CONTRIBUTING.mdorAI_USAGE.md) or a documentation maintenance guide.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@onboarding.md` around lines 491 - 508, The "12. Token Evaluation" meta-analysis should be extracted out of onboarding.md and moved to a separate contributor-focused doc (e.g., CONTRIBUTING.md or AI_USAGE.md); locate the "12. Token Evaluation" heading and the accompanying token table and move that entire block into the new file, leaving a short pointer in onboarding.md (one sentence) that references the new document; also update any internal links or TOC entries to point to the new file so readers of onboarding.md still find the guidance without mixing meta-analysis into end-user documentation.
352-357: Clarify version requirements and prerequisites.The prerequisites section mentions "SQL Server Developer Edition" and ".NET Framework support" but doesn't specify minimum versions. New engineers may need more specific guidance on compatible versions.
📋 Suggested enhancement
Consider adding specific version requirements:
### Prerequisites -Use a Windows development machine with Visual Studio or Rider, SQL Server Developer Edition, and .NET Framework support enabled. The primary solution file is `PortalMVC.sln`. +Use a Windows development machine with Visual Studio 2019+ or Rider 2021.3+, SQL Server 2016 Developer Edition or later, and .NET Framework 4.x support enabled. The primary solution file is `PortalMVC.sln`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@onboarding.md` around lines 352 - 357, Update the "## 9. Local Development Setup" prerequisites to specify exact minimum versions and supported tooling: state minimum SQL Server Developer Edition version (e.g., SQL Server 2019 or later), minimum .NET runtime/framework version and SDK (e.g., .NET Framework 4.8 or .NET 6/7 if applicable), and supported IDE versions for Visual Studio and Rider that work with the PortalMVC.sln; also mention any required SDKs or workloads (e.g., ".NET desktop development" or "ASP.NET and web development") and whether SQL Server Management Studio or LocalDB is acceptable for local testing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@onboarding.md`:
- Around line 371-382: Update the "Configure local application settings."
section (the block showing keys like FeatureFlags:ConfigPath,
Redis:ConnectionString, Cosmos:Endpoint, Cosmos:Key, AzureBlob:ConnectionString,
Notifications:FromEmail) to include a short, actionable paragraph explaining how
to obtain team-managed secrets: list the team or role to contact (e.g.,
infra/DevOps), the preferred channels (Slack channel name or email), the formal
request process (ticketing system or access request form), required approvals or
onboarding steps, and any ephemeral/local dev alternatives (e.g., use Azure
Storage emulator or development keys) plus a note to rotate secrets after access
is granted.
- Around line 393-395: Make the database-updater warning much more prominent in
onboarding.md: emphasize the PortalDBVersion check and the updater in the portal
startup path that applies scripts from PortalMVC/SQLScriptFix/, and add a short
actionable safety checklist (verify the updater’s connection string points only
to a local/dev instance, confirm connection string patterns do not match
shared/production hosts, require an explicit env flag or confirmation before
running the updater, and instruct how to disable the updater in production).
Reference the PortalDBVersion table, the PortalMVC/SQLScriptFix/ folder, and the
updater/startup path so readers can locate the updater, and convert the text
into a clear, bolded warning block with the checklist steps to prevent
accidental schema changes.
- Around line 491-508: The "12. Token Evaluation" meta-analysis should be
extracted out of onboarding.md and moved to a separate contributor-focused doc
(e.g., CONTRIBUTING.md or AI_USAGE.md); locate the "12. Token Evaluation"
heading and the accompanying token table and move that entire block into the new
file, leaving a short pointer in onboarding.md (one sentence) that references
the new document; also update any internal links or TOC entries to point to the
new file so readers of onboarding.md still find the guidance without mixing
meta-analysis into end-user documentation.
- Around line 352-357: Update the "## 9. Local Development Setup" prerequisites
to specify exact minimum versions and supported tooling: state minimum SQL
Server Developer Edition version (e.g., SQL Server 2019 or later), minimum .NET
runtime/framework version and SDK (e.g., .NET Framework 4.8 or .NET 6/7 if
applicable), and supported IDE versions for Visual Studio and Rider that work
with the PortalMVC.sln; also mention any required SDKs or workloads (e.g., ".NET
desktop development" or "ASP.NET and web development") and whether SQL Server
Management Studio or LocalDB is acceptable for local testing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task 01: Added onboarding.md
Summary by CodeRabbit