feat(auth-api): Filter delegation indexing by provider #16141
Conversation
WalkthroughThe changes introduce a new constant, Changes
Possibly related PRs
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
|
saevarma
added
the
deprecated:automerge
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
libs/auth-api-lib/src/lib/delegations/delegations-index.service.ts (1)
45-54: LGTM! Consider enhancing the comment for future maintainers.The addition of
INDEXED_DELEGATION_PROVIDERSaligns well with the PR objective of filtering delegation indexing by provider. The comment provides good context for the change.Consider adding a brief explanation of what "indexed" means in this context to make the comment even more informative for future maintainers. For example:
// When delegation providers have been refactored to use the webhook method // with hard check on action we need to exclude them from the standard indexing. // We register our current providers as indexed, as all new providers are expected -// to use the webhook method. +// to use the webhook method. "Indexed" providers are those that still rely on +// the standard indexing process rather than the webhook method.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/auth-api-lib/src/lib/delegations/delegations-index.service.ts (2 hunks)
Additional context used
Path-based instructions (1)
libs/auth-api-lib/src/lib/delegations/delegations-index.service.ts (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."
Additional comments not posted (2)
libs/auth-api-lib/src/lib/delegations/delegations-index.service.ts (2)
Line range hint
1-724: Summary: Changes align well with PR objectives and maintain code quality.The modifications in this file successfully implement the filtering mechanism for delegation indexing based on providers, as outlined in the PR objectives. The changes are well-documented and maintain good code quality and readability. They address the concern of potential data loss during reindexing for new providers using the webhook method.
Key points:
- The new
INDEXED_DELEGATION_PROVIDERSconstant clearly defines which providers should be included in the standard indexing process.- The
saveToIndexmethod has been updated to use this constant, ensuring only relevant delegations are processed.These changes appear to meet the requirements without introducing any obvious issues. The code adheres to the coding guidelines for the
libs/**/*pattern, maintaining reusability and effective TypeScript usage.
357-357: LGTM! Verify the impact on existing functionality.The modification to filter delegations based on
INDEXED_DELEGATION_PROVIDERSaligns perfectly with the PR objective. This change ensures that only delegations from indexed providers are processed during reindexing, preventing unintended data loss for new providers using the webhook method.To ensure this change doesn't unintentionally affect existing functionality, let's verify the usage of
saveToIndexmethod:This script will help us identify any potential areas that might need adjustment due to the new filtering mechanism.
Verification successful
Please execute the following updated shell scripts to continue the verification process:
Please execute the following shell script to identify the actual
INDEXED_DELEGATION_PROVIDERS:
Please execute the following shell script to extract the actual values of
INDEXED_DELEGATION_PROVIDERS:
Verified the impact on existing functionality.
All direct calls to
saveToIndexare correctly filtered based onINDEXED_DELEGATION_PROVIDERS(CompanyRegistry,Custom,NationalRegistry,PersonalRepresentativeRegistry). This ensures that only delegations from these indexed providers are processed, aligning with the PR objectives and preventing unintended data loss for new providers using the webhook method.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any direct calls to saveToIndex that might be affected by this change # Search for direct calls to saveToIndex echo "Direct calls to saveToIndex:" rg --type typescript "saveToIndex\(" libs/auth-api-lib # Search for any references to non-indexed providers that might be affected echo "\nReferences to non-indexed providers:" rg --type typescript "AuthDelegationProvider\." libs/auth-api-lib | grep -vE "$(echo $INDEXED_DELEGATION_PROVIDERS | tr ',' '|')"Length of output: 382
Script:
#!/bin/bash # Description: Check for any direct calls to saveToIndex that might be affected by this change # Define INDEXED_DELEGATION_PROVIDERS if not already set INDEXED_DELEGATION_PROVIDERS="${INDEXED_DELEGATION_PROVIDERS:-provider1,provider2}" # Replace with actual providers as needed # Search for direct calls to saveToIndex in .ts and .tsx files echo "Direct calls to saveToIndex:" rg "saveToIndex\(" --glob "*.ts" --glob "*.tsx" libs/auth-api-lib # Search for any references to non-indexed providers that might be affected echo "\nReferences to non-indexed providers:" rg "AuthDelegationProvider\." --glob "*.ts" --glob "*.tsx" libs/auth-api-lib | grep -vE "$(echo $INDEXED_DELEGATION_PROVIDERS | tr ',' '|')"Length of output: 6701
Script:
#!/bin/bash # Description: Locate the definition of INDEXED_DELEGATION_PROVIDERS in the codebase echo "Searching for the definition of INDEXED_DELEGATION_PROVIDERS..." rg "INDEXED_DELEGATION_PROVIDERS" --glob "*.ts" --glob "*.tsx" libs/auth-api-libLength of output: 438
Script:
#!/bin/bash # Description: Extract the elements of the INDEXED_DELEGATION_PROVIDERS array echo "Extracting INDEXED_DELEGATION_PROVIDERS values..." rg -A 10 "const INDEXED_DELEGATION_PROVIDERS\s*=" --glob "*.ts" --glob "*.tsx" libs/auth-api-lib | \ grep -E "AuthDelegationProvider\." | \ sed -E 's/.*AuthDelegationProvider\.([A-Za-z0-9_]+).*/\1/' | \ awk '{print $1}' | sort | uniqLength of output: 409
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16141 +/- ##
==========================================
- Coverage 36.71% 36.70% -0.01%
==========================================
Files 6761 6761
Lines 139076 139077 +1
Branches 39501 39501
==========================================
- Hits 51055 51054 -1
- Misses 88021 88023 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* feat(ids-api): Use syslumenn api to verify delegation. (#16029) * Use syslumenn api to verify delegation. * Fix api error case. * Fix config. * chore: nx format:write update dirty files * Handle error in api. * Fix typo in name. * Refactor mock. * Update infra. * Update host in infra. * Add syslumenn infra to other auth apis. * Also return empty array if error. * Use post. * Fix tests. * Single delegation type. * Remove infra config from pr public. * Fix type. * Refactor error handling in check for scopes. * Openapi fix. * Refactor verification error handling. * Fix status code. * Decrease syslumenn api timeout to 3s. --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: Valur Einarsson <valure@live.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(auth-api): Filter delegation indexing by provider (#16141) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * feat(portals-admin-ids-admin): Add LegalRepresentative delegation type (#16069) * Add missing translation for legal representation delegation type in IDS Admin * Refactor delegation type filtering from client and scope creation to account for LegalRepresentative as superuser only. * Update patch super user delegation type check. Add tests. * Handle legal representative in scope patch. Add tests. * Hide it from the UI. * Fix duplicated message id --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: Valur Einarsson <valure@live.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Filter the delegation indexing by providers, as new providers are now using the webhook and providing hard check method, so their delegations are deleted if not protected by providers filter.
Tests will be added in a follow up PR due to pressure of starting to test this.
Why
So not all delegations are removed on reindexing.
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes