-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(auth-api-lib): Migration bff admin portal back_channel_logout_uri #16710
Conversation
WalkthroughThis pull request introduces a new migration file for updating the database schema related to the "client" table. It includes two asynchronous functions: Changes
Suggested labels
Suggested reviewers
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
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
libs/auth-api-lib/migrations/20241104143212-add_back_channel_logout_uri_to_client_bff-stjornbord.js (2)
1-1
: Remove redundant 'use strict' directiveThe 'use strict' directive is unnecessary in ES modules as they are automatically in strict mode.
-'use strict'
🧰 Tools
🪛 Biome
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
1-27
: Consider adding back_channel_logout_uri management to admin interfaceWhile this migration addresses the immediate need, consider adding the capability to manage
back_channel_logout_uri
through the IDS admin interface to avoid future migrations for similar changes.🧰 Tools
🪛 Biome
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
libs/auth-api-lib/migrations/20241104143212-add_back_channel_logout_uri_to_client_bff-stjornbord.js
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/auth-api-lib/migrations/20241104143212-add_back_channel_logout_uri_to_client_bff-stjornbord.js (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 Biome
libs/auth-api-lib/migrations/20241104143212-add_back_channel_logout_uri_to_client_bff-stjornbord.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
...th-api-lib/migrations/20241104143212-add_back_channel_logout_uri_to_client_bff-stjornbord.js
Show resolved
Hide resolved
...th-api-lib/migrations/20241104143212-add_back_channel_logout_uri_to_client_bff-stjornbord.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
libs/auth-api-lib/migrations/20241104143212-add_back_channel_logout_uri_to_client_bff-stjornbord.js (1)
1-2
: Remove redundant 'use strict' directiveThe 'use strict' directive is unnecessary as JavaScript modules are automatically in strict mode.
-'use strict' -🧰 Tools
🪛 Biome
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
libs/auth-api-lib/migrations/20241104143212-add_back_channel_logout_uri_to_client_bff-stjornbord.js
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/auth-api-lib/migrations/20241104143212-add_back_channel_logout_uri_to_client_bff-stjornbord.js (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 Biome
libs/auth-api-lib/migrations/20241104143212-add_back_channel_logout_uri_to_client_bff-stjornbord.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
🔇 Additional comments (2)
libs/auth-api-lib/migrations/20241104143212-add_back_channel_logout_uri_to_client_bff-stjornbord.js (2)
3-4
: LGTM! Good use of constant for client ID
Using a constant for the client ID improves maintainability and reduces the risk of typos.
1-27
: Verify client existence before applying migration
Let's verify that the client exists in the database before applying this migration.
✅ Verification successful
Let me analyze the database schema and seed data to verify if this client is properly initialized before the migration.
Client ID exists in configuration across environments
Based on the search results, the client ID @admin.island.is/bff-stjornbord
is properly configured in all environment values files (dev, staging, and production). This indicates the client is an expected part of the system setup. While we cannot directly verify the database state, the consistent presence in configuration files across all environments provides reasonable confidence that this is a valid client ID for the migration.
- Found in:
charts/islandis/values.dev.yaml
charts/islandis/values.staging.yaml
charts/islandis/values.prod.yaml
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for references to the BFF admin portal client ID
# to ensure it exists in the codebase
# Look for client ID references in SQL files and migrations
rg -t sql -t js "@admin.island.is/bff-stjornbord"
# Look for client configuration
fd -e json -e yaml -e config | xargs rg "@admin.island.is/bff-stjornbord"
Length of output: 588
Script:
#!/bin/bash
# Search for any seed data or initial client setup in auth-api-lib
fd -e sql -e js | rg -l "client" | xargs rg -A 5 -B 5 "@admin.island.is/bff-stjornbord"
# Look for client registration or setup code
ast-grep --pattern 'INSERT INTO "client"'
ast-grep --pattern 'CREATE TABLE "client"'
Length of output: 222
🧰 Tools
🪛 Biome
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
...th-api-lib/migrations/20241104143212-add_back_channel_logout_uri_to_client_bff-stjornbord.js
Show resolved
Hide resolved
...th-api-lib/migrations/20241104143212-add_back_channel_logout_uri_to_client_bff-stjornbord.js
Show resolved
Hide resolved
Datadog ReportAll test runs ✅ 70 Total Test Services: 0 Failed, 67 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (2) |
IDS db migration
What
Migration to update bff admin portal client back_channel_logout_uri
Why
This cant be done in the ids admin
Checklist:
Summary by CodeRabbit
New Features
@admin.island.is/bff-stjornbord
.Bug Fixes