LNK-3310: Admin UI doesn't work when anonymous authentication is configured #597
Conversation
Detecting if login is required in AppComponent. Showing navigation when login is _not_ required. Adding extra logging to CORS configuration/initialization for BFF. Extra: Fixing docker compose healthcheck command on MSSQL.
WalkthroughThis pull request involves a comprehensive renaming and restructuring of the Admin UI and Admin Backend-for-Frontend (BFF) components. The changes span multiple files across Azure Pipelines configurations, Dockerfiles, Angular project files, and source code. The primary focus is on transitioning from a "DemoApp" context to a more specific "Admin UI" and "Admin.BFF" project structure, including updates to selectors, paths, and configuration settings. Changes
Assessment against linked issues
Suggested Reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 1
🔭 Outside diff range comments (1)
Web/Admin.UI/src/app/app.component.html (1)
Routes require server-side authentication but lack client-side guards
Based on the verification:
- The application uses server-side authentication with login/logout endpoints
- No route guards are implemented to protect sensitive routes on the client side
- Routes like
/tenant,/measure-def,/accountare exposed without client-side protectionRecommendations:
- Implement an
AuthGuardto protect sensitive routes using the existingAuthenticationService- Add
canActivateguards to the following routes inapp-routing.module.ts:
/tenantand tenant-related routes/measure-defroutes/accountroutes/auditroutes/notification-configuration/integration-test🔗 Analysis chain
Line range hint
32-95: Verify access control for sensitive routesWhile the navigation is now visible with anonymous authentication, ensure that sensitive routes (e.g., tenant management, measure definitions) have appropriate guards to prevent unauthorized access.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for route guards in the routing configuration and components # Look for route guard implementations echo "Checking for route guards..." rg -t ts "implements.*Guard" src/ # Check route configurations for guard usage echo "Checking route configurations for guards..." rg -t ts "canActivate:|canActivateChild:" src/ # Look for any TODO comments related to authentication echo "Checking for authentication-related TODOs..." rg -i "todo.*auth|auth.*todo" src/Length of output: 519
Script:
#!/bin/bash # Description: Check for route guards in the routing configuration and components # Look for route guard implementations echo "Checking for route guards..." rg -t ts "implements.*Guard" Web/Admin.UI/src/ # Check route configurations for guard usage echo "Checking route configurations for guards..." rg -t ts "canActivate:|canActivateChild:" Web/Admin.UI/src/ # Look for any TODO comments related to authentication echo "Checking for authentication-related TODOs..." rg -i "todo.*auth|auth.*todo" Web/Admin.UI/src/ # Check app routing module for route configurations echo "Checking app routing module..." rg -t ts "const\s+routes.*=.*\[" -A 50 Web/Admin.UI/src/Length of output: 4268
Script:
#!/bin/bash # Let's check for authentication service and any auth-related code # Check for auth service implementations echo "Checking for auth service..." rg -t ts "class.*AuthService" Web/Admin.UI/src/ # Check for auth-related interceptors echo "Checking for auth interceptors..." rg -t ts "class.*Interceptor.*implements.*HttpInterceptor" Web/Admin.UI/src/ # Check app.component.ts for auth-related logic echo "Checking app component..." cat Web/Admin.UI/src/app/app.component.ts # Check for JWT or token handling echo "Checking for token handling..." rg -t ts "token|jwt|bearer" Web/Admin.UI/src/Length of output: 6845
Script:
#!/bin/bash # Let's check the authentication service implementation and security guards # Check authentication service implementation echo "Checking authentication service implementation..." cat Web/Admin.UI/src/app/services/security/authentication.service.ts # Check for any security-related guards echo "Checking for security guards..." fd -e ts -e js guard Web/Admin.UI/src/ -x cat {}Length of output: 2097
🧹 Nitpick comments (14)
Web/Admin.UI/src/app/components/tenant/tenant-dashboard/tenant-dashboard.component.ts (2)
48-48: Consider adding error handling or a fallback.
While invokinggetFacilities()on initialization is appropriate, it’s beneficial to handle potential subscription errors or failures to ensure the UI remains stable and users receive clear feedback.
56-56: No error handling ingetFacilities().
Currently, the subscription call has no.errorcallback. Consider including an error handler or a fallback to gracefully handle network or API failures.Web/Admin.UI/src/app/components/core/loading-indicator/loading-indicator.component.ts (1)
19-19: No functional changes; minor formatting tweak.This constructor formatting update does not introduce any functional changes. It’s purely stylistic and does not pose any issues.
Web/Admin.UI/src/app/app.component.ts (1)
19-19: Use a more descriptive default value or comment for clarity.The
loginRequiredis initially set totrue, which might be confusing in scenarios where login is not always mandatory. Consider adding a clarifying comment or defaulting it to the result of a check that ensures it aligns with real-world usage.DotNet/Admin.BFF/Application/Models/Configuration/CorsConfig.cs (1)
57-61: Provide documentation about the interaction betweenAllowAllOriginsandAllowedOrigins.When
AllowAllOriginsis set totrue, theAllowedOriginslist is effectively ignored. Ensure that the rest of the code and documentation consistently handle or explain these two properties, so future maintainers don't misinterpret the configuration.Web/Admin.UI/src/app/components/notification/notification-config-dialog/notification-config-dialog.component.ts (1)
49-49: Debugging code can be replaced or removed for production.You are currently logging form references in
ngAfterViewInit(). Ensure these logs are necessary or consider removing them in production to avoid cluttering the console.DotNet/Admin.BFF/Infrastructure/Extensions/Security/CorsServiceExtension.cs (2)
17-25: Consider clarifyingAllowAllOriginsvs.AllowedOriginspriority.When
AllowAllOriginsis false andAllowedOriginsis set, origins are explicitly restricted, which is secure. Ensure the documentation or naming clarifies which property takes precedence.
30-40: Check for debug logs in production.Multiple debug logs help trace CORS policy decisions. Decide whether to switch to a less verbose log level in production to avoid large log volumes.
Web/Admin.UI/src/app/components/notification/notification-dashboard/notification-dashboard.component.ts (1)
81-81: Confirm potential filters
Currently, only"All"is specified innotificationTypeFilter. If more notification types become relevant, ensure they are added or loaded dynamically for better maintainability.Web/Admin.UI/src/app/components/notification/notification-dashboard/create-notification-dialog/create-notification-dialog.component.ts (2)
42-42: Revisit placeholder comment
The comment indicates this is “temp until tenant functionality is brought into the ui.” Ensure you remove or update the comment once the feature is implemented.
195-195: Spelling in the method name
clearSubjetcould be renamed toclearSubjectfor clarity and consistency.- clearSubjet(): void { + clearSubject(): void {Azure_Pipelines/azure-pipelines.admin-ui.ci.yaml (2)
4-4: Fix trailing spaces in path includesRemove trailing spaces after the path entries for consistent YAML formatting.
- - Web/Admin.UI/* + - Web/Admin.UI/*Also applies to: 9-9
11-11: Update pipeline name to match serviceThe pipeline name "Demo API Gateway CI" seems inconsistent with the renamed Admin UI service.
-name: Demo API Gateway CI +name: Admin UI CIWeb/Admin.UI/src/app/app.component.html (1)
32-32: Add comments explaining the authentication logicThe condition
!loginRequired || userProfile?.emailhandles both anonymous and authenticated scenarios, but its purpose isn't immediately clear.- <mat-nav-list *ngIf="!loginRequired || userProfile?.email"> + <!-- Show navigation when either anonymous access is allowed (!loginRequired) or user is authenticated (userProfile?.email) --> + <mat-nav-list *ngIf="!loginRequired || userProfile?.email">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Web/Admin.UI/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (33)
Azure_Pipelines/azure-pipelines.admin-bff.cd.yaml(2 hunks)Azure_Pipelines/azure-pipelines.admin-bff.ci.yaml(3 hunks)Azure_Pipelines/azure-pipelines.admin-ui.cd.yaml(2 hunks)Azure_Pipelines/azure-pipelines.admin-ui.ci.yaml(2 hunks)DotNet/Admin.BFF/Application/Models/Configuration/CorsConfig.cs(1 hunks)DotNet/Admin.BFF/Dockerfile(1 hunks)DotNet/Admin.BFF/Infrastructure/Extensions/Security/CorsServiceExtension.cs(2 hunks)DotNet/Admin.BFF/Infrastructure/Extensions/Security/InitializeSecurity.cs(1 hunks)DotNet/Admin.BFF/Program.cs(2 hunks)Web/Admin.UI/Admin.UI.esproj(3 hunks)Web/Admin.UI/Dockerfile(1 hunks)Web/Admin.UI/README.md(1 hunks)Web/Admin.UI/angular.json(3 hunks)Web/Admin.UI/karma.conf.js(1 hunks)Web/Admin.UI/package.json(1 hunks)Web/Admin.UI/src/app/app.component.html(3 hunks)Web/Admin.UI/src/app/app.component.spec.ts(0 hunks)Web/Admin.UI/src/app/app.component.ts(4 hunks)Web/Admin.UI/src/app/components/audit/audit-dashboard/audit-dashboard.component.ts(4 hunks)Web/Admin.UI/src/app/components/core/delete-item-dialog/delete-item-dialog.component.ts(1 hunks)Web/Admin.UI/src/app/components/core/loading-indicator/loading-indicator.component.ts(2 hunks)Web/Admin.UI/src/app/components/dashboard/admin-dashboard/admin-dashboard.component.ts(1 hunks)Web/Admin.UI/src/app/components/notification/facility-configuration/notification-configuration.component.ts(5 hunks)Web/Admin.UI/src/app/components/notification/notification-config-dialog/notification-config-dialog.component.html(1 hunks)Web/Admin.UI/src/app/components/notification/notification-config-dialog/notification-config-dialog.component.ts(4 hunks)Web/Admin.UI/src/app/components/notification/notification-config-form/notification-config-form.component.ts(4 hunks)Web/Admin.UI/src/app/components/notification/notification-dashboard/create-notification-dialog/create-notification-dialog.component.ts(4 hunks)Web/Admin.UI/src/app/components/notification/notification-dashboard/notification-dashboard.component.ts(3 hunks)Web/Admin.UI/src/app/components/report/report-dashboard/report-dashboard.component.ts(1 hunks)Web/Admin.UI/src/app/components/tenant/facility-view/facility-view.component.html(2 hunks)Web/Admin.UI/src/app/components/tenant/tenant-dashboard/tenant-dashboard.component.ts(2 hunks)Web/Admin.UI/src/app/components/testing/integration-test/integration-test.component.ts(1 hunks)Web/Admin.UI/src/app/components/theme-showcase/theme-showcase.component.ts(1 hunks)
💤 Files with no reviewable changes (1)
- Web/Admin.UI/src/app/app.component.spec.ts
✅ Files skipped from review due to trivial changes (7)
- Web/Admin.UI/package.json
- Web/Admin.UI/README.md
- Web/Admin.UI/src/app/components/report/report-dashboard/report-dashboard.component.ts
- Web/Admin.UI/karma.conf.js
- Web/Admin.UI/src/app/components/notification/facility-configuration/notification-configuration.component.ts
- Web/Admin.UI/src/app/components/audit/audit-dashboard/audit-dashboard.component.ts
- Web/Admin.UI/src/app/components/notification/notification-config-form/notification-config-form.component.ts
🧰 Additional context used
🪛 yamllint (1.35.1)
Azure_Pipelines/azure-pipelines.admin-ui.ci.yaml
[error] 5-5: trailing spaces
(trailing-spaces)
🪛 Biome (1.9.4)
Web/Admin.UI/angular.json
[error] 108-108: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 109-109: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
[error] 109-109: End of file expected
Use an array for a sequence of values: [1, 2]
(parse)
🔇 Additional comments (36)
Web/Admin.UI/src/app/components/theme-showcase/theme-showcase.component.ts (1)
20-20: Good shift to a more standardized naming convention.
Renaming the selector helps maintain consistency with the 'app-*' namespace throughout the UI. This change ensures a more intuitive component reference in HTML templates.
Web/Admin.UI/src/app/components/testing/integration-test/integration-test.component.ts (1)
39-39: Good consistency with the standard naming convention.
Changing the selector to 'app-integration-test' aligns well with the project's updated naming approach. This ensures consistency across selectors, making the code more maintainable.
Web/Admin.UI/src/app/components/tenant/tenant-dashboard/tenant-dashboard.component.ts (1)
17-17: Selector updated to standard naming convention.
Renaming demo-tenant-dashboard to app-tenant-dashboard aligns well with typical Angular naming patterns.
Web/Admin.UI/src/app/components/core/loading-indicator/loading-indicator.component.ts (1)
7-7: Appropriate rename of the component’s selector.
Renaming from demo-loading-indicator to app-loading-indicator aligns with the updated naming conventions across the application.
Web/Admin.UI/src/app/components/core/delete-item-dialog/delete-item-dialog.component.ts (1)
8-8: Selector rename maintained consistently.
Renaming from demo-delete-item-dialog to app-delete-item-dialog follows the new naming pattern across the codebase.
Web/Admin.UI/src/app/components/dashboard/admin-dashboard/admin-dashboard.component.ts (1)
9-9: Selector renamed to match updated naming scheme.
Moving from demo-admin-dashboard to app-admin-dashboard is in line with the standardized app- conventions.
Web/Admin.UI/Admin.UI.esproj (2)
11-11: Output folder rename aligns with rebranding efforts.
Changing from dist/demo-app to dist/admin-ui ensures consistency with the new project naming.
1-1: SDK version upgrade check.
Upgrading from 0.5.74-alpha to 1.0.2191419 may require verifying compatibility with other build processes and tooling.
✅ Verification successful
SDK version upgrade appears safe
The upgrade to Microsoft.VisualStudio.JavaScript.Sdk version 1.0.2191419 appears to be compatible with the project's setup:
- The project uses Angular 15.x with TypeScript, which is well-supported by the newer SDK version
- The package.json shows standard Angular dependencies and build processes
- No other projects in the codebase are using the JavaScript SDK, so there are no version conflicts to worry about
- The project uses standard npm commands for build and test, which are supported across SDK versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Checking references to older JavaScript Sdk lines in the codebase
rg "Microsoft.VisualStudio.JavaScript.Sdk" -A 3
Length of output: 357
Script:
#!/bin/bash
# Let's check the git history for this file to see the previous SDK version
git log -p -- Web/Admin.UI/Admin.UI.esproj | grep -A 3 "Microsoft.VisualStudio.JavaScript.Sdk"
# Also check for any package.json to verify Node.js dependencies
fd package.json --exec cat {}
Length of output: 1401
Web/Admin.UI/src/app/app.component.ts (4)
31-31: Subscription initialization looks immediate.
This subscription is established in the constructor, binding updates to this.userProfile. This approach is appropriate for listening to profile changes. Verify that unsubscribing is handled elsewhere if the component's lifecycle can end before the service completes its emissions.
40-40: Load-time authentication check.
Fetching isLoginRequired asynchronously on component initialization is a good approach. Verify that any other dependent services or child components are prepared for cases where loginRequired might still be unknown temporarily.
54-56: Conditional logout behavior.
The added check before logging out is beneficial when dealing with anonymous scenarios. Ensure the rest of the application code properly handles these no-login-needed flows so that session handling remains consistent.
43-43: Conditional login trigger.
The empty string check on this.userProfile.email could fail if the property is undefined. Consider using optional chaining (this.userProfile?.email) or a null/undefined guard to avoid potential errors.
- if (this.userProfile.email === '' && this.loginRequired) {
+ if ((this.userProfile?.email ?? '') === '' && this.loginRequired) {
this.authService.login();
}Web/Admin.UI/src/app/components/notification/notification-config-dialog/notification-config-dialog.component.ts (3)
13-13: Selector rename for consistent naming conventions.
Renaming the selector to app-notification-config-dialog is aligned with Angular best practices. Ensure that any references in templates or tests are updated accordingly.
34-34: Constructor injection is correct.
All dependencies (dialog data, dialog reference, snackBar) are properly defined and injected. This ensures consistent usage of Angular's DI system.
60-60: Error handling on creation failure.
Alerting the user with a snackbar is a good UX approach. Verify that error details are also properly logged in the console or a monitoring system for easier debugging.
DotNet/Admin.BFF/Infrastructure/Extensions/Security/CorsServiceExtension.cs (2)
8-8: Logger injection is a good addition.
Passing an optional Serilog.ILogger allows more fine-grained debugging. Confirm that any calling code either supplies a logger or handles the null scenario gracefully.
46-47: Using defaults when arrays are null is a clean approach.
Applying AllowedMethods ?? DefaultAllowedMethods and AllowedExposedHeaders ?? DefaultAllowedExposedHeaders is straightforward and reduces code duplication.
Web/Admin.UI/src/app/components/notification/notification-dashboard/notification-dashboard.component.ts (2)
27-27: Use consistent selectors across the app
Renaming the selector to app-notification-dashboard follows a more standardized naming convention and ensures clarity in referencing the component. Good job!
132-132: No functional changes
This line marks the end of the method, introducing no feature or logic change.
Web/Admin.UI/src/app/components/notification/notification-dashboard/create-notification-dialog/create-notification-dialog.component.ts (2)
20-20: Selector renamed for clarity
Switching from demo-create-notification-dialog to app-create-notification-dialog aligns with the updated naming scheme.
219-219: No net changes
This appears to be a blank or formatting line.
DotNet/Admin.BFF/Infrastructure/Extensions/Security/InitializeSecurity.cs (1)
236-236: No additional changes beyond final bracket
No functional or structural adjustments are introduced here.
DotNet/Admin.BFF/Program.cs (2)
426-426: Trivial formatting change
No significant impact on functionality; looks good.
179-200: Centralized CORS setup
Configuring CORS here regardless of authentication mode is an excellent approach to ensure consistent behavior. Consider verifying that logging statements do not expose sensitive data.
✅ Verification successful
Let me check for any potential sensitive data exposure in CORS-related logging by searching for the actual CORS configuration logging.
CORS logging implementation is safe and well-structured
After reviewing the CORS-related logging statements, I can confirm that the logging is implemented safely. The debug logs only include configuration details that are not sensitive, such as allowed origins, headers, and methods. The implementation in CorsServiceExtension.cs follows good practices by using debug level for configuration details and not exposing any sensitive information.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# This script checks for any sensitive data in log statements related to CORS.
rg "Log\.Logger\.(Information|Debug)\(" -A 3
Length of output: 3069
Script:
#!/bin/bash
# Search for CORS configuration logging in more detail
rg "CORS" -A 5 --type cs
Length of output: 18741
Web/Admin.UI/Dockerfile (1)
18-18: Verify Angular build output directory configuration
The path change from demo-app to admin-ui aligns with the project restructuring. However, we should verify that this matches the outputPath configuration in Angular.
✅ Verification successful
Build output directory configuration is correctly aligned
The outputPath in angular.json is configured as "dist/admin-ui", which perfectly matches the path used in the Dockerfile's COPY instruction: /build/dist/admin-ui/.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Angular build output path configuration matches Dockerfile
# Test: Search for output path configuration in angular.json
rg -A 5 '"outputPath"' Web/Admin.UI/angular.json
Length of output: 281
Azure_Pipelines/azure-pipelines.admin-bff.ci.yaml (1)
4-4: Verify path consistency across the codebase
The path changes from LinkAdmin.BFF to Admin.BFF are consistent within this file. However, we should verify that all related configurations and references have been updated.
Also applies to: 13-13, 25-25
✅ Verification successful
Several files still contain references to LinkAdmin.BFF and need to be updated
The verification revealed that while the pipeline configuration has been updated, there are still numerous files containing references to the old LinkAdmin.BFF naming. These need to be updated for consistency:
- Multiple health check files in
DotNet/Admin.BFF/Infrastructure/Health/ - Authentication-related files like
LinkBearerServiceAuthExtension.csandLinkClaimsTransformer.cs - Configuration files containing references like
LinkAdminConstants.csandILinkAdminMetrics.cs - Infrastructure components like
LinkAzureKeyVault.cs - Various application interfaces, models, and endpoints still using the old naming
Let's verify if these are just namespace references or actual file/path dependencies:
Path changes in pipeline are correct, namespace references don't require updates
The investigation reveals that the references to LinkAdmin.BFF are primarily within namespace declarations and using statements (e.g., LantanaGroup.Link.LinkAdmin.BFF.*), which are part of the code's namespace hierarchy and don't require updates. These are not file path references that would need to be changed.
The pipeline configuration correctly updates the physical path from LinkAdmin.BFF to Admin.BFF, which is the only change needed as it refers to the actual directory structure.
The solution file (*.sln) also correctly references the project as Admin.BFF, confirming that the structural changes are properly aligned.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to the old path
# Test 1: Search for any remaining references to LinkAdmin.BFF
echo "Checking for old LinkAdmin.BFF references..."
rg -l "LinkAdmin\.BFF"
# Test 2: Verify solution file updates
echo "Checking solution file references..."
fd -e sln -x cat {} \; | grep -i "Admin.BFF"
Length of output: 7451
Script:
#!/bin/bash
# Check the context of these references to determine if they're namespaces or actual paths
echo "Checking context of LinkAdmin.BFF references in a few key files..."
rg "LinkAdmin\.BFF" DotNet/Admin.BFF/Program.cs -C 2
rg "LinkAdmin\.BFF" DotNet/Admin.BFF/Infrastructure/SecretManagers/LinkAzureKeyVault.cs -C 2
rg "LinkAdmin\.BFF" DotNet/Admin.BFF/Settings/LinkAdminConstants.cs -C 2
Length of output: 2445
Web/Admin.UI/src/app/components/notification/notification-config-dialog/notification-config-dialog.component.html (2)
Line range hint 1-11: LGTM! Template structure and bindings are well-organized
The dialog template maintains a clean structure with proper material dialog components and conditional rendering.
3-3: Verify component selector updates across the application
The selector change from demo-notification-config-form to app-notification-config-form looks good. Let's verify all related component references have been updated consistently.
✅ Verification successful
Let's check the remaining instance to ensure it's properly handled.
Component selector updates are consistent
The verification shows that the demo- prefix has been properly removed across the application. The only match found was in the theme-picker component, but upon inspection, it's actually using selector: 'theme-picker' without any demo- prefix. The file was likely a false positive due to other content (possibly in comments or documentation).
The change from demo-notification-config-form to app-notification-config-form is consistent with the codebase, and no remaining inconsistent usages were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining demo- prefixed components
# Test 1: Search for any remaining demo- prefixed components in templates
echo "Checking for remaining demo- prefixed components..."
rg -l "demo-[a-z-]+" "Web/Admin.UI/src"
# Test 2: Search for demo selectors in component decorators
echo "Checking for demo selectors in components..."
ast-grep --pattern 'selector: "demo-$_"' "Web/Admin.UI/src"
Length of output: 387
Script:
#!/bin/bash
# Check the specific content of theme-picker component
echo "Content of theme-picker.component.ts:"
cat "Web/Admin.UI/src/app/components/core/theme-picker/theme-picker.component.ts"
# Search for any usage of theme-picker component with demo prefix
echo -e "\nChecking usage of theme-picker component:"
rg "demo-theme-picker" "Web/Admin.UI/src"
Length of output: 4405
DotNet/Admin.BFF/Dockerfile (1)
14-15: LGTM! Clean and consistent renaming
The changes correctly update all references from LinkAdmin.BFF to Admin.BFF while maintaining the proper multi-stage build pattern and security best practices.
Also applies to: 19-19, 21-21, 24-24, 29-29
Azure_Pipelines/azure-pipelines.admin-bff.cd.yaml (2)
14-14: LGTM! Path updates are consistent
The changes correctly update all path references from LinkAdmin.BFF to Admin.BFF.
Also applies to: 28-28, 32-32
Line range hint 1-91: Verify alignment with PR objectives
While the renaming changes are correct, the PR objectives mention fixing CORS and anonymous authentication issues. However, this pipeline configuration doesn't include any changes related to those objectives. Please verify if additional configuration changes are needed for CORS or authentication settings.
Azure_Pipelines/azure-pipelines.admin-ui.cd.yaml (1)
14-14: LGTM! Pipeline configuration updated correctly
The changes consistently reflect the transition from DemoApp to Admin.UI across paths and variables.
Also applies to: 28-32
Web/Admin.UI/angular.json (1)
6-6: LGTM! Angular configuration updated consistently
The project renaming from DemoApp to AdminUI is applied consistently across all relevant configuration entries.
Also applies to: 16-16, 101-104, 112-112
Web/Admin.UI/src/app/app.component.html (2)
1-1: LGTM! Component updates are consistent
The loading indicator selector and title updates align with the project renaming.
Also applies to: 11-11
Line range hint 14-26: LGTM! User profile section maintains security
The user profile section correctly remains dependent on the presence of user email, ensuring proper authentication state handling.
Web/Admin.UI/src/app/components/tenant/facility-view/facility-view.component.html (1)
125-125: Verify the app-report-dashboard component registration
The component rename from demo-report-dashboard to app-report-dashboard aligns with the project-wide standardization of component prefixes.
Let's verify the component is properly registered:
✅ Verification successful
Component rename verification successful
The verification confirms that:
- The
app-report-dashboardcomponent exists atWeb/Admin.UI/src/app/components/report/report-dashboard/ - The component is properly registered with selector
app-report-dashboard - It's a standalone component with the correct inputs (
facilityIdanddisplayReportDashboard) - No references to the old
demo-report-dashboardselector remain in the codebase - The component is correctly imported and declared in
facility-view.component.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the report dashboard component exists and is properly registered
# Check for the component definition
ast-grep --pattern 'selector: "app-report-dashboard"'
# Check for any remaining usages of the old selector
rg "demo-report-dashboard"
Length of output: 82
Script:
#!/bin/bash
# Let's try a broader search to find the component files and registrations
# Find report dashboard component files
fd "report-dashboard.component"
# Search for any component with report-dashboard in its selector
ast-grep --pattern 'selector: $_'
# Look for imports or declarations of ReportDashboard
rg -i "reportdashboard" -A 2 -B 2
Length of output: 9174
Script:
#!/bin/bash
# Let's check the component decorator to verify the selector
# Check the component decorator in the report dashboard component
cat "Web/Admin.UI/src/app/components/report/report-dashboard/report-dashboard.component.ts" | grep -A 5 "@Component"
# Search for any remaining references to demo-report-dashboard
rg "demo-report-dashboard" -l
Length of output: 265
Web/Admin.UI/src/app/components/tenant/facility-view/facility-view.component.html
Outdated
Show resolved
Hide resolved
🛠️ Description of Changes
Always configuring CORS in BFF regardless of anonymous auth config.
Detecting if login is required in AppComponent.
Showing navigation when login is not required.
Adding extra logging to CORS configuration/initialization for BFF.
Extra: Fixing docker compose healthcheck command on MSSQL.
🧪 Testing Performed
Built/ran BFF in Docker Compose, which is configured to use anonymous authentication. Observed that CORS was correctly being registered regardless of anonymous auth.
Built/ran UI in both Docker Compose and locally against the BFF running in Docker Compose.
Opened the UI and observed that it was correctly showing the navigation, and that the Tenants screen successfully received responses from the BFF for a list of tenants (although I don't have any tenants configured). Observed similar successes when looking at audits (though I don't have any audits in the db).
📓 Documentation Updated