Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Web: Fix Discover setup access screen erroring when updating user #43558

Merged
merged 3 commits into from
Jun 26, 2024
Merged

Web: Fix Discover setup access screen erroring when updating user #43558

merged 3 commits into from
Jun 26, 2024

Conversation

kimlisa
Copy link
Contributor

@kimlisa kimlisa commented Jun 26, 2024 •
edited
Loading

fixes bug depicted in image below introduced with #42620

when updating or creating user, backend allows only traits or allTraits to be specified, not both, which was what the issues was.

Fetching a user returns both allTraits and traits, and we were sending of a request with both fields defined when updating.

This PR will prevent sending of both in the api service layer by requiring a excludeUserField param

image

changelog: Fixes Discover setup access step in the web UI from erroring when updating user

@github-actions github-actions bot requested review from avatus and ibeckermayer June 26, 2024 18:22
@github-actions GitHub Actions
Copy link

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@kimlisa kimlisa requested review from flyinghermit and removed request for ibeckermayer June 26, 2024 18:22
kimlisa
kimlisa commented Jun 26, 2024
View reviewed changes
web/packages/teleport/src/Users/useUsers.ts Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the main Users create/edit screen, will only ever edit the allTraits field, therefore we exclude the traits field

flyinghermit
flyinghermit approved these changes Jun 26, 2024
View reviewed changes
Copy link
Contributor

@flyinghermit flyinghermit left a comment

Choose a reason for hiding this comment

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

Thank you!

web/packages/teleport/src/services/user/types.ts Outdated Show resolved Hide resolved
web/packages/teleport/src/services/user/user.test.ts Outdated Show resolved Hide resolved
@kimlisa kimlisa enabled auto-merge June 26, 2024 19:31
@kimlisa kimlisa added this pull request to the merge queue Jun 26, 2024
Merged via the queue into master with commit d6deb0c Jun 26, 2024
40 checks passed
@kimlisa kimlisa deleted the lisa/fix-user-update branch June 26, 2024 19:57
@public-teleport-github-review-bot
Copy link

@kimlisa See the table below for backport results.

Branch Result
branch/v14 Failed
branch/v15 Create PR
branch/v16 Create PR

This was referenced Jun 26, 2024
Merged
Merged
kimlisa added a commit that referenced this pull request Jun 26, 2024
@kimlisa
Web: Fix Discover setup access screen erroring when updating user (#4…
dcc782e 
…3558)

* Web: fix discover user update error

* Address CRs
@kimlisa kimlisa mentioned this pull request Jun 26, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jun 26, 2024
@kimlisa
Web: Fix Discover setup access screen erroring when updating user (#4…
3cbef58 
…3558) (#43565)

* Web: fix discover user update error

* Address CRs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flyinghermit flyinghermit flyinghermit approved these changes

@avatus avatus avatus approved these changes

Assignees
No one assigned
Labels
backport/branch/v14 backport/branch/v15 backport/branch/v16 size/md ui
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kimlisa @avatus @flyinghermit