Skip to content

fix: document proper claim update logic#2520

Merged
hperl merged 3 commits into
masterfrom
hperl/document-traits-updates
Apr 23, 2026
Merged

fix: document proper claim update logic#2520
hperl merged 3 commits into
masterfrom
hperl/document-traits-updates

Conversation

@hperl
Copy link
Copy Markdown
Member

@hperl hperl commented Apr 21, 2026

Summary by CodeRabbit

  • Documentation
    • Updated social sign-in data mapping example to clarify how display names are selected and when they appear in emitted identity traits, improving clarity and reducing ambiguity for implementers.

@hperl hperl self-assigned this Apr 21, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 21, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 02c2ebb2-3681-4042-91e5-2398bca8b109

📥 Commits

Reviewing files that changed from the base of the PR and between 9212c2a and 428ad25.

📒 Files selected for processing (1)
  • docs/kratos/social-signin/90_data-mapping.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/kratos/social-signin/90_data-mapping.mdx

📝 Walkthrough

Walkthrough

The documentation file for Kratos social sign-in data mapping was updated: the Jsonnet example now precomputes a displayName using std.get (preferring identity.traits.display_name then claims.display_name), and conditionally includes the "display_name" trait only when displayName != "".

Changes

Cohort / File(s) Summary
Documentation Update
docs/kratos/social-signin/90_data-mapping.mdx
Refactored Jsonnet snippet to compute displayName with std.get, replaced previous inline conditional that checked identity.traits.display_name, and simplified conditional inclusion of the display_name trait.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is completely empty, missing all required sections including the summary, issue reference, and checklist items as specified in the template. Add a comprehensive description following the template: include a summary of changes, reference the related issue/design document, and complete the checklist items.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: document proper claim update logic' is specific and directly related to the PR's main change of updating a Jsonnet documentation example to show proper claim handling logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hperl/document-traits-updates

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the social sign-in data-mapping documentation to clarify/adjust how identity traits (notably display_name) should be updated during update_identity_on_login=automatic, while continuing to sync provider groups.

Changes:

  • Refactors the Jsonnet example to precompute a displayName value.
  • Updates the conditional trait output to use the computed displayName value.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/kratos/social-signin/90_data-mapping.mdx Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/kratos/social-signin/90_data-mapping.mdx`:
- Line 276: The dynamic object key that conditions on displayName uses an
incomplete Jsonnet conditional and will parse-fail; update the dynamic key
expression that checks displayName != "" (the conditional used to produce the
"display_name" field) to include an else branch (use else null) so the field is
omitted when empty, leaving the value expression (displayName) unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 863ffff9-c17a-4c62-b95c-d6f3a57078e0

📥 Commits

Reviewing files that changed from the base of the PR and between 7cc71ca and 9212c2a.

📒 Files selected for processing (1)
  • docs/kratos/social-signin/90_data-mapping.mdx

Comment thread docs/kratos/social-signin/90_data-mapping.mdx Outdated
@hperl hperl merged commit 337888a into master Apr 23, 2026
10 checks passed
@hperl hperl deleted the hperl/document-traits-updates branch April 23, 2026 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants